Skip to content

feat(opentelemetry): Remove hub from context #10983

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
Mar 11, 2024
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
1 change: 0 additions & 1 deletion packages/node-experimental/src/sdk/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ function getCjsOnlyIntegrations(isCjs = typeof require !== 'undefined'): Integra

/** Get the default integrations for the Node Experimental SDK. */
export function getDefaultIntegrations(options: Options): Integration[] {
// TODO
return [
// Common
inboundFiltersIntegration(),
Expand Down
3 changes: 0 additions & 3 deletions packages/opentelemetry/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ export const SENTRY_BAGGAGE_HEADER = 'baggage';
export const SENTRY_TRACE_STATE_DSC = 'sentry.trace';
export const SENTRY_TRACE_STATE_PARENT_SPAN_ID = 'sentry.parent_span_id';

/** Context Key to hold a Hub. */
export const SENTRY_HUB_CONTEXT_KEY = createContextKey('sentry_hub');

export const SENTRY_SCOPES_CONTEXT_KEY = createContextKey('sentry_scopes');

export const SENTRY_FORK_ISOLATION_SCOPE_CONTEXT_KEY = createContextKey('sentry_fork_isolation_scope');
Expand Down
19 changes: 5 additions & 14 deletions packages/opentelemetry/src/contextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import {
SENTRY_FORK_SET_ISOLATION_SCOPE_CONTEXT_KEY,
SENTRY_FORK_SET_SCOPE_CONTEXT_KEY,
} from './constants';
import { getCurrentHub } from './custom/getCurrentHub';
import { getScopesFromContext, setContextOnScope, setHubOnContext, setScopesOnContext } from './utils/contextData';
import { getScopesFromContext, setContextOnScope, setScopesOnContext } from './utils/contextData';
import { setIsSetup } from './utils/setupCheck';

/**
Expand Down Expand Up @@ -59,25 +58,17 @@ export function wrapContextManagerClass<ContextManagerInstance extends ContextMa
isolationScope || (shouldForkIsolationScope ? currentIsolationScope.clone() : currentIsolationScope);
const scopes = { scope: newCurrentScope, isolationScope: newIsolationScope };

const mockHub = {
// eslint-disable-next-line deprecation/deprecation
...getCurrentHub(),
getScope: () => newCurrentScope,
getIsolationScope: () => newIsolationScope,
};

const ctx1 = setHubOnContext(context, mockHub);
const ctx2 = setScopesOnContext(ctx1, scopes);
const ctx1 = setScopesOnContext(context, scopes);

// Remove the unneeded values again
const ctx3 = ctx2
const ctx2 = ctx1
.deleteValue(SENTRY_FORK_ISOLATION_SCOPE_CONTEXT_KEY)
.deleteValue(SENTRY_FORK_SET_SCOPE_CONTEXT_KEY)
.deleteValue(SENTRY_FORK_SET_ISOLATION_SCOPE_CONTEXT_KEY);

setContextOnScope(newCurrentScope, ctx3);
setContextOnScope(newCurrentScope, ctx2);

return super.with(ctx3, fn, thisArg, ...args);
return super.with(ctx2, fn, thisArg, ...args);
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export { wrapClientClass } from './custom/client';

export { getSpanKind } from './utils/getSpanKind';
export {
getSpanHub,
getSpanMetadata,
getSpanParent,
setSpanMetadata,
Expand Down
33 changes: 14 additions & 19 deletions packages/opentelemetry/src/spanProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,39 @@ import type { Context } from '@opentelemetry/api';
import { ROOT_CONTEXT, trace } from '@opentelemetry/api';
import type { Span, SpanProcessor as SpanProcessorInterface } from '@opentelemetry/sdk-trace-base';
import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-base';
import { getClient, getCurrentHub } from '@sentry/core';
import { getClient, getDefaultCurrentScope, getDefaultIsolationScope } from '@sentry/core';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from './debug-build';
import { SentrySpanExporter } from './spanExporter';
import { maybeCaptureExceptionForTimedEvent } from './utils/captureExceptionForTimedEvent';
import { getHubFromContext } from './utils/contextData';
import { getScopesFromContext } from './utils/contextData';
import { setIsSetup } from './utils/setupCheck';
import { getSpanHub, setSpanHub, setSpanParent, setSpanScopes } from './utils/spanData';
import { setSpanParent, setSpanScopes } from './utils/spanData';

function onSpanStart(span: Span, parentContext: Context): void {
// This is a reliable way to get the parent span - because this is exactly how the parent is identified in the OTEL SDK
const parentSpan = trace.getSpan(parentContext);
const hub = getHubFromContext(parentContext);

let scopes = getScopesFromContext(parentContext);

// We need access to the parent span in order to be able to move up the span tree for breadcrumbs
if (parentSpan) {
setSpanParent(span, parentSpan);
}

// The root context does not have a hub stored, so we check for this specifically
// We do this instead of just falling back to `getCurrentHub` to avoid attaching the wrong hub
let actualHub = hub;
// The root context does not have scopes stored, so we check for this specifically
// As fallback we attach the global scopes
if (parentContext === ROOT_CONTEXT) {
// eslint-disable-next-line deprecation/deprecation
actualHub = getCurrentHub();
scopes = {
scope: getDefaultCurrentScope(),
isolationScope: getDefaultIsolationScope(),
};
}

// We need the scope at time of span creation in order to apply it to the event when the span is finished
if (actualHub) {
// eslint-disable-next-line deprecation/deprecation
const scope = actualHub.getScope();
// eslint-disable-next-line deprecation/deprecation
const isolationScope = actualHub.getIsolationScope();
setSpanHub(span, actualHub);
setSpanScopes(span, { scope, isolationScope });
if (scopes) {
setSpanScopes(span, scopes);
}

const client = getClient();
Expand All @@ -46,10 +43,8 @@ function onSpanStart(span: Span, parentContext: Context): void {

function onSpanEnd(span: Span): void {
// Capture exceptions as events
// eslint-disable-next-line deprecation/deprecation
const hub = getSpanHub(span) || getCurrentHub();
span.events.forEach(event => {
maybeCaptureExceptionForTimedEvent(hub, event, span);
maybeCaptureExceptionForTimedEvent(event, span);
});

const client = getClient();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import type { Span as OtelSpan, TimedEvent } from '@opentelemetry/sdk-trace-base';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import type { Hub } from '@sentry/types';
import { captureException } from '@sentry/core';
import { isString } from '@sentry/utils';

/**
* Maybe capture a Sentry exception for an OTEL timed event.
* This will check if the event is exception-like and in that case capture it as an exception.
*/
export function maybeCaptureExceptionForTimedEvent(hub: Hub, event: TimedEvent, otelSpan?: OtelSpan): void {
export function maybeCaptureExceptionForTimedEvent(event: TimedEvent, otelSpan?: OtelSpan): void {
if (event.name !== 'exception') {
return;
}
Expand Down Expand Up @@ -35,8 +35,7 @@ export function maybeCaptureExceptionForTimedEvent(hub: Hub, event: TimedEvent,
syntheticError.name = type;
}

// eslint-disable-next-line deprecation/deprecation
hub.captureException(syntheticError, {
captureException(syntheticError, {
captureContext: otelSpan
? {
contexts: {
Expand Down
20 changes: 2 additions & 18 deletions packages/opentelemetry/src/utils/contextData.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,11 @@
import type { Context } from '@opentelemetry/api';
import type { Hub, Scope } from '@sentry/types';
import type { Scope } from '@sentry/types';

import { SENTRY_HUB_CONTEXT_KEY, SENTRY_SCOPES_CONTEXT_KEY } from '../constants';
import { SENTRY_SCOPES_CONTEXT_KEY } from '../constants';
import type { CurrentScopes } from '../types';

const SCOPE_CONTEXT_MAP = new WeakMap<Scope, Context>();

/**
* Try to get the Hub from the given OTEL context.
* This requires a Context Manager that was wrapped with getWrappedContextManager.
*/
export function getHubFromContext(context: Context): Hub | undefined {
return context.getValue(SENTRY_HUB_CONTEXT_KEY) as Hub | undefined;
}

/**
* Set a Hub on an OTEL context..
* This will return a forked context with the Propagation Context set.
*/
export function setHubOnContext(context: Context, hub: Hub): Context {
return context.setValue(SENTRY_HUB_CONTEXT_KEY, hub);
}

/**
* Try to get the current scopes from the given OTEL context.
* This requires a Context Manager that was wrapped with getWrappedContextManager.
Expand Down
13 changes: 1 addition & 12 deletions packages/opentelemetry/src/utils/spanData.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Span } from '@opentelemetry/api';
import type { Hub, Scope, TransactionMetadata } from '@sentry/types';
import type { Scope, TransactionMetadata } from '@sentry/types';

import type { AbstractSpan } from '../types';

Expand All @@ -13,20 +13,9 @@ const SpanScopes = new WeakMap<
isolationScope: Scope;
}
>();
const SpanHub = new WeakMap<AbstractSpan, Hub>();
const SpanParent = new WeakMap<AbstractSpan, Span>();
const SpanMetadata = new WeakMap<AbstractSpan, Partial<TransactionMetadata>>();

/** Set the Sentry hub on an OTEL span. */
export function setSpanHub(span: AbstractSpan, hub: Hub): void {
SpanHub.set(span, hub);
}

/** Get the Sentry hub of an OTEL span. */
export function getSpanHub(span: AbstractSpan): Hub | undefined {
return SpanHub.get(span);
}

/** Set the parent OTEL span on an OTEL span. */
export function setSpanParent(span: AbstractSpan, parentSpan: Span): void {
SpanParent.set(span, parentSpan);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import type { Span as OtelSpan, TimedEvent } from '@opentelemetry/sdk-trace-base';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import type { Hub } from '@sentry/types';
import * as SentryCore from '@sentry/core';

import { maybeCaptureExceptionForTimedEvent } from '../../src/utils/captureExceptionForTimedEvent';

describe('maybeCaptureExceptionForTimedEvent', () => {
afterEach(() => {
jest.clearAllMocks();
});

it('ignores non-exception events', async () => {
const event: TimedEvent = {
time: [12345, 0],
name: 'test event',
};

const captureException = jest.fn();
const hub = {
captureException,
} as unknown as Hub;

maybeCaptureExceptionForTimedEvent(hub, event);
const captureException = jest.spyOn(SentryCore, 'captureException');
maybeCaptureExceptionForTimedEvent(event);

expect(captureException).not.toHaveBeenCalled();
});
Expand All @@ -27,12 +27,8 @@ describe('maybeCaptureExceptionForTimedEvent', () => {
name: 'exception',
};

const captureException = jest.fn();
const hub = {
captureException,
} as unknown as Hub;

maybeCaptureExceptionForTimedEvent(hub, event);
const captureException = jest.spyOn(SentryCore, 'captureException');
maybeCaptureExceptionForTimedEvent(event);

expect(captureException).not.toHaveBeenCalled();
});
Expand All @@ -46,12 +42,8 @@ describe('maybeCaptureExceptionForTimedEvent', () => {
},
};

const captureException = jest.fn();
const hub = {
captureException,
} as unknown as Hub;

maybeCaptureExceptionForTimedEvent(hub, event);
const captureException = jest.spyOn(SentryCore, 'captureException');
maybeCaptureExceptionForTimedEvent(event);

expect(captureException).toHaveBeenCalledTimes(1);
expect(captureException).toHaveBeenCalledWith(expect.objectContaining({ message: 'test-message' }), {
Expand All @@ -73,12 +65,8 @@ describe('maybeCaptureExceptionForTimedEvent', () => {
},
};

const captureException = jest.fn();
const hub = {
captureException,
} as unknown as Hub;

maybeCaptureExceptionForTimedEvent(hub, event);
const captureException = jest.spyOn(SentryCore, 'captureException');
maybeCaptureExceptionForTimedEvent(event);

expect(captureException).toHaveBeenCalledTimes(1);
expect(captureException).toHaveBeenCalledWith(
Expand Down Expand Up @@ -116,12 +104,8 @@ describe('maybeCaptureExceptionForTimedEvent', () => {
},
} as unknown as OtelSpan;

const captureException = jest.fn();
const hub = {
captureException,
} as unknown as Hub;

maybeCaptureExceptionForTimedEvent(hub, event, span);
const captureException = jest.spyOn(SentryCore, 'captureException');
maybeCaptureExceptionForTimedEvent(event, span);

expect(captureException).toHaveBeenCalledTimes(1);
expect(captureException).toHaveBeenCalledWith(expect.objectContaining({ message: 'test-message' }), {
Expand Down