From 44aab889dee7413f68366d8b9363c8ef81eb9b66 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 11 Aug 2023 10:42:16 +0200 Subject: [PATCH 1/3] feat(node-experimental): Provide otel-powered async context strategy --- .../node-experimental/src/sdk/initOtel.ts | 3 ++ .../sdk/otelContextAsyncContextStrategy.ts | 43 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts diff --git a/packages/node-experimental/src/sdk/initOtel.ts b/packages/node-experimental/src/sdk/initOtel.ts index cd2b27b79c05..782281f928dc 100644 --- a/packages/node-experimental/src/sdk/initOtel.ts +++ b/packages/node-experimental/src/sdk/initOtel.ts @@ -4,6 +4,7 @@ import { getCurrentHub } from '@sentry/core'; import { SentryPropagator, SentrySpanProcessor } from '@sentry/opentelemetry-node'; import type { NodeExperimentalClient } from './client'; +import { setOtelContextAsyncContextStrategy } from './otelContextAsyncContextStrategy'; /** * Initialize OpenTelemetry for Node. @@ -27,6 +28,8 @@ export function initOtel(): () => void { propagator: new SentryPropagator(), }); + setOtelContextAsyncContextStrategy(); + // Cleanup function return () => { void provider.forceFlush(); diff --git a/packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts b/packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts new file mode 100644 index 000000000000..1718943d7288 --- /dev/null +++ b/packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts @@ -0,0 +1,43 @@ +import * as api from '@opentelemetry/api'; +import type { Carrier, Hub, RunWithAsyncContextOptions } from '@sentry/core'; +import { ensureHubOnCarrier, getHubFromCarrier, setAsyncContextStrategy } from '@sentry/core'; + +const hubKey = api.createContextKey('sentry_hub'); + +/** + * Sets the async context strategy to use follow the OTEL context under the hood. + */ +export function setOtelContextAsyncContextStrategy(): void { + function getCurrentHub(): Hub | undefined { + const ctx = api.context.active(); + + // Returning undefined means the global hub will be used + return ctx.getValue(hubKey) as Hub | undefined; + } + + function createNewHub(parent: Hub | undefined): Hub { + const carrier: Carrier = {}; + ensureHubOnCarrier(carrier, parent); + return getHubFromCarrier(carrier); + } + + function runWithAsyncContext(callback: () => T, options: RunWithAsyncContextOptions): T { + const existingHub = getCurrentHub(); + + if (existingHub && options?.reuseExisting) { + // We're already in an async context, so we don't need to create a new one + // just call the callback with the current hub + return callback(); + } + + const newHub = createNewHub(existingHub); + + const ctx = api.context.active(); + + return api.context.with(ctx.setValue(hubKey, newHub), () => { + return callback(); + }); + } + + setAsyncContextStrategy({ getCurrentHub, runWithAsyncContext }); +} From 8a5da44fae6551439cde5d68238025b21ad99dae Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 30 Aug 2023 09:56:23 +0200 Subject: [PATCH 2/3] Add otel contextManager & use it for hub management --- packages/node-experimental/package.json | 1 + packages/node-experimental/src/sdk/init.ts | 2 ++ .../node-experimental/src/sdk/initOtel.ts | 9 ++++-- ...trategy.ts => otelAsyncContextStrategy.ts} | 20 ++++-------- .../src/sdk/otelContextManager.ts | 32 +++++++++++++++++++ yarn.lock | 5 +++ 6 files changed, 53 insertions(+), 16 deletions(-) rename packages/node-experimental/src/sdk/{otelContextAsyncContextStrategy.ts => otelAsyncContextStrategy.ts} (60%) create mode 100644 packages/node-experimental/src/sdk/otelContextManager.ts diff --git a/packages/node-experimental/package.json b/packages/node-experimental/package.json index 24c42b08d607..a20d2d72d69f 100644 --- a/packages/node-experimental/package.json +++ b/packages/node-experimental/package.json @@ -37,6 +37,7 @@ "@opentelemetry/instrumentation-pg": "~0.36.0", "@opentelemetry/sdk-trace-node": "~1.15.0", "@opentelemetry/semantic-conventions": "~1.15.0", + "@opentelemetry/context-async-hooks": "~1.15.0", "@prisma/instrumentation": "~5.0.0", "@sentry/core": "7.66.0", "@sentry/node": "7.66.0", diff --git a/packages/node-experimental/src/sdk/init.ts b/packages/node-experimental/src/sdk/init.ts index ef451bb83c3a..070728367925 100644 --- a/packages/node-experimental/src/sdk/init.ts +++ b/packages/node-experimental/src/sdk/init.ts @@ -6,6 +6,7 @@ import { Http } from '../integrations/http'; import type { NodeExperimentalOptions } from '../types'; import { NodeExperimentalClient } from './client'; import { initOtel } from './initOtel'; +import { setOtelContextAsyncContextStrategy } from './otelAsyncContextStrategy'; const ignoredDefaultIntegrations = ['Http', 'Undici']; @@ -35,4 +36,5 @@ export function init(options: NodeExperimentalOptions | undefined = {}): void { // Always init Otel, even if tracing is disabled, because we need it for trace propagation & the HTTP integration initOtel(); + setOtelContextAsyncContextStrategy(); } diff --git a/packages/node-experimental/src/sdk/initOtel.ts b/packages/node-experimental/src/sdk/initOtel.ts index 782281f928dc..92cf794c8b29 100644 --- a/packages/node-experimental/src/sdk/initOtel.ts +++ b/packages/node-experimental/src/sdk/initOtel.ts @@ -4,7 +4,7 @@ import { getCurrentHub } from '@sentry/core'; import { SentryPropagator, SentrySpanProcessor } from '@sentry/opentelemetry-node'; import type { NodeExperimentalClient } from './client'; -import { setOtelContextAsyncContextStrategy } from './otelContextAsyncContextStrategy'; +import { SentryContextManager } from './otelContextManager'; /** * Initialize OpenTelemetry for Node. @@ -23,13 +23,16 @@ export function initOtel(): () => void { }); provider.addSpanProcessor(new SentrySpanProcessor()); + // We use a custom context manager to keep context in sync with sentry scope + const contextManager = new SentryContextManager(); + contextManager.enable(); + // Initialize the provider provider.register({ propagator: new SentryPropagator(), + contextManager, }); - setOtelContextAsyncContextStrategy(); - // Cleanup function return () => { void provider.forceFlush(); diff --git a/packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts b/packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts similarity index 60% rename from packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts rename to packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts index 1718943d7288..f0935f326607 100644 --- a/packages/node-experimental/src/sdk/otelContextAsyncContextStrategy.ts +++ b/packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts @@ -1,8 +1,8 @@ import * as api from '@opentelemetry/api'; -import type { Carrier, Hub, RunWithAsyncContextOptions } from '@sentry/core'; -import { ensureHubOnCarrier, getHubFromCarrier, setAsyncContextStrategy } from '@sentry/core'; +import type { Hub, RunWithAsyncContextOptions } from '@sentry/core'; +import { setAsyncContextStrategy } from '@sentry/core'; -const hubKey = api.createContextKey('sentry_hub'); +import { OTEL_CONTEXT_HUB_KEY } from './otelContextManager'; /** * Sets the async context strategy to use follow the OTEL context under the hood. @@ -12,15 +12,10 @@ export function setOtelContextAsyncContextStrategy(): void { const ctx = api.context.active(); // Returning undefined means the global hub will be used - return ctx.getValue(hubKey) as Hub | undefined; - } - - function createNewHub(parent: Hub | undefined): Hub { - const carrier: Carrier = {}; - ensureHubOnCarrier(carrier, parent); - return getHubFromCarrier(carrier); + return ctx.getValue(OTEL_CONTEXT_HUB_KEY) as Hub | undefined; } + /* This is more or less a NOOP - we rely on the OTEL context manager for this */ function runWithAsyncContext(callback: () => T, options: RunWithAsyncContextOptions): T { const existingHub = getCurrentHub(); @@ -30,11 +25,10 @@ export function setOtelContextAsyncContextStrategy(): void { return callback(); } - const newHub = createNewHub(existingHub); - const ctx = api.context.active(); - return api.context.with(ctx.setValue(hubKey, newHub), () => { + // We depend on the otelContextManager to handle the context/hub + return api.context.with(ctx, () => { return callback(); }); } diff --git a/packages/node-experimental/src/sdk/otelContextManager.ts b/packages/node-experimental/src/sdk/otelContextManager.ts new file mode 100644 index 000000000000..7f3f642c2fa6 --- /dev/null +++ b/packages/node-experimental/src/sdk/otelContextManager.ts @@ -0,0 +1,32 @@ +import type { Context } from '@opentelemetry/api'; +import * as api from '@opentelemetry/api'; +import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks'; +import type { Carrier, Hub } from '@sentry/core'; +import { ensureHubOnCarrier, getCurrentHub, getHubFromCarrier } from '@sentry/core'; + +export const OTEL_CONTEXT_HUB_KEY = api.createContextKey('sentry_hub'); + +function createNewHub(parent: Hub | undefined): Hub { + const carrier: Carrier = {}; + ensureHubOnCarrier(carrier, parent); + return getHubFromCarrier(carrier); +} + +/** TODO docs */ +export class SentryContextManager extends AsyncLocalStorageContextManager { + /** + * Overwrite with() of the original AsyncLocalStorageContextManager + * to ensure we also create a new hub per context. + */ + public with ReturnType>( + context: Context, + fn: F, + thisArg?: ThisParameterType, + ...args: A + ): ReturnType { + const existingHub = getCurrentHub(); + const newHub = createNewHub(existingHub); + + return super.with(context.setValue(OTEL_CONTEXT_HUB_KEY, newHub), fn, thisArg, ...args); + } +} diff --git a/yarn.lock b/yarn.lock index 40f7682f22b7..ec2d72d684e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3733,6 +3733,11 @@ dependencies: tslib "^2.3.1" +"@opentelemetry/context-async-hooks@~1.15.0": + version "1.15.2" + resolved "https://registry.yarnpkg.com/@opentelemetry/context-async-hooks/-/context-async-hooks-1.15.2.tgz#116bd5fef231137198d5bf551e8c0521fbdfe928" + integrity sha512-VAMHG67srGFQDG/N2ns5AyUT9vUcoKpZ/NpJ5fDQIPfJd7t3ju+aHwvDsMcrYBWuCh03U3Ky6o16+872CZchBg== + "@opentelemetry/context-base@^0.12.0": version "0.12.0" resolved "https://registry.yarnpkg.com/@opentelemetry/context-base/-/context-base-0.12.0.tgz#4906ae27359d3311e3dea1b63770a16f60848550" From b111b9706684b7277abdbab704c02f4eb668e36c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 1 Sep 2023 10:19:51 +0200 Subject: [PATCH 3/3] better docs --- .../node-experimental/src/sdk/otelAsyncContextStrategy.ts | 1 + packages/node-experimental/src/sdk/otelContextManager.ts | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts b/packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts index f0935f326607..455dc4717422 100644 --- a/packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts +++ b/packages/node-experimental/src/sdk/otelAsyncContextStrategy.ts @@ -6,6 +6,7 @@ import { OTEL_CONTEXT_HUB_KEY } from './otelContextManager'; /** * Sets the async context strategy to use follow the OTEL context under the hood. + * We handle forking a hub inside of our custom OTEL Context Manager (./otelContextManager.ts) */ export function setOtelContextAsyncContextStrategy(): void { function getCurrentHub(): Hub | undefined { diff --git a/packages/node-experimental/src/sdk/otelContextManager.ts b/packages/node-experimental/src/sdk/otelContextManager.ts index 7f3f642c2fa6..9110b9e62328 100644 --- a/packages/node-experimental/src/sdk/otelContextManager.ts +++ b/packages/node-experimental/src/sdk/otelContextManager.ts @@ -12,7 +12,13 @@ function createNewHub(parent: Hub | undefined): Hub { return getHubFromCarrier(carrier); } -/** TODO docs */ +/** + * This is a custom ContextManager for OpenTelemetry, which extends the default AsyncLocalStorageContextManager. + * It ensures that we create a new hub per context, so that the OTEL Context & the Sentry Hub are always in sync. + * + * Note that we currently only support AsyncHooks with this, + * but since this should work for Node 14+ anyhow that should be good enough. + */ export class SentryContextManager extends AsyncLocalStorageContextManager { /** * Overwrite with() of the original AsyncLocalStorageContextManager