Skip to content

ref: Use getCurrentScope() / getClient instead of hub.xxx #9862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
import { NavigationCancel, NavigationError, Router } from '@angular/router';
import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router';
import { WINDOW, getCurrentHub } from '@sentry/browser';
import { WINDOW, getCurrentScope } from '@sentry/browser';
import type { Span, Transaction, TransactionContext } from '@sentry/types';
import { logger, stripUrlQueryAndFragment, timestampInSeconds } from '@sentry/utils';
import type { Observable } from 'rxjs';
Expand Down Expand Up @@ -50,14 +50,7 @@ export const instrumentAngularRouting = routingInstrumentation;
* Grabs active transaction off scope
*/
export function getActiveTransaction(): Transaction | undefined {
const currentHub = getCurrentHub();

if (currentHub) {
const scope = currentHub.getScope();
return scope.getTransaction();
}

return undefined;
return getCurrentScope().getTransaction();
}

/**
Expand Down
12 changes: 4 additions & 8 deletions packages/angular/test/tracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,12 @@ jest.mock('@sentry/browser', () => {
const original = jest.requireActual('@sentry/browser');
return {
...original,
getCurrentHub: () => {
getCurrentScope() {
return {
getScope: () => {
return {
getTransaction: () => {
return transaction;
},
};
getTransaction: () => {
return transaction;
},
} as unknown as Hub;
};
},
};
});
Expand Down
10 changes: 6 additions & 4 deletions packages/astro/src/server/meta.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getDynamicSamplingContextFromClient } from '@sentry/core';
import type { Hub, Span } from '@sentry/types';
import type { Client, Scope, Span } from '@sentry/types';
import {
TRACEPARENT_REGEXP,
dynamicSamplingContextToSentryBaggageHeader,
Expand All @@ -22,9 +22,11 @@ import {
*
* @returns an object with the two serialized <meta> tags
*/
export function getTracingMetaTags(span: Span | undefined, hub: Hub): { sentryTrace: string; baggage?: string } {
const scope = hub.getScope();
const client = hub.getClient();
export function getTracingMetaTags(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lms24 Refactored this to take a scope & client directly, this is not exported as far as I can tell?

span: Span | undefined,
scope: Scope,
client: Client | undefined,
): { sentryTrace: string; baggage?: string } {
const { dsc, sampled, traceId } = scope.getPropagationContext();
const transaction = span?.transaction;

Expand Down
16 changes: 8 additions & 8 deletions packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {
captureException,
continueTrace,
getCurrentHub,
getClient,
getCurrentScope,
runWithAsyncContext,
startSpan,
} from '@sentry/node';
import type { Hub, Span } from '@sentry/types';
import type { Client, Scope, Span } from '@sentry/types';
import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
import type { APIContext, MiddlewareResponseHandler } from 'astro';

Expand Down Expand Up @@ -69,7 +69,7 @@ export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseH
// if there is an active span, we know that this handle call is nested and hence
// we don't create a new domain for it. If we created one, nested server calls would
// create new transactions instead of adding a child span to the currently active span.
if (getCurrentHub().getScope().getSpan()) {
if (getCurrentScope().getSpan()) {
return instrumentRequest(ctx, next, handlerOptions);
}
return runWithAsyncContext(() => {
Expand Down Expand Up @@ -139,8 +139,8 @@ async function instrumentRequest(
span.setHttpStatus(originalResponse.status);
}

const hub = getCurrentHub();
const client = hub.getClient();
const scope = getCurrentScope();
const client = getClient();
const contentType = originalResponse.headers.get('content-type');

const isPageloadRequest = contentType && contentType.startsWith('text/html');
Expand All @@ -163,7 +163,7 @@ async function instrumentRequest(
start: async controller => {
for await (const chunk of originalBody) {
const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk);
const modifiedHtml = addMetaTagToHead(html, hub, span);
const modifiedHtml = addMetaTagToHead(html, scope, client, span);
controller.enqueue(new TextEncoder().encode(modifiedHtml));
}
controller.close();
Expand All @@ -185,12 +185,12 @@ async function instrumentRequest(
* This function optimistically assumes that the HTML coming in chunks will not be split
* within the <head> tag. If this still happens, we simply won't replace anything.
*/
function addMetaTagToHead(htmlChunk: string, hub: Hub, span?: Span): string {
function addMetaTagToHead(htmlChunk: string, scope: Scope, client: Client, span?: Span): string {
if (typeof htmlChunk !== 'string') {
return htmlChunk;
}

const { sentryTrace, baggage } = getTracingMetaTags(span, hub);
const { sentryTrace, baggage } = getTracingMetaTags(span, scope, client);
const content = `<head>\n${sentryTrace}\n${baggage}\n`;
return htmlChunk.replace('<head>', content);
}
Expand Down
39 changes: 18 additions & 21 deletions packages/astro/test/server/meta.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,20 @@ const mockedSpan = {
environment: 'production',
}),
},
};
} as any;

const mockedHub = {
getScope: () => ({
getPropagationContext: () => ({
traceId: '123',
}),
const mockedClient = {} as any;

const mockedScope = {
getPropagationContext: () => ({
traceId: '123',
}),
getClient: () => ({}),
};
} as any;

describe('getTracingMetaTags', () => {
it('returns the tracing tags from the span, if it is provided', () => {
{
// @ts-expect-error - only passing a partial span object
const tags = getTracingMetaTags(mockedSpan, mockedHub);
const tags = getTracingMetaTags(mockedSpan, mockedScope, mockedClient);

expect(tags).toEqual({
sentryTrace: '<meta name="sentry-trace" content="12345678901234567890123456789012-1234567890123456-1"/>',
Expand All @@ -35,10 +33,9 @@ describe('getTracingMetaTags', () => {
});

it('returns propagationContext DSC data if no span is available', () => {
const tags = getTracingMetaTags(undefined, {
...mockedHub,
// @ts-expect-error - only passing a partial scope object
getScope: () => ({
const tags = getTracingMetaTags(
undefined,
{
getPropagationContext: () => ({
traceId: '12345678901234567890123456789012',
sampled: true,
Expand All @@ -49,8 +46,9 @@ describe('getTracingMetaTags', () => {
trace_id: '12345678901234567890123456789012',
},
}),
}),
});
} as any,
mockedClient,
);

expect(tags).toEqual({
sentryTrace: expect.stringMatching(
Expand All @@ -73,7 +71,8 @@ describe('getTracingMetaTags', () => {
toTraceparent: () => '12345678901234567890123456789012-1234567890123456-1',
transaction: undefined,
},
mockedHub,
mockedScope,
mockedClient,
);

expect(tags).toEqual({
Expand All @@ -93,10 +92,8 @@ describe('getTracingMetaTags', () => {
toTraceparent: () => '12345678901234567890123456789012-1234567890123456-1',
transaction: undefined,
},
{
...mockedHub,
getClient: () => undefined,
},
mockedScope,
undefined,
);

expect(tags).toEqual({
Expand Down
27 changes: 13 additions & 14 deletions packages/astro/test/server/middleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as SentryNode from '@sentry/node';
import { vi } from 'vitest';
import type { Client } from '@sentry/types';
import { SpyInstance, vi } from 'vitest';

import { handleRequest, interpolateRouteFromUrlAndParams } from '../../src/server/middleware';

Expand All @@ -14,14 +15,17 @@ describe('sentryMiddleware', () => {
const startSpanSpy = vi.spyOn(SentryNode, 'startSpan');

const getSpanMock = vi.fn(() => {});
// @ts-expect-error only returning a partial hub here
vi.spyOn(SentryNode, 'getCurrentHub').mockImplementation(() => {
return {
getScope: () => ({
const setUserMock = vi.fn();

beforeEach(() => {
vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => {
return {
setUser: setUserMock,
setPropagationContext: vi.fn(),
getSpan: getSpanMock,
}),
getClient: () => ({}),
};
} as any;
});
vi.spyOn(SentryNode, 'getClient').mockImplementation(() => ({}) as Client);
});

const nextResult = Promise.resolve(new Response(null, { status: 200, headers: new Headers() }));
Expand Down Expand Up @@ -170,10 +174,6 @@ describe('sentryMiddleware', () => {
});

it('attaches client IP and request headers if options are set', async () => {
const scope = { setUser: vi.fn(), setPropagationContext: vi.fn() };
// @ts-expect-error, only passing a partial Scope object
const getCurrentScopeSpy = vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => scope);

const middleware = handleRequest({ trackClientIp: true, trackHeaders: true });
const ctx = {
request: {
Expand All @@ -192,8 +192,7 @@ describe('sentryMiddleware', () => {
// @ts-expect-error, a partial ctx object is fine here
await middleware(ctx, next);

expect(getCurrentScopeSpy).toHaveBeenCalledTimes(1);
expect(scope.setUser).toHaveBeenCalledWith({ ip_address: '192.168.0.1' });
expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' });

expect(startSpanSpy).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down
5 changes: 2 additions & 3 deletions packages/browser/src/eventbuilder.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getCurrentHub } from '@sentry/core';
import { getClient } from '@sentry/core';
import type { Event, EventHint, Exception, Severity, SeverityLevel, StackFrame, StackParser } from '@sentry/types';
import {
addExceptionMechanism,
Expand Down Expand Up @@ -48,8 +48,7 @@ export function eventFromPlainObject(
syntheticException?: Error,
isUnhandledRejection?: boolean,
): Event {
const hub = getCurrentHub();
const client = hub.getClient();
const client = getClient();
const normalizeDepth = client && client.getOptions().normalizeDepth;

const event: Event = {
Expand Down
19 changes: 5 additions & 14 deletions packages/browser/src/profiling/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable max-lines */

import { DEFAULT_ENVIRONMENT, getClient, getCurrentHub } from '@sentry/core';
import { DEFAULT_ENVIRONMENT, getClient } from '@sentry/core';
import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';
import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';
import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';
Expand Down Expand Up @@ -347,19 +347,10 @@ export function applyDebugMetadata(resource_paths: ReadonlyArray<string>): Debug
return [];
}

const hub = getCurrentHub();
if (!hub) {
return [];
}
const client = hub.getClient();
if (!client) {
return [];
}
const options = client.getOptions();
if (!options) {
return [];
}
const stackParser = options.stackParser;
const client = getClient();
const options = client && client.getOptions();
const stackParser = options && options.stackParser;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored this a bit to be a bit more straightforward IMHO.


if (!stackParser) {
return [];
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import {
import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
import { DEBUG_BUILD } from './debug-build';
import { createEventEnvelope, createSessionEnvelope } from './envelope';
import { getCurrentHub } from './hub';
import { getClient } from './exports';
import type { IntegrationIndex } from './integration';
import { setupIntegration, setupIntegrations } from './integration';
import { createMetricEnvelope } from './metrics/envelope';
Expand Down Expand Up @@ -870,7 +870,7 @@ function isTransactionEvent(event: Event): event is TransactionEvent {
* This event processor will run for all events processed by this client.
*/
export function addEventProcessor(callback: EventProcessor): void {
const client = getCurrentHub().getClient();
const client = getClient();

if (!client || !client.addEventProcessor) {
return;
Expand Down
5 changes: 2 additions & 3 deletions packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,8 @@ export function startTransaction(
* to create a monitor automatically when sending a check in.
*/
export function captureCheckIn(checkIn: CheckIn, upsertMonitorConfig?: MonitorConfig): string {
const hub = getCurrentHub();
const scope = hub.getScope();
const client = hub.getClient();
const scope = getCurrentScope();
const client = getClient();
if (!client) {
DEBUG_BUILD && logger.warn('Cannot capture check-in. No client defined.');
} else if (!client.captureCheckIn) {
Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/metrics/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { ClientOptions, MeasurementUnit, Primitive } from '@sentry/types';
import { logger } from '@sentry/utils';
import type { BaseClient } from '../baseclient';
import { DEBUG_BUILD } from '../debug-build';
import { getCurrentHub } from '../hub';
import { getClient, getCurrentScope } from '../exports';
import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants';
import { MetricsAggregator } from './integration';
import type { MetricType } from './types';
Expand All @@ -19,9 +19,8 @@ function addToMetricsAggregator(
value: number | string,
data: MetricData = {},
): void {
const hub = getCurrentHub();
const client = hub.getClient() as BaseClient<ClientOptions>;
const scope = hub.getScope();
const client = getClient<BaseClient<ClientOptions>>();
const scope = getCurrentScope();
if (client) {
if (!client.metricsAggregator) {
DEBUG_BUILD &&
Expand Down
5 changes: 2 additions & 3 deletions packages/core/src/sessionflusher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import type {
SessionFlusherLike,
} from '@sentry/types';
import { dropUndefinedKeys } from '@sentry/utils';

import { getCurrentHub } from './hub';
import { getCurrentScope } from './exports';

type ReleaseHealthAttributes = {
environment?: string;
Expand Down Expand Up @@ -75,7 +74,7 @@ export class SessionFlusher implements SessionFlusherLike {
if (!this._isEnabled) {
return;
}
const scope = getCurrentHub().getScope();
const scope = getCurrentScope();
const requestSession = scope.getRequestSession();

if (requestSession && requestSession.status) {
Expand Down
Loading