From 05d7ca6a5d5ad2b4a65f4fcb3f689283799c3f4c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Aug 2024 16:52:50 +0200 Subject: [PATCH 1/7] feat(core): Add OpenTelemetry-specific `getTraceData` version --- packages/core/src/asyncContext/types.ts | 4 ++ packages/core/src/index.ts | 1 + packages/core/src/utils/traceData.ts | 10 ++++- .../opentelemetry/src/asyncContextStrategy.ts | 6 ++- .../opentelemetry/src/utils/getTraceData.ts | 40 +++++++++++++++++++ 5 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 packages/opentelemetry/src/utils/getTraceData.ts diff --git a/packages/core/src/asyncContext/types.ts b/packages/core/src/asyncContext/types.ts index bd69c8e63e78..a99f9eaae567 100644 --- a/packages/core/src/asyncContext/types.ts +++ b/packages/core/src/asyncContext/types.ts @@ -1,4 +1,5 @@ import type { Scope } from '@sentry/types'; +import { getTraceData } from '../utils/traceData'; import type { startInactiveSpan, startSpan, @@ -64,4 +65,7 @@ export interface AsyncContextStrategy { /** Suppress tracing in the given callback, ensuring no spans are generated inside of it. */ suppressTracing?: typeof suppressTracing; + + /** get trace data as serialized string values for propagation via `sentry-trace` and `baggage` */ + getTraceData?: typeof getTraceData; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 73295f7df64c..79299f8092d6 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -5,6 +5,7 @@ export type { OfflineStore, OfflineTransportOptions } from './transports/offline export type { ServerRuntimeClientOptions } from './server-runtime-client'; export type { RequestDataIntegrationOptions } from './integrations/requestdata'; export type { IntegrationIndex } from './integration'; +export type { TraceData } from './utils/traceData'; export * from './tracing'; export * from './semanticAttributes'; diff --git a/packages/core/src/utils/traceData.ts b/packages/core/src/utils/traceData.ts index abc05f449365..854bdd41dd28 100644 --- a/packages/core/src/utils/traceData.ts +++ b/packages/core/src/utils/traceData.ts @@ -5,11 +5,13 @@ import { generateSentryTraceHeader, logger, } from '@sentry/utils'; +import { getAsyncContextStrategy } from '../asyncContext'; +import { getMainCarrier } from '../carrier'; import { getClient, getCurrentScope } from '../currentScopes'; import { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from '../tracing'; import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils'; -type TraceData = { +export type TraceData = { 'sentry-trace'?: string; baggage?: string; }; @@ -30,6 +32,12 @@ type TraceData = { * or meta tag name. */ export function getTraceData(span?: Span, scope?: Scope, client?: Client): TraceData { + const carrier = getMainCarrier(); + const acs = getAsyncContextStrategy(carrier); + if (acs.getTraceData) { + return acs.getTraceData(span, scope, client); + } + const clientToUse = client || getClient(); const scopeToUse = scope || getCurrentScope(); const spanToUse = span || getActiveSpan(); diff --git a/packages/opentelemetry/src/asyncContextStrategy.ts b/packages/opentelemetry/src/asyncContextStrategy.ts index 69878d27b252..7d81a76d5d80 100644 --- a/packages/opentelemetry/src/asyncContextStrategy.ts +++ b/packages/opentelemetry/src/asyncContextStrategy.ts @@ -1,6 +1,6 @@ import * as api from '@opentelemetry/api'; import { getDefaultCurrentScope, getDefaultIsolationScope, setAsyncContextStrategy } from '@sentry/core'; -import type { withActiveSpan as defaultWithActiveSpan } from '@sentry/core'; +import type { getTraceData as defaultGetTraceData, withActiveSpan as defaultWithActiveSpan } from '@sentry/core'; import type { Scope } from '@sentry/types'; import { @@ -12,6 +12,7 @@ import { startInactiveSpan, startSpan, startSpanManual, withActiveSpan } from '. import type { CurrentScopes } from './types'; import { getScopesFromContext } from './utils/contextData'; import { getActiveSpan } from './utils/getActiveSpan'; +import { getTraceData } from './utils/getTraceData'; import { suppressTracing } from './utils/suppressTracing'; /** @@ -102,9 +103,10 @@ export function setOpenTelemetryContextAsyncContextStrategy(): void { startSpanManual, startInactiveSpan, getActiveSpan, + suppressTracing, // The types here don't fully align, because our own `Span` type is narrower // than the OTEL one - but this is OK for here, as we now we'll only have OTEL spans passed around withActiveSpan: withActiveSpan as typeof defaultWithActiveSpan, - suppressTracing: suppressTracing, + getTraceData: getTraceData as typeof defaultGetTraceData, }); } diff --git a/packages/opentelemetry/src/utils/getTraceData.ts b/packages/opentelemetry/src/utils/getTraceData.ts new file mode 100644 index 000000000000..d3fcca662673 --- /dev/null +++ b/packages/opentelemetry/src/utils/getTraceData.ts @@ -0,0 +1,40 @@ +import * as api from '@opentelemetry/api'; +import type { Span } from '@opentelemetry/api'; +import type { TraceData } from '@sentry/core'; +import { getCurrentScope } from '@sentry/core'; +import { getPropagationContextFromSpan } from '../propagator'; +import { generateSpanContextForPropagationContext } from './generateSpanContextForPropagationContext'; + +/** + * Otel-specific implementation of `getTraceData`. + * @see `@sentry/core` version of `getTraceData` for more information + */ +export function getTraceData(span?: Span): TraceData { + const ctx = api.context.active(); + const spanToUse = span || api.trace.getSpan(ctx); + + // This should never happen, given we always create an ambient non-recording span if there's no active span. + if (!spanToUse) { + return {}; + } + const headersObject: Record = {}; + + const propagationContext = spanToUse + ? getPropagationContextFromSpan(spanToUse) + : getCurrentScope().getPropagationContext(); + + const spanContext = generateSpanContextForPropagationContext(propagationContext); + + const context = api.trace.setSpanContext(ctx, spanContext); + + api.propagation.inject(context, headersObject); + + if (!headersObject['sentry-trace']) { + return {}; + } + + return { + 'sentry-trace': headersObject['sentry-trace'], + baggage: headersObject['baggage'], + }; +} From d4a299bf4b2cc59a526f5273a7cef878bddebeee Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Aug 2024 17:16:01 +0200 Subject: [PATCH 2/7] add integration test --- .../suites/tracing/meta-tags-twp/server.js | 34 +++++++++++++++++++ .../suites/tracing/meta-tags-twp/test.ts | 33 ++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js new file mode 100644 index 000000000000..a9fed518b1f3 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js @@ -0,0 +1,34 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + debug: true, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.get('/test', (_req, res) => { + res.send({ + response: ` + + + ${Sentry.getTraceMetaTags()} + + + Hi :) + + + `, + }); +}); + +Sentry.setupExpressErrorHandler(app); + +// TODO: remove port again +startExpressServerAndSendPortToRunner(app, 3000); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts new file mode 100644 index 000000000000..9366d9b78cfa --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts @@ -0,0 +1,33 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('getTraceMetaTags', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('injects sentry tracing tags without sampled flag for Tracing Without Performance', async () => { + const runner = createRunner(__dirname, 'server.js').start(); + + const response = await runner.makeRequest('get', '/test'); + + // @ts-ignore - response is defined, types just don't reflect it + const html = response?.response as unknown as string; + + console.log(html); + + const [_, traceId, spanId] = html.match(//) || [ + undefined, + undefined, + undefined, + ]; + + expect(traceId).toBeDefined(); + expect(spanId).toBeDefined(); + + const sentryBaggageContent = html.match(//)?.[1]; + + expect(sentryBaggageContent).toEqual( + `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${traceId}`, + ); + }); +}); From b90ec8d66ddd9cfaa01e24397772ddd5fc0f89e6 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 09:54:29 +0200 Subject: [PATCH 3/7] remove params --- packages/astro/src/server/middleware.ts | 9 +- packages/core/src/asyncContext/types.ts | 2 +- packages/core/src/index.ts | 3 +- packages/core/src/utils/meta.ts | 5 +- packages/core/src/utils/traceData.ts | 31 ++--- .../core/test/lib/utils/traceData.test.ts | 124 +++++++++--------- .../opentelemetry/src/asyncContextStrategy.ts | 4 +- .../opentelemetry/src/utils/getTraceData.ts | 7 +- packages/types/src/index.ts | 2 +- packages/types/src/tracing.ts | 9 ++ 10 files changed, 99 insertions(+), 97 deletions(-) diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 828b2c3b58f5..3752bd30d448 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -11,7 +11,7 @@ import { startSpan, withIsolationScope, } from '@sentry/node'; -import type { Client, Scope, Span, SpanAttributes } from '@sentry/types'; +import type { Scope, SpanAttributes } from '@sentry/types'; import { addNonEnumerableProperty, objectify, @@ -151,7 +151,6 @@ async function instrumentRequest( setHttpStatus(span, originalResponse.status); } - const scope = getCurrentScope(); const client = getClient(); const contentType = originalResponse.headers.get('content-type'); @@ -175,7 +174,7 @@ async function instrumentRequest( start: async controller => { for await (const chunk of originalBody) { const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html, scope, client, span); + const modifiedHtml = addMetaTagToHead(html); controller.enqueue(new TextEncoder().encode(modifiedHtml)); } controller.close(); @@ -199,11 +198,11 @@ async function instrumentRequest( * This function optimistically assumes that the HTML coming in chunks will not be split * within the tag. If this still happens, we simply won't replace anything. */ -function addMetaTagToHead(htmlChunk: string, scope: Scope, client: Client, span?: Span): string { +function addMetaTagToHead(htmlChunk: string): string { if (typeof htmlChunk !== 'string') { return htmlChunk; } - const metaTags = getTraceMetaTags(span, scope, client); + const metaTags = getTraceMetaTags(); if (!metaTags) { return htmlChunk; diff --git a/packages/core/src/asyncContext/types.ts b/packages/core/src/asyncContext/types.ts index a99f9eaae567..af70ed6f0f8d 100644 --- a/packages/core/src/asyncContext/types.ts +++ b/packages/core/src/asyncContext/types.ts @@ -66,6 +66,6 @@ export interface AsyncContextStrategy { /** Suppress tracing in the given callback, ensuring no spans are generated inside of it. */ suppressTracing?: typeof suppressTracing; - /** get trace data as serialized string values for propagation via `sentry-trace` and `baggage` */ + /** Get trace data as serialized string values for propagation via `sentry-trace` and `baggage`. */ getTraceData?: typeof getTraceData; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 79299f8092d6..792bf3572934 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,11 +1,10 @@ -export type { ClientClass } from './sdk'; +export type { ClientClass as SentryCoreCurrentScopes } from './sdk'; export type { AsyncContextStrategy } from './asyncContext/types'; export type { Carrier } from './carrier'; export type { OfflineStore, OfflineTransportOptions } from './transports/offline'; export type { ServerRuntimeClientOptions } from './server-runtime-client'; export type { RequestDataIntegrationOptions } from './integrations/requestdata'; export type { IntegrationIndex } from './integration'; -export type { TraceData } from './utils/traceData'; export * from './tracing'; export * from './semanticAttributes'; diff --git a/packages/core/src/utils/meta.ts b/packages/core/src/utils/meta.ts index 339dfcee2f28..7db802582eef 100644 --- a/packages/core/src/utils/meta.ts +++ b/packages/core/src/utils/meta.ts @@ -1,4 +1,3 @@ -import type { Client, Scope, Span } from '@sentry/types'; import { getTraceData } from './traceData'; /** @@ -22,8 +21,8 @@ import { getTraceData } from './traceData'; * ``` * */ -export function getTraceMetaTags(span?: Span, scope?: Scope, client?: Client): string { - return Object.entries(getTraceData(span, scope, client)) +export function getTraceMetaTags(): string { + return Object.entries(getTraceData()) .map(([key, value]) => ``) .join('\n'); } diff --git a/packages/core/src/utils/traceData.ts b/packages/core/src/utils/traceData.ts index 854bdd41dd28..831e8187996e 100644 --- a/packages/core/src/utils/traceData.ts +++ b/packages/core/src/utils/traceData.ts @@ -1,4 +1,4 @@ -import type { Client, Scope, Span } from '@sentry/types'; +import type { SerializedTraceData } from '@sentry/types'; import { TRACEPARENT_REGEXP, dynamicSamplingContextToSentryBaggageHeader, @@ -11,11 +11,6 @@ import { getClient, getCurrentScope } from '../currentScopes'; import { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from '../tracing'; import { getActiveSpan, getRootSpan, spanToTraceHeader } from './spanUtils'; -export type TraceData = { - 'sentry-trace'?: string; - baggage?: string; -}; - /** * Extracts trace propagation data from the current span or from the client's scope (via transaction or propagation * context) and serializes it to `sentry-trace` and `baggage` values to strings. These values can be used to propagate @@ -24,35 +19,31 @@ export type TraceData = { * This function also applies some validation to the generated sentry-trace and baggage values to ensure that * only valid strings are returned. * - * @param span a span to take the trace data from. By default, the currently active span is used. - * @param scope the scope to take trace data from By default, the active current scope is used. - * @param client the SDK's client to take trace data from. By default, the current client is used. - * * @returns an object with the tracing data values. The object keys are the name of the tracing key to be used as header * or meta tag name. */ -export function getTraceData(span?: Span, scope?: Scope, client?: Client): TraceData { +export function getTraceData(): SerializedTraceData { const carrier = getMainCarrier(); const acs = getAsyncContextStrategy(carrier); if (acs.getTraceData) { - return acs.getTraceData(span, scope, client); + return acs.getTraceData(); } - const clientToUse = client || getClient(); - const scopeToUse = scope || getCurrentScope(); - const spanToUse = span || getActiveSpan(); + const client = getClient(); + const scope = getCurrentScope(); + const span = getActiveSpan(); - const { dsc, sampled, traceId } = scopeToUse.getPropagationContext(); - const rootSpan = spanToUse && getRootSpan(spanToUse); + const { dsc, sampled, traceId } = scope.getPropagationContext(); + const rootSpan = span && getRootSpan(span); - const sentryTrace = spanToUse ? spanToTraceHeader(spanToUse) : generateSentryTraceHeader(traceId, undefined, sampled); + const sentryTrace = span ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, undefined, sampled); const dynamicSamplingContext = rootSpan ? getDynamicSamplingContextFromSpan(rootSpan) : dsc ? dsc - : clientToUse - ? getDynamicSamplingContextFromClient(traceId, clientToUse) + : client + ? getDynamicSamplingContextFromClient(traceId, client) : undefined; const baggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); diff --git a/packages/core/test/lib/utils/traceData.test.ts b/packages/core/test/lib/utils/traceData.test.ts index e757926ca30d..a6fb3c57814e 100644 --- a/packages/core/test/lib/utils/traceData.test.ts +++ b/packages/core/test/lib/utils/traceData.test.ts @@ -1,5 +1,7 @@ import { SentrySpan, getTraceData } from '../../../src/'; +import * as SentryCoreCurrentScopes from '../../../src/currentScopes'; import * as SentryCoreTracing from '../../../src/tracing'; +import * as SentryCoreSpanUtils from '../../../src/utils/spanUtils'; import { isValidBaggageString } from '../../../src/utils/traceData'; @@ -25,10 +27,12 @@ describe('getTraceData', () => { jest.spyOn(SentryCoreTracing, 'getDynamicSamplingContextFromSpan').mockReturnValueOnce({ environment: 'production', }); + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => mockedSpan); + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce(() => mockedScope); - const tags = getTraceData(mockedSpan, mockedScope, mockedClient); + const data = getTraceData(); - expect(tags).toEqual({ + expect(data).toEqual({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', baggage: 'sentry-environment=production', }); @@ -36,22 +40,25 @@ describe('getTraceData', () => { }); it('returns propagationContext DSC data if no span is available', () => { - const traceData = getTraceData( - undefined, - { - getPropagationContext: () => ({ - traceId: '12345678901234567890123456789012', - sampled: true, - spanId: '1234567890123456', - dsc: { - environment: 'staging', - public_key: 'key', - trace_id: '12345678901234567890123456789012', - }, - }), - } as any, - mockedClient, + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => undefined); + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce( + () => + ({ + getPropagationContext: () => ({ + traceId: '12345678901234567890123456789012', + sampled: true, + spanId: '1234567890123456', + dsc: { + environment: 'staging', + public_key: 'key', + trace_id: '12345678901234567890123456789012', + }, + }), + }) as any, ); + jest.spyOn(SentryCoreCurrentScopes, 'getClient').mockImplementationOnce(() => mockedClient); + + const traceData = getTraceData(); expect(traceData).toEqual({ 'sentry-trace': expect.stringMatching(/12345678901234567890123456789012-(.{16})-1/), @@ -65,21 +72,22 @@ describe('getTraceData', () => { public_key: undefined, }); - const traceData = getTraceData( - // @ts-expect-error - we don't need to provide all the properties - { - isRecording: () => true, - spanContext: () => { - return { - traceId: '12345678901234567890123456789012', - spanId: '1234567890123456', - traceFlags: TRACE_FLAG_SAMPLED, - }; - }, + // @ts-expect-error - we don't need to provide all the properties + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => ({ + isRecording: () => true, + spanContext: () => { + return { + traceId: '12345678901234567890123456789012', + spanId: '1234567890123456', + traceFlags: TRACE_FLAG_SAMPLED, + }; }, - mockedScope, - mockedClient, - ); + })); + + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce(() => mockedScope); + jest.spyOn(SentryCoreCurrentScopes, 'getClient').mockImplementationOnce(() => mockedClient); + + const traceData = getTraceData(); expect(traceData).toEqual({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', @@ -92,21 +100,21 @@ describe('getTraceData', () => { public_key: undefined, }); - const traceData = getTraceData( - // @ts-expect-error - we don't need to provide all the properties - { - isRecording: () => true, - spanContext: () => { - return { - traceId: '12345678901234567890123456789012', - spanId: '1234567890123456', - traceFlags: TRACE_FLAG_SAMPLED, - }; - }, + // @ts-expect-error - we don't need to provide all the properties + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => ({ + isRecording: () => true, + spanContext: () => { + return { + traceId: '12345678901234567890123456789012', + spanId: '1234567890123456', + traceFlags: TRACE_FLAG_SAMPLED, + }; }, - mockedScope, - undefined, - ); + })); + jest.spyOn(SentryCoreCurrentScopes, 'getCurrentScope').mockImplementationOnce(() => mockedScope); + jest.spyOn(SentryCoreCurrentScopes, 'getClient').mockImplementationOnce(() => undefined); + + const traceData = getTraceData(); expect(traceData).toEqual({ 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', @@ -115,21 +123,19 @@ describe('getTraceData', () => { }); it('returns an empty object if the `sentry-trace` value is invalid', () => { - const traceData = getTraceData( - // @ts-expect-error - we don't need to provide all the properties - { - isRecording: () => true, - spanContext: () => { - return { - traceId: '1234567890123456789012345678901+', - spanId: '1234567890123456', - traceFlags: TRACE_FLAG_SAMPLED, - }; - }, + // @ts-expect-error - we don't need to provide all the properties + jest.spyOn(SentryCoreSpanUtils, 'getActiveSpan').mockImplementationOnce(() => ({ + isRecording: () => true, + spanContext: () => { + return { + traceId: '1234567890123456789012345678901+', + spanId: '1234567890123456', + traceFlags: TRACE_FLAG_SAMPLED, + }; }, - mockedScope, - mockedClient, - ); + })); + + const traceData = getTraceData(); expect(traceData).toEqual({}); }); diff --git a/packages/opentelemetry/src/asyncContextStrategy.ts b/packages/opentelemetry/src/asyncContextStrategy.ts index 7d81a76d5d80..31da9479921f 100644 --- a/packages/opentelemetry/src/asyncContextStrategy.ts +++ b/packages/opentelemetry/src/asyncContextStrategy.ts @@ -1,6 +1,6 @@ import * as api from '@opentelemetry/api'; import { getDefaultCurrentScope, getDefaultIsolationScope, setAsyncContextStrategy } from '@sentry/core'; -import type { getTraceData as defaultGetTraceData, withActiveSpan as defaultWithActiveSpan } from '@sentry/core'; +import type { withActiveSpan as defaultWithActiveSpan } from '@sentry/core'; import type { Scope } from '@sentry/types'; import { @@ -104,9 +104,9 @@ export function setOpenTelemetryContextAsyncContextStrategy(): void { startInactiveSpan, getActiveSpan, suppressTracing, + getTraceData, // The types here don't fully align, because our own `Span` type is narrower // than the OTEL one - but this is OK for here, as we now we'll only have OTEL spans passed around withActiveSpan: withActiveSpan as typeof defaultWithActiveSpan, - getTraceData: getTraceData as typeof defaultGetTraceData, }); } diff --git a/packages/opentelemetry/src/utils/getTraceData.ts b/packages/opentelemetry/src/utils/getTraceData.ts index d3fcca662673..0e8037054419 100644 --- a/packages/opentelemetry/src/utils/getTraceData.ts +++ b/packages/opentelemetry/src/utils/getTraceData.ts @@ -1,7 +1,6 @@ import * as api from '@opentelemetry/api'; -import type { Span } from '@opentelemetry/api'; -import type { TraceData } from '@sentry/core'; import { getCurrentScope } from '@sentry/core'; +import type { SerializedTraceData } from '@sentry/types'; import { getPropagationContextFromSpan } from '../propagator'; import { generateSpanContextForPropagationContext } from './generateSpanContextForPropagationContext'; @@ -9,9 +8,9 @@ import { generateSpanContextForPropagationContext } from './generateSpanContextF * Otel-specific implementation of `getTraceData`. * @see `@sentry/core` version of `getTraceData` for more information */ -export function getTraceData(span?: Span): TraceData { +export function getTraceData(): SerializedTraceData { const ctx = api.context.active(); - const spanToUse = span || api.trace.getSpan(ctx); + const spanToUse = api.trace.getSpan(ctx); // This should never happen, given we always create an ambient non-recording span if there's no active span. if (!spanToUse) { diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 8f7fdce74c33..1022e69ad49e 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -121,7 +121,7 @@ export type { SpanStatus } from './spanStatus'; export type { TimedEvent } from './timedEvent'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; -export type { PropagationContext, TracePropagationTargets } from './tracing'; +export type { PropagationContext, TracePropagationTargets, SerializedTraceData } from './tracing'; export type { StartSpanOptions } from './startSpanOptions'; export type { TraceparentData, diff --git a/packages/types/src/tracing.ts b/packages/types/src/tracing.ts index 701f9930d314..7af40f3507f7 100644 --- a/packages/types/src/tracing.ts +++ b/packages/types/src/tracing.ts @@ -42,3 +42,12 @@ export interface PropagationContext { */ dsc?: Partial; } + +/** + * An object holding trace data, like span and trace ids, sampling decision, and dynamic sampling context + * in a serialized form. Both keys are expected to be used as Http headers or Html meta tags. + */ +export interface SerializedTraceData { + 'sentry-trace'?: string; + baggage?: string; +} From 0de4eb11bc6d33dd1f4df5502bb54be4f59b4384 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 10:10:04 +0200 Subject: [PATCH 4/7] simplify otel implementation --- .../suites/tracing/meta-tags-twp/server.js | 4 +--- .../suites/tracing/meta-tags-twp/test.ts | 4 +--- .../opentelemetry/src/utils/getTraceData.ts | 24 ++++++------------- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js index a9fed518b1f3..4dded9cd0ef6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/server.js @@ -4,7 +4,6 @@ const Sentry = require('@sentry/node'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', transport: loggingTransport, - debug: true, }); // express must be required after Sentry is initialized @@ -30,5 +29,4 @@ app.get('/test', (_req, res) => { Sentry.setupExpressErrorHandler(app); -// TODO: remove port again -startExpressServerAndSendPortToRunner(app, 3000); +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts index 9366d9b78cfa..26614e96f1f2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts @@ -13,9 +13,7 @@ describe('getTraceMetaTags', () => { // @ts-ignore - response is defined, types just don't reflect it const html = response?.response as unknown as string; - console.log(html); - - const [_, traceId, spanId] = html.match(//) || [ + const [, traceId, spanId] = html.match(//) || [ undefined, undefined, undefined, diff --git a/packages/opentelemetry/src/utils/getTraceData.ts b/packages/opentelemetry/src/utils/getTraceData.ts index 0e8037054419..0647899b3890 100644 --- a/packages/opentelemetry/src/utils/getTraceData.ts +++ b/packages/opentelemetry/src/utils/getTraceData.ts @@ -1,30 +1,20 @@ import * as api from '@opentelemetry/api'; -import { getCurrentScope } from '@sentry/core'; import type { SerializedTraceData } from '@sentry/types'; -import { getPropagationContextFromSpan } from '../propagator'; -import { generateSpanContextForPropagationContext } from './generateSpanContextForPropagationContext'; +import { dropUndefinedKeys } from '@sentry/utils'; /** * Otel-specific implementation of `getTraceData`. * @see `@sentry/core` version of `getTraceData` for more information */ export function getTraceData(): SerializedTraceData { - const ctx = api.context.active(); - const spanToUse = api.trace.getSpan(ctx); + const context = api.context.active(); // This should never happen, given we always create an ambient non-recording span if there's no active span. - if (!spanToUse) { + if (!context) { return {}; } - const headersObject: Record = {}; - - const propagationContext = spanToUse - ? getPropagationContextFromSpan(spanToUse) - : getCurrentScope().getPropagationContext(); - const spanContext = generateSpanContextForPropagationContext(propagationContext); - - const context = api.trace.setSpanContext(ctx, spanContext); + const headersObject: Record = {}; api.propagation.inject(context, headersObject); @@ -32,8 +22,8 @@ export function getTraceData(): SerializedTraceData { return {}; } - return { + return dropUndefinedKeys({ 'sentry-trace': headersObject['sentry-trace'], - baggage: headersObject['baggage'], - }; + baggage: headersObject.baggage, + }); } From c6208ba1c2bbd08718229f6aabee1d0443e8c567 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 10:35:12 +0200 Subject: [PATCH 5/7] cleanup --- packages/core/src/asyncContext/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/asyncContext/types.ts b/packages/core/src/asyncContext/types.ts index af70ed6f0f8d..9fb9f9f4bec8 100644 --- a/packages/core/src/asyncContext/types.ts +++ b/packages/core/src/asyncContext/types.ts @@ -1,5 +1,5 @@ import type { Scope } from '@sentry/types'; -import { getTraceData } from '../utils/traceData'; +import type { getTraceData } from '../utils/traceData'; import type { startInactiveSpan, startSpan, From 3688841a171a78db34bf03b0581dad1804f447cd Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 12:17:31 +0200 Subject: [PATCH 6/7] fix test pls --- .../suites/tracing/meta-tags-twp/test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts index 26614e96f1f2..f3179beede6d 100644 --- a/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags-twp/test.ts @@ -24,8 +24,8 @@ describe('getTraceMetaTags', () => { const sentryBaggageContent = html.match(//)?.[1]; - expect(sentryBaggageContent).toEqual( - `sentry-environment=production,sentry-public_key=public,sentry-trace_id=${traceId}`, - ); + expect(sentryBaggageContent).toContain('sentry-environment=production'); + expect(sentryBaggageContent).toContain('sentry-public_key=public'); + expect(sentryBaggageContent).toContain(`sentry-trace_id=${traceId}`); }); }); From dda6ee67cc014bad499bda251c8f3f6afd8f1f07 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 17:11:29 +0200 Subject: [PATCH 7/7] remove unnecessary definedness check --- packages/opentelemetry/src/utils/getTraceData.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/opentelemetry/src/utils/getTraceData.ts b/packages/opentelemetry/src/utils/getTraceData.ts index 0647899b3890..d85f6f699ef3 100644 --- a/packages/opentelemetry/src/utils/getTraceData.ts +++ b/packages/opentelemetry/src/utils/getTraceData.ts @@ -7,16 +7,9 @@ import { dropUndefinedKeys } from '@sentry/utils'; * @see `@sentry/core` version of `getTraceData` for more information */ export function getTraceData(): SerializedTraceData { - const context = api.context.active(); - - // This should never happen, given we always create an ambient non-recording span if there's no active span. - if (!context) { - return {}; - } - const headersObject: Record = {}; - api.propagation.inject(context, headersObject); + api.propagation.inject(api.context.active(), headersObject); if (!headersObject['sentry-trace']) { return {};