Skip to content

ref(core): Replace Scope.clone() with non-static scope.clone() #9801

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 2 commits into from
Dec 12, 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
4 changes: 2 additions & 2 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class Hub implements HubInterface {
*/
public pushScope(): Scope {
// We want to clone the content of prev scope
const scope = Scope.clone(this.getScope());
const scope = this.getScope().clone();
this.getStack().push({
client: this.getClient(),
scope,
Expand Down Expand Up @@ -578,7 +578,7 @@ export function ensureHubOnCarrier(carrier: Carrier, parent: Hub = getGlobalHub(
// If there's no hub on current domain, or it's an old API, assign a new one
if (!hasHubOnCarrier(carrier) || getHubFromCarrier(carrier).isOlderThan(API_VERSION)) {
const globalHubTopStack = parent.getStackTop();
setHubOnCarrier(carrier, new Hub(globalHubTopStack.client, Scope.clone(globalHubTopStack.scope)));
setHubOnCarrier(carrier, new Hub(globalHubTopStack.client, globalHubTopStack.scope.clone()));
}
}

Expand Down
42 changes: 24 additions & 18 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,27 +110,33 @@ export class Scope implements ScopeInterface {

/**
* Inherit values from the parent scope.
* @param scope to clone.
* @deprecated Use `scope.clone()` and `new Scope()` instead.
*/
public static clone(scope?: Scope): Scope {
return scope ? scope.clone() : new Scope();
}

/**
* Clone this scope instance.
*/
public clone(): Scope {
const newScope = new Scope();
if (scope) {
newScope._breadcrumbs = [...scope._breadcrumbs];
newScope._tags = { ...scope._tags };
newScope._extra = { ...scope._extra };
newScope._contexts = { ...scope._contexts };
newScope._user = scope._user;
newScope._level = scope._level;
newScope._span = scope._span;
newScope._session = scope._session;
newScope._transactionName = scope._transactionName;
newScope._fingerprint = scope._fingerprint;
newScope._eventProcessors = [...scope._eventProcessors];
newScope._requestSession = scope._requestSession;
newScope._attachments = [...scope._attachments];
newScope._sdkProcessingMetadata = { ...scope._sdkProcessingMetadata };
newScope._propagationContext = { ...scope._propagationContext };
}
newScope._breadcrumbs = [...this._breadcrumbs];
newScope._tags = { ...this._tags };
newScope._extra = { ...this._extra };
newScope._contexts = { ...this._contexts };
newScope._user = this._user;
newScope._level = this._level;
newScope._span = this._span;
newScope._session = this._session;
newScope._transactionName = this._transactionName;
newScope._fingerprint = this._fingerprint;
newScope._eventProcessors = [...this._eventProcessors];
newScope._requestSession = this._requestSession;
newScope._attachments = [...this._attachments];
newScope._sdkProcessingMetadata = { ...this._sdkProcessingMetadata };
newScope._propagationContext = { ...this._propagationContext };

return newScope;
}

Expand Down
15 changes: 11 additions & 4 deletions packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,7 @@ export function prepareEvent(

// If we have scope given to us, use it as the base for further modifications.
// This allows us to prevent unnecessary copying of data if `captureContext` is not provided.
let finalScope = scope;
if (hint.captureContext) {
finalScope = Scope.clone(finalScope).update(hint.captureContext);
}
const finalScope = getFinalScope(scope, hint.captureContext);

if (hint.mechanism) {
addExceptionMechanism(prepared, hint.mechanism);
Expand Down Expand Up @@ -349,6 +346,16 @@ function normalizeEvent(event: Event | null, depth: number, maxBreadth: number):
return normalized;
}

function getFinalScope(scope: Scope | undefined, captureContext: CaptureContext | undefined): Scope | undefined {
if (!captureContext) {
return scope;
}

const finalScope = scope ? scope.clone() : new Scope();
finalScope.update(captureContext);
return finalScope;
}

/**
* Parse either an `EventHint` directly, or convert a `CaptureContext` to an `EventHint`.
* This is used to allow to update method signatures that used to accept a `CaptureContext` but should now accept an `EventHint`.
Expand Down
12 changes: 2 additions & 10 deletions packages/opentelemetry-node/test/spanprocessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,7 @@ import type { Span as OtelSpan } from '@opentelemetry/sdk-trace-base';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import { SemanticAttributes, SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
import type { SpanStatusType } from '@sentry/core';
import {
Hub,
Scope,
Span as SentrySpan,
Transaction,
addTracingExtensions,
createTransport,
makeMain,
} from '@sentry/core';
import { Hub, Span as SentrySpan, Transaction, addTracingExtensions, createTransport, makeMain } from '@sentry/core';
import { NodeClient } from '@sentry/node';
import { resolvedSyncPromise } from '@sentry/utils';

Expand Down Expand Up @@ -973,7 +965,7 @@ describe('SentrySpanProcessor', () => {
hub = new Hub(client);
makeMain(hub);

const newHub = new Hub(client, Scope.clone(hub.getScope()));
const newHub = new Hub(client, hub.getScope().clone());
newHub.configureScope(scope => {
scope.setTag('foo', 'bar');
});
Expand Down
3 changes: 2 additions & 1 deletion packages/opentelemetry/src/contextManager.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { Context, ContextManager } from '@opentelemetry/api';
import type { Carrier, Hub } from '@sentry/core';
import { ensureHubOnCarrier } from '@sentry/core';

import { ensureHubOnCarrier, getCurrentHub, getHubFromCarrier } from './custom/hub';
import { getCurrentHub, getHubFromCarrier } from './custom/hub';
import { setHubOnContext } from './utils/contextData';

function createNewHub(parent: Hub | undefined): Hub {
Expand Down
12 changes: 9 additions & 3 deletions packages/opentelemetry/src/custom/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { trace } from '@opentelemetry/api';
import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
import type { BaseClient, Scope } from '@sentry/core';
import { SDK_VERSION } from '@sentry/core';
import type { Client, Event, EventHint } from '@sentry/types';
import type { CaptureContext, Client, Event, EventHint } from '@sentry/types';

import type { OpenTelemetryClient as OpenTelemetryClientInterface } from '../types';
import { OpenTelemetryScope } from './scope';
Expand Down Expand Up @@ -65,14 +65,14 @@ export function wrapClientClass<

/**
* Extends the base `_prepareEvent` so that we can properly handle `captureContext`.
* This uses `Scope.clone()`, which we need to replace with `NodeExperimentalScope.clone()` for this client.
* This uses `Scope.clone()`, which we need to replace with `OpenTelemetryScope.clone()` for this client.
*/
protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event | null> {
let actualScope = scope;

// Remove `captureContext` hint and instead clone already here
if (hint && hint.captureContext) {
actualScope = OpenTelemetryScope.clone(scope);
actualScope = getFinalScope(scope, hint.captureContext);
delete hint.captureContext;
}

Expand All @@ -83,3 +83,9 @@ export function wrapClientClass<
return OpenTelemetryClient as unknown as WrappedClassConstructor;
}
/* eslint-enable @typescript-eslint/no-explicit-any */

function getFinalScope(scope: Scope | undefined, captureContext: CaptureContext): Scope | undefined {
const finalScope = scope ? scope.clone() : new OpenTelemetryScope();
finalScope.update(captureContext);
return finalScope;
}
18 changes: 1 addition & 17 deletions packages/opentelemetry/src/custom/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,6 @@ export class OpenTelemetryHub extends Hub {
public constructor(client?: Client, scope: Scope = new OpenTelemetryScope()) {
super(client, scope);
}

/**
* @inheritDoc
*/
public pushScope(): Scope {
// We want to clone the content of prev scope
const scope = OpenTelemetryScope.clone(this.getScope());
this.getStack().push({
client: this.getClient(),
scope,
});
return scope;
}
}

/** Custom getClient method that uses the custom hub. */
Expand Down Expand Up @@ -110,10 +97,7 @@ export function ensureHubOnCarrier(carrier: Carrier, parent: Hub = getGlobalHub(
// If there's no hub on current domain, or it's an old API, assign a new one
if (!hasHubOnCarrier(carrier) || getHubFromCarrier(carrier).isOlderThan(API_VERSION)) {
const globalHubTopStack = parent.getStackTop();
setHubOnCarrier(
carrier,
new OpenTelemetryHub(globalHubTopStack.client, OpenTelemetryScope.clone(globalHubTopStack.scope)),
);
setHubOnCarrier(carrier, new OpenTelemetryHub(globalHubTopStack.client, globalHubTopStack.scope.clone()));
}
}

Expand Down
40 changes: 23 additions & 17 deletions packages/opentelemetry/src/custom/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,30 @@ export class OpenTelemetryScope extends Scope {
* @inheritDoc
*/
public static clone(scope?: Scope): Scope {
return scope ? scope.clone() : new OpenTelemetryScope();
}

/**
* Clone this scope instance.
*/
public clone(): OpenTelemetryScope {
const newScope = new OpenTelemetryScope();
if (scope) {
newScope._breadcrumbs = [...scope['_breadcrumbs']];
newScope._tags = { ...scope['_tags'] };
newScope._extra = { ...scope['_extra'] };
newScope._contexts = { ...scope['_contexts'] };
newScope._user = scope['_user'];
newScope._level = scope['_level'];
newScope._span = scope['_span'];
newScope._session = scope['_session'];
newScope._transactionName = scope['_transactionName'];
newScope._fingerprint = scope['_fingerprint'];
newScope._eventProcessors = [...scope['_eventProcessors']];
newScope._requestSession = scope['_requestSession'];
newScope._attachments = [...scope['_attachments']];
newScope._sdkProcessingMetadata = { ...scope['_sdkProcessingMetadata'] };
newScope._propagationContext = { ...scope['_propagationContext'] };
}
newScope._breadcrumbs = [...this['_breadcrumbs']];
newScope._tags = { ...this['_tags'] };
newScope._extra = { ...this['_extra'] };
newScope._contexts = { ...this['_contexts'] };
newScope._user = this['_user'];
newScope._level = this['_level'];
newScope._span = this['_span'];
newScope._session = this['_session'];
newScope._transactionName = this['_transactionName'];
newScope._fingerprint = this['_fingerprint'];
newScope._eventProcessors = [...this['_eventProcessors']];
newScope._requestSession = this['_requestSession'];
newScope._attachments = [...this['_attachments']];
newScope._sdkProcessingMetadata = { ...this['_sdkProcessingMetadata'] };
newScope._propagationContext = { ...this['_propagationContext'] };

return newScope;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry/src/spanExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ function maybeSend(spans: ReadableSpan[]): ReadableSpan[] {

// Now finish the transaction, which will send it together with all the spans
// We make sure to use the current span as the activeSpan for this transaction
const scope = getSpanScope(span);
const forkedScope = OpenTelemetryScope.clone(scope as OpenTelemetryScope | undefined) as OpenTelemetryScope;
const scope = getSpanScope(span) as OpenTelemetryScope | undefined;
const forkedScope = scope ? scope.clone() : new OpenTelemetryScope();
forkedScope.activeSpan = span as unknown as Span;

transaction.finishWithScope(convertOtelTimeToSeconds(span.endTime), forkedScope);
Expand Down
2 changes: 2 additions & 0 deletions packages/opentelemetry/test/custom/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('NodeExperimentalScope', () => {
scope['_attachments'] = [{ data: '123', filename: 'test.txt' }];
scope['_sdkProcessingMetadata'] = { sdk: 'bar' };

// eslint-disable-next-line deprecation/deprecation
const scope2 = OpenTelemetryScope.clone(scope);

expect(scope2).toBeInstanceOf(OpenTelemetryScope);
Expand Down Expand Up @@ -68,6 +69,7 @@ describe('NodeExperimentalScope', () => {
});

it('clone() works without existing scope', () => {
// eslint-disable-next-line deprecation/deprecation
const scope = OpenTelemetryScope.clone(undefined);

expect(scope).toBeInstanceOf(OpenTelemetryScope);
Expand Down