From 882a24ecbe4aec75d7f971e34c0d549332fb5e35 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 26 Feb 2024 11:03:01 +0000 Subject: [PATCH 01/10] feat(opentelemetry): Ensure DSC & attributes are correctly set --- .../src/tracing/dynamicSamplingContext.ts | 1 - packages/opentelemetry/src/constants.ts | 1 + packages/opentelemetry/src/propagator.ts | 29 +++++++-- .../opentelemetry/src/semanticAttributes.ts | 1 - .../src/setupEventContextTrace.ts | 19 ++++-- packages/opentelemetry/src/spanExporter.ts | 35 ++++++++--- .../src/utils/dynamicSamplingContext.ts | 61 +++++++++++++++++++ .../test/integration/scope.test.ts | 35 ++++++++--- .../test/integration/transactions.test.ts | 12 +++- 9 files changed, 163 insertions(+), 31 deletions(-) create mode 100644 packages/opentelemetry/src/utils/dynamicSamplingContext.ts diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 222763a8a781..e2dd9af12b0b 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -47,7 +47,6 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly((b, [dscKey, dscValue]) => { @@ -58,11 +65,19 @@ export class SentryPropagator extends W3CBaggagePropagator { // Add propagation context to context const contextWithPropagationContext = setPropagationContextOnContext(context, propagationContext); + // We store the DSC as OTEL trace state on the span context + const dscString = propagationContext.dsc + ? dynamicSamplingContextToSentryBaggageHeader(propagationContext.dsc) + : undefined; + + const traceState = dscString ? new TraceState().set(SENTRY_TRACE_STATE_DSC, dscString) : undefined; + const spanContext: SpanContext = { traceId: propagationContext.traceId, spanId: propagationContext.parentSpanId || '', isRemote: true, traceFlags: propagationContext.sampled === true ? TraceFlags.SAMPLED : TraceFlags.NONE, + traceState, }; // Add remote parent span context @@ -77,7 +92,8 @@ export class SentryPropagator extends W3CBaggagePropagator { } } -function getDsc( +/** Get the DSC. */ +function getDynamicSamplingContext( propagationContext: PropagationContext, traceId: string | undefined, ): Partial | undefined { @@ -96,6 +112,7 @@ function getDsc( return undefined; } +/** Get the trace data for propagation. */ function getSentryTraceData( context: Context, propagationContext: PropagationContext | undefined, diff --git a/packages/opentelemetry/src/semanticAttributes.ts b/packages/opentelemetry/src/semanticAttributes.ts index 00c37f061079..49983f14dcd2 100644 --- a/packages/opentelemetry/src/semanticAttributes.ts +++ b/packages/opentelemetry/src/semanticAttributes.ts @@ -6,7 +6,6 @@ export const InternalSentrySemanticAttributes = { ORIGIN: 'sentry.origin', OP: 'sentry.op', SOURCE: 'sentry.source', - SAMPLE_RATE: 'sentry.sample_rate', PARENT_SAMPLED: 'sentry.parentSampled', BREADCRUMB_TYPE: 'sentry.breadcrumb.type', BREADCRUMB_LEVEL: 'sentry.breadcrumb.level', diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts index fb41ce463af5..8d45fb787326 100644 --- a/packages/opentelemetry/src/setupEventContextTrace.ts +++ b/packages/opentelemetry/src/setupEventContextTrace.ts @@ -1,13 +1,16 @@ import type { Client } from '@sentry/types'; +import { dropUndefinedKeys } from '@sentry/utils'; -import { getActiveSpan } from './utils/getActiveSpan'; -import { spanHasParentId } from './utils/spanTypes'; +import { getActiveSpan, getRootSpan } from './utils/getActiveSpan'; +import { spanHasName, spanHasParentId } from './utils/spanTypes'; /** Ensure the `trace` context is set on all events. */ export function setupEventContextTrace(client: Client): void { client.addEventProcessor(event => { const span = getActiveSpan(); - if (!span) { + // For transaction events, this is handled separately + // Because the active span may not be the span that is actually the transaction event + if (!span || event.type === 'transaction') { return event; } @@ -15,14 +18,20 @@ export function setupEventContextTrace(client: Client): void { // If event has already set `trace` context, use that one. event.contexts = { - trace: { + trace: dropUndefinedKeys({ trace_id: spanContext.traceId, span_id: spanContext.spanId, parent_span_id: spanHasParentId(span) ? span.parentSpanId : undefined, - }, + }), ...event.contexts, }; + const rootSpan = getRootSpan(span); + const transactionName = spanHasName(rootSpan) ? rootSpan.name : undefined; + if (transactionName) { + event.tags = { transaction: transactionName, ...event.tags }; + } + return event; }); } diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index a0488ac22f64..aadaa9a6ace0 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -7,12 +7,13 @@ import type { Transaction } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, flush, getCurrentHub } from '@sentry/core'; import type { Scope, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types'; -import { addNonEnumerableProperty, logger } from '@sentry/utils'; +import { addNonEnumerableProperty, dropUndefinedKeys, logger } from '@sentry/utils'; import { startTransaction } from './custom/transaction'; import { DEBUG_BUILD } from './debug-build'; import { InternalSentrySemanticAttributes } from './semanticAttributes'; import { convertOtelTimeToSeconds } from './utils/convertOtelTimeToSeconds'; +import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; import { getRequestSpanData } from './utils/getRequestSpanData'; import type { SpanNode } from './utils/groupSpansWithParents'; import { groupSpansWithParents } from './utils/groupSpansWithParents'; @@ -149,6 +150,17 @@ function createTransactionForOtelSpan(span: ReadableSpan): Transaction { const metadata = getSpanMetadata(span); const capturedSpanScopes = getSpanScopes(span); + const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined; + + const attributes = { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, + ...data, + ...removeSentryAttributes(span.attributes), + }; + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(span); + const transaction = startTransaction(hub, { spanId, traceId, @@ -159,13 +171,13 @@ function createTransactionForOtelSpan(span: ReadableSpan): Transaction { status: mapStatus(span), startTimestamp: convertOtelTimeToSeconds(span.startTime), metadata: { - sampleRate: span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined, + ...dropUndefinedKeys({ + dynamicSamplingContext, + sampleRate, + }), ...metadata, }, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, - }, - data: removeSentryAttributes(data), + attributes, origin, tags, sampled: true, @@ -180,7 +192,14 @@ function createTransactionForOtelSpan(span: ReadableSpan): Transaction { }); if (capturedSpanScopes) { - setCapturedScopesOnTransaction(transaction, capturedSpanScopes.scope, capturedSpanScopes.isolationScope); + // Ensure the `transaction` tag is correctly set on the transaction event + const scope = capturedSpanScopes.scope.clone(); + scope.addEventProcessor(event => { + event.tags = { transaction: description, ...event.tags }; + return event; + }); + + setCapturedScopesOnTransaction(transaction, scope, capturedSpanScopes.isolationScope); } return transaction; @@ -264,6 +283,8 @@ function removeSentryAttributes(data: Record): Record> { + const client = getClient(); + if (!client) { + return {}; + } + + const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client); + + const traceState = span.spanContext().traceState; + const traceStateDsc = traceState?.get(SENTRY_TRACE_STATE_DSC); + + if (traceStateDsc) { + const dsc = baggageHeaderToDynamicSamplingContext(traceStateDsc); + if (dsc) { + return dsc; + } + } + + const attributes = spanHasAttributes(span) ? span.attributes : {}; + + const sampleRate = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]; + if (sampleRate != null) { + dsc.sample_rate = `${sampleRate}`; + } + + // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII + const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; + const name = spanHasName(span) ? span.name : ''; + + if (source !== 'url' && name) { + dsc.transaction = name; + } + + // eslint-disable-next-line no-bitwise + const sampled = Boolean(span.spanContext().traceFlags & TraceFlags.SAMPLED); + dsc.sampled = String(sampled); + + client.emit('createDsc', dsc); + + return dsc; +} diff --git a/packages/opentelemetry/test/integration/scope.test.ts b/packages/opentelemetry/test/integration/scope.test.ts index 594fa1bcf9ae..7a32acbe2364 100644 --- a/packages/opentelemetry/test/integration/scope.test.ts +++ b/packages/opentelemetry/test/integration/scope.test.ts @@ -64,21 +64,33 @@ describe('Integration | Scope', () => { await client.flush(); expect(beforeSend).toHaveBeenCalledTimes(1); + + if (spanId) { + expect(beforeSend).toHaveBeenCalledWith( + expect.objectContaining({ + contexts: { + trace: { + span_id: spanId, + trace_id: traceId, + }, + }, + }), + { + event_id: expect.any(String), + originalException: error, + syntheticException: expect.any(Error), + }, + ); + } + expect(beforeSend).toHaveBeenCalledWith( expect.objectContaining({ - contexts: expect.objectContaining({ - trace: spanId - ? { - span_id: spanId, - trace_id: traceId, - } - : expect.any(Object), - }), tags: { tag1: 'val1', tag2: 'val2', tag3: 'val3', tag4: 'val4', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { @@ -99,6 +111,7 @@ describe('Integration | Scope', () => { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual', 'sentry.source': 'custom', + 'sentry.sample_rate': 1, }, span_id: spanId, status: 'ok', @@ -106,7 +119,6 @@ describe('Integration | Scope', () => { origin: 'manual', }, }), - spans: [], start_timestamp: expect.any(Number), tags: { @@ -114,6 +126,7 @@ describe('Integration | Scope', () => { tag2: 'val2', tag3: 'val3', tag4: 'val4', + transaction: 'outer', }, timestamp: expect.any(Number), transaction: 'outer', @@ -207,6 +220,7 @@ describe('Integration | Scope', () => { tag2: 'val2a', tag3: 'val3a', tag4: 'val4a', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { @@ -232,6 +246,7 @@ describe('Integration | Scope', () => { tag2: 'val2b', tag3: 'val3b', tag4: 'val4b', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { @@ -331,6 +346,7 @@ describe('Integration | Scope', () => { tag4: 'val4a', isolationTag1: 'val1', isolationTag2: 'val2', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { @@ -358,6 +374,7 @@ describe('Integration | Scope', () => { tag4: 'val4b', isolationTag1: 'val1', isolationTag2: 'val2b', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 3ce365a6e886..0023b84b3b54 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -89,6 +89,8 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', + 'sentry.sample_rate': 1, + 'test.outer': 'test value', }, op: 'test op', span_id: expect.any(String), @@ -119,6 +121,7 @@ describe('Integration | Transactions', () => { tags: { 'outer.tag': 'test value', 'test.tag': 'test value', + transaction: 'test name', }, timestamp: expect.any(Number), transaction: 'test name', @@ -246,6 +249,8 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', + 'test.outer': 'test value', + 'sentry.sample_rate': 1, }, op: 'test op', span_id: expect.any(String), @@ -256,7 +261,7 @@ describe('Integration | Transactions', () => { }), spans: [expect.any(Object), expect.any(Object)], start_timestamp: expect.any(Number), - tags: { 'test.tag': 'test value' }, + tags: { 'test.tag': 'test value', transaction: 'test name' }, timestamp: expect.any(Number), transaction: 'test name', transaction_info: { source: 'task' }, @@ -286,6 +291,8 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op b', 'sentry.origin': 'manual', 'sentry.source': 'custom', + 'test.outer': 'test value b', + 'sentry.sample_rate': 1, }, op: 'test op b', span_id: expect.any(String), @@ -296,7 +303,7 @@ describe('Integration | Transactions', () => { }), spans: [expect.any(Object), expect.any(Object)], start_timestamp: expect.any(Number), - tags: { 'test.tag': 'test value b' }, + tags: { 'test.tag': 'test value b', transaction: 'test name b' }, timestamp: expect.any(Number), transaction: 'test name b', transaction_info: { source: 'custom' }, @@ -361,6 +368,7 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', + 'sentry.sample_rate': 1, }, op: 'test op', span_id: expect.any(String), From 28bffa1fc12ff75c6d702095e7ac0ed49e8910ab Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 26 Feb 2024 11:51:02 +0000 Subject: [PATCH 02/10] fix tests for node --- .../test/integration/scope.test.ts | 39 +++++++--- .../test/integration/transactions.test.ts | 77 ++++++++++--------- .../test/integration/transactions.test.ts | 2 +- 3 files changed, 70 insertions(+), 48 deletions(-) diff --git a/packages/node-experimental/test/integration/scope.test.ts b/packages/node-experimental/test/integration/scope.test.ts index 6552037c548d..ed5815e2df40 100644 --- a/packages/node-experimental/test/integration/scope.test.ts +++ b/packages/node-experimental/test/integration/scope.test.ts @@ -56,22 +56,33 @@ describe('Integration | Scope', () => { await client.flush(); expect(beforeSend).toHaveBeenCalledTimes(1); + + if (spanId) { + expect(beforeSend).toHaveBeenCalledWith( + expect.objectContaining({ + contexts: expect.objectContaining({ + trace: { + span_id: spanId, + trace_id: traceId, + }, + }), + }), + { + event_id: expect.any(String), + originalException: error, + syntheticException: expect.any(Error), + }, + ); + } + expect(beforeSend).toHaveBeenCalledWith( expect.objectContaining({ - contexts: expect.objectContaining({ - trace: spanId - ? { - span_id: spanId, - trace_id: traceId, - parent_span_id: undefined, - } - : expect.any(Object), - }), tags: { tag1: 'val1', tag2: 'val2', tag3: 'val3', tag4: 'val4', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { @@ -88,7 +99,12 @@ describe('Integration | Scope', () => { expect.objectContaining({ contexts: expect.objectContaining({ trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual', 'sentry.source': 'custom' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + }, span_id: spanId, status: 'ok', trace_id: traceId, @@ -102,6 +118,7 @@ describe('Integration | Scope', () => { tag2: 'val2', tag3: 'val3', tag4: 'val4', + transaction: 'outer', }, timestamp: expect.any(Number), transaction: 'outer', @@ -186,6 +203,7 @@ describe('Integration | Scope', () => { tag2: 'val2a', tag3: 'val3a', tag4: 'val4a', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { @@ -211,6 +229,7 @@ describe('Integration | Scope', () => { tag2: 'val2b', tag3: 'val3b', tag4: 'val4b', + ...(enableTracing ? { transaction: 'outer' } : {}), }, }), { diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts index f5753055bc1c..b1fbc25cdceb 100644 --- a/packages/node-experimental/test/integration/transactions.test.ts +++ b/packages/node-experimental/test/integration/transactions.test.ts @@ -1,12 +1,11 @@ import { TraceFlags, context, trace } from '@opentelemetry/api'; import type { SpanProcessor } from '@opentelemetry/sdk-trace-base'; -import { spanToJSON } from '@sentry/core'; -import { SentrySpanProcessor, getClient, setPropagationContextOnContext } from '@sentry/opentelemetry'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core'; +import { SentrySpanProcessor, setPropagationContextOnContext } from '@sentry/opentelemetry'; import type { PropagationContext, TransactionEvent } from '@sentry/types'; import { logger } from '@sentry/utils'; import * as Sentry from '../../src'; -import type { NodeClient } from '../../src/sdk/client'; import { cleanupOtel, getProvider, mockSdkInit } from '../helpers/mockSdkInit'; describe('Integration | Transactions', () => { @@ -20,7 +19,7 @@ describe('Integration | Transactions', () => { mockSdkInit({ enableTracing: true, beforeSendTransaction }); - const client = Sentry.getClient(); + const client = Sentry.getClient()!; Sentry.addBreadcrumb({ message: 'test breadcrumb 1', timestamp: 123456 }); Sentry.setTag('outer.tag', 'test value'); @@ -29,11 +28,11 @@ describe('Integration | Transactions', () => { { op: 'test op', name: 'test name', - attributes: { - [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', - }, origin: 'auto.test', metadata: { requestPath: 'test-path' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task', + }, }, span => { Sentry.addBreadcrumb({ message: 'test breadcrumb 2', timestamp: 123456 }); @@ -88,6 +87,8 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', + 'sentry.sample_rate': 1, + 'test.outer': 'test value', }, op: 'test op', span_id: expect.any(String), @@ -120,6 +121,7 @@ describe('Integration | Transactions', () => { tags: { 'outer.tag': 'test value', 'test.tag': 'test value', + transaction: 'test name', }, timestamp: expect.any(Number), transaction: 'test name', @@ -175,39 +177,31 @@ describe('Integration | Transactions', () => { mockSdkInit({ enableTracing: true, beforeSendTransaction }); - const client = Sentry.getClient(); + const client = Sentry.getClient()!; Sentry.addBreadcrumb({ message: 'test breadcrumb 1', timestamp: 123456 }); Sentry.withIsolationScope(() => { - Sentry.startSpan( - { - op: 'test op', - name: 'test name', - origin: 'auto.test', - attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'task' }, - }, - span => { - Sentry.addBreadcrumb({ message: 'test breadcrumb 2', timestamp: 123456 }); + Sentry.startSpan({ op: 'test op', name: 'test name', source: 'task', origin: 'auto.test' }, span => { + Sentry.addBreadcrumb({ message: 'test breadcrumb 2', timestamp: 123456 }); - span.setAttributes({ - 'test.outer': 'test value', - }); + span.setAttributes({ + 'test.outer': 'test value', + }); - const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' }); - subSpan.end(); + const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' }); + subSpan.end(); - Sentry.setTag('test.tag', 'test value'); + Sentry.setTag('test.tag', 'test value'); - Sentry.startSpan({ name: 'inner span 2' }, innerSpan => { - Sentry.addBreadcrumb({ message: 'test breadcrumb 3', timestamp: 123456 }); + Sentry.startSpan({ name: 'inner span 2' }, innerSpan => { + Sentry.addBreadcrumb({ message: 'test breadcrumb 3', timestamp: 123456 }); - innerSpan.setAttributes({ - 'test.inner': 'test value', - }); + innerSpan.setAttributes({ + 'test.inner': 'test value', }); - }, - ); + }); + }); }); Sentry.withIsolationScope(() => { @@ -255,6 +249,8 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', + 'test.outer': 'test value', + 'sentry.sample_rate': 1, }, op: 'test op', span_id: expect.any(String), @@ -265,7 +261,7 @@ describe('Integration | Transactions', () => { }), spans: [expect.any(Object), expect.any(Object)], start_timestamp: expect.any(Number), - tags: { 'test.tag': 'test value' }, + tags: { 'test.tag': 'test value', transaction: 'test name' }, timestamp: expect.any(Number), transaction: 'test name', transaction_info: { source: 'task' }, @@ -295,6 +291,8 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op b', 'sentry.origin': 'manual', 'sentry.source': 'custom', + 'test.outer': 'test value b', + 'sentry.sample_rate': 1, }, op: 'test op b', span_id: expect.any(String), @@ -305,7 +303,7 @@ describe('Integration | Transactions', () => { }), spans: [expect.any(Object), expect.any(Object)], start_timestamp: expect.any(Number), - tags: { 'test.tag': 'test value b' }, + tags: { 'test.tag': 'test value b', transaction: 'test name b' }, timestamp: expect.any(Number), transaction: 'test name b', transaction_info: { source: 'custom' }, @@ -322,7 +320,7 @@ describe('Integration | Transactions', () => { mockSdkInit({ enableTracing: true, beforeSendTransaction }); - const client = Sentry.getClient(); + const client = Sentry.getClient(); Sentry.addBreadcrumb({ message: 'test breadcrumb 1', timestamp: 123456 }); @@ -401,6 +399,8 @@ describe('Integration | Transactions', () => { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual', 'sentry.source': 'custom', + 'test.outer': 'test value', + 'sentry.sample_rate': 1, }, span_id: expect.any(String), status: 'ok', @@ -410,7 +410,7 @@ describe('Integration | Transactions', () => { }), spans: [expect.any(Object), expect.any(Object)], start_timestamp: expect.any(Number), - tags: { 'test.tag': 'test value' }, + tags: { 'test.tag': 'test value', transaction: 'test name' }, timestamp: expect.any(Number), transaction: 'test name', type: 'transaction', @@ -438,6 +438,8 @@ describe('Integration | Transactions', () => { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual', 'sentry.source': 'custom', + 'test.outer': 'test value b', + 'sentry.sample_rate': 1, }, span_id: expect.any(String), status: 'ok', @@ -447,7 +449,7 @@ describe('Integration | Transactions', () => { }), spans: [expect.any(Object), expect.any(Object)], start_timestamp: expect.any(Number), - tags: { 'test.tag': 'test value b' }, + tags: { 'test.tag': 'test value b', transaction: 'test name b' }, timestamp: expect.any(Number), transaction: 'test name b', type: 'transaction', @@ -481,7 +483,7 @@ describe('Integration | Transactions', () => { mockSdkInit({ enableTracing: true, beforeSendTransaction }); - const client = getClient() as NodeClient; + const client = Sentry.getClient()!; // We simulate the correct context we'd normally get from the SentryPropagator context.with( @@ -519,6 +521,7 @@ describe('Integration | Transactions', () => { 'sentry.op': 'test op', 'sentry.origin': 'auto.test', 'sentry.source': 'task', + 'sentry.sample_rate': 1, }, op: 'test op', span_id: expect.any(String), @@ -592,7 +595,7 @@ describe('Integration | Transactions', () => { mockSdkInit({ enableTracing: true, beforeSendTransaction }); - const client = getClient() as NodeClient; + const client = Sentry.getClient()!; const provider = getProvider(); const multiSpanProcessor = provider?.activeSpanProcessor as | (SpanProcessor & { _spanProcessors?: SpanProcessor[] }) diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 0023b84b3b54..9377ca1bc4c7 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -20,7 +20,7 @@ describe('Integration | Transactions', () => { it('correctly creates transaction & spans', async () => { const beforeSendTransaction = jest.fn(() => null); - mockSdkInit({ enableTracing: true, beforeSendTransaction, debug: true }); + mockSdkInit({ enableTracing: true, beforeSendTransaction }); const client = getClient() as TestClientInterface; From 4933d92469e137048e9f6326a78115302bc71e39 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 26 Feb 2024 14:11:49 +0000 Subject: [PATCH 03/10] fix e2e test? --- .../tests/propagation.test.ts | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index cbb2c889edec..0317b9eedd03 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -60,12 +60,29 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { contexts: expect.objectContaining({ trace: { data: { - url: 'http://localhost:3030/test-outgoing-http', - 'otel.kind': 'SERVER', + 'http.flavor': '1.1', + 'http.host': 'localhost:3030', + 'http.method': 'GET', 'http.response.status_code': 200, + 'http.route': '/test-outgoing-http', + 'http.scheme': 'http', + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.target': '/test-outgoing-http', + 'http.url': 'http://localhost:3030/test-outgoing-http', + 'http.user_agent': 'axios/1.6.7', + 'net.host.ip': '::1', + 'net.host.name': 'localhost', + 'net.host.port': 3030, + 'net.peer.ip': '::1', + 'net.peer.port': 35152, + 'net.transport': 'ip_tcp', + 'otel.kind': 'SERVER', 'sentry.op': 'http.server', 'sentry.origin': 'auto.http.otel.http', + 'sentry.sample_rate': 1, 'sentry.source': 'route', + url: 'http://localhost:3030/test-outgoing-http', }, op: 'http.server', span_id: expect.any(String), @@ -159,12 +176,29 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { contexts: expect.objectContaining({ trace: { data: { - url: 'http://localhost:3030/test-outgoing-fetch', - 'otel.kind': 'SERVER', + 'http.flavor': '1.1', + 'http.host': 'localhost:3030', + 'http.method': 'GET', 'http.response.status_code': 200, + 'http.route': '/test-outgoing-fetch', + 'http.scheme': 'http', + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.target': '/test-outgoing-fetch', + 'http.url': 'http://localhost:3030/test-outgoing-fetch', + 'http.user_agent': 'axios/1.6.7', + 'net.host.ip': '::1', + 'net.host.name': 'localhost', + 'net.host.port': 3030, + 'net.peer.ip': '::1', + 'net.peer.port': 40084, + 'net.transport': 'ip_tcp', + 'otel.kind': 'SERVER', 'sentry.op': 'http.server', 'sentry.origin': 'auto.http.otel.http', + 'sentry.sample_rate': 1, 'sentry.source': 'route', + url: 'http://localhost:3030/test-outgoing-fetch', }, op: 'http.server', span_id: expect.any(String), From 0796c63b874916b2f9031aac0cf94a98c020c557 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 26 Feb 2024 16:18:18 +0000 Subject: [PATCH 04/10] fix tests?? --- .../tests/propagation.test.ts | 277 +++++++++--------- .../tests/transactions.test.ts | 59 ++-- 2 files changed, 178 insertions(+), 158 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index 0317b9eedd03..8a11ddab62d2 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -3,11 +3,6 @@ import { Span } from '@sentry/types'; import axios from 'axios'; import { waitForTransaction } from '../event-proxy-server'; -const authToken = process.env.E2E_TEST_AUTH_TOKEN; -const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; -const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; -const EVENT_POLLING_TIMEOUT = 90_000; - test('Propagates trace for outgoing http requests', async ({ baseURL }) => { const inboundTransactionPromise = waitForTransaction('node-experimental-fastify-app', transactionEvent => { return ( @@ -55,73 +50,77 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { ]), ); - expect(outboundTransaction).toEqual( - expect.objectContaining({ - contexts: expect.objectContaining({ - trace: { - data: { - 'http.flavor': '1.1', - 'http.host': 'localhost:3030', - 'http.method': 'GET', - 'http.response.status_code': 200, - 'http.route': '/test-outgoing-http', - 'http.scheme': 'http', - 'http.status_code': 200, - 'http.status_text': 'OK', - 'http.target': '/test-outgoing-http', - 'http.url': 'http://localhost:3030/test-outgoing-http', - 'http.user_agent': 'axios/1.6.7', - 'net.host.ip': '::1', - 'net.host.name': 'localhost', - 'net.host.port': 3030, - 'net.peer.ip': '::1', - 'net.peer.port': 35152, - 'net.transport': 'ip_tcp', - 'otel.kind': 'SERVER', - 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.otel.http', - 'sentry.sample_rate': 1, - 'sentry.source': 'route', - url: 'http://localhost:3030/test-outgoing-http', - }, - op: 'http.server', - span_id: expect.any(String), - status: 'ok', - tags: { - 'http.status_code': '200', - }, - trace_id: traceId, - origin: 'auto.http.otel.http', - }, - }), - }), - ); + expect(outboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-outgoing-http', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-outgoing-http', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-outgoing-http', + 'http.user_agent': 'axios/1.6.7', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': '127.0.0.1', + 'net.host.port': expect.any(Number), + 'net.peer.ip': '127.0.0.1', + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-outgoing-http', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + tags: { + 'http.status_code': '200', + }, + trace_id: traceId, + origin: 'auto.http.otel.http', + }); - expect(inboundTransaction).toEqual( - expect.objectContaining({ - contexts: expect.objectContaining({ - trace: { - data: { - url: 'http://localhost:3030/test-inbound-headers', - 'otel.kind': 'SERVER', - 'http.response.status_code': 200, - 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.otel.http', - 'sentry.source': 'route', - }, - op: 'http.server', - parent_span_id: outgoingHttpSpanId, - span_id: expect.any(String), - status: 'ok', - tags: { - 'http.status_code': '200', - }, - trace_id: traceId, - origin: 'auto.http.otel.http', - }, - }), - }), - ); + expect(inboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-inbound-headers', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-inbound-headers', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-inbound-headers', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': '127.0.0.1', + 'net.host.port': expect.any(Number), + 'net.peer.ip': '127.0.0.1', + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-inbound-headers', + }, + op: 'http.server', + parent_span_id: outgoingHttpSpanId, + span_id: expect.any(String), + status: 'ok', + tags: { + 'http.status_code': '200', + }, + trace_id: traceId, + origin: 'auto.http.otel.http', + }); }); test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { @@ -171,71 +170,75 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { ]), ); - expect(outboundTransaction).toEqual( - expect.objectContaining({ - contexts: expect.objectContaining({ - trace: { - data: { - 'http.flavor': '1.1', - 'http.host': 'localhost:3030', - 'http.method': 'GET', - 'http.response.status_code': 200, - 'http.route': '/test-outgoing-fetch', - 'http.scheme': 'http', - 'http.status_code': 200, - 'http.status_text': 'OK', - 'http.target': '/test-outgoing-fetch', - 'http.url': 'http://localhost:3030/test-outgoing-fetch', - 'http.user_agent': 'axios/1.6.7', - 'net.host.ip': '::1', - 'net.host.name': 'localhost', - 'net.host.port': 3030, - 'net.peer.ip': '::1', - 'net.peer.port': 40084, - 'net.transport': 'ip_tcp', - 'otel.kind': 'SERVER', - 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.otel.http', - 'sentry.sample_rate': 1, - 'sentry.source': 'route', - url: 'http://localhost:3030/test-outgoing-fetch', - }, - op: 'http.server', - span_id: expect.any(String), - status: 'ok', - tags: { - 'http.status_code': '200', - }, - trace_id: traceId, - origin: 'auto.http.otel.http', - }, - }), - }), - ); + expect(outboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-outgoing-fetch', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-outgoing-fetch', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-outgoing-http', + 'http.user_agent': 'axios/1.6.7', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': '127.0.0.1', + 'net.host.port': expect.any(Number), + 'net.peer.ip': '127.0.0.1', + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-outgoing-fetch', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + tags: { + 'http.status_code': '200', + }, + trace_id: traceId, + origin: 'auto.http.otel.http', + }); - expect(inboundTransaction).toEqual( - expect.objectContaining({ - contexts: expect.objectContaining({ - trace: { - data: { - url: 'http://localhost:3030/test-inbound-headers', - 'otel.kind': 'SERVER', - 'http.response.status_code': 200, - 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.otel.http', - 'sentry.source': 'route', - }, - op: 'http.server', - parent_span_id: outgoingHttpSpanId, - span_id: expect.any(String), - status: 'ok', - tags: { - 'http.status_code': '200', - }, - trace_id: traceId, - origin: 'auto.http.otel.http', - }, - }), - }), - ); + expect(inboundTransaction.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-inbound-headers', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-inbound-headers', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-inbound-headers', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': '127.0.0.1', + 'net.host.port': expect.any(Number), + 'net.peer.ip': '127.0.0.1', + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-inbound-headers', + }, + op: 'http.server', + parent_span_id: outgoingHttpSpanId, + span_id: expect.any(String), + status: 'ok', + tags: { + 'http.status_code': '200', + }, + trace_id: traceId, + origin: 'auto.http.otel.http', + }); }); diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts index 0429adca8623..9b5c8deaf987 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts @@ -20,29 +20,46 @@ test('Sends an API route transaction', async ({ baseURL }) => { const transactionEvent = await pageloadTransactionEventPromise; const transactionEventId = transactionEvent.event_id; + console.log(JSON.stringify(transactionEvent, null, 2)); + + expect(transactionEvent.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.http.otel.http', + 'sentry.op': 'http.server', + 'sentry.sample_rate': 1, + url: 'http://localhost:3030/test-transaction', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'http.url': 'http://localhost:3030/test-transaction', + 'http.host': 'localhost:3030', + 'net.host.name': 'localhost', + 'http.method': 'GET', + 'http.scheme': 'http', + 'http.target': '/test-transaction', + 'http.user_agent': 'axios/1.6.7', + 'http.flavor': '1.1', + 'net.transport': 'ip_tcp', + 'net.host.ip': '127.0.0.1', + 'net.host.port': expect.any(Number), + 'net.peer.ip': '127.0.0.1', + 'net.peer.port': expect.any(Number), + 'http.status_code': 200, + 'http.status_text': 'OK', + 'http.route': '/test-transaction', + }, + op: 'http.server', + span_id: expect.any(String), + status: 'ok', + tags: { + 'http.status_code': '200', + }, + trace_id: expect.any(String), + origin: 'auto.http.otel.http', + }); + expect(transactionEvent).toEqual( expect.objectContaining({ - contexts: expect.objectContaining({ - trace: { - data: { - url: 'http://localhost:3030/test-transaction', - 'otel.kind': 'SERVER', - 'http.response.status_code': 200, - 'sentry.op': 'http.server', - 'sentry.origin': 'auto.http.otel.http', - 'sentry.source': 'route', - }, - op: 'http.server', - span_id: expect.any(String), - status: 'ok', - tags: { - 'http.status_code': '200', - }, - trace_id: expect.any(String), - origin: 'auto.http.otel.http', - }, - }), - spans: [ { data: { From 6de768d550b7a9c48e0b3211cf01723db145590c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 27 Feb 2024 08:49:59 +0000 Subject: [PATCH 05/10] fix e2e test --- .../tests/propagation.test.ts | 18 +++++++++--------- .../tests/transactions.test.ts | 6 ++---- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index 8a11ddab62d2..a7e2f5a3eed8 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -68,9 +68,9 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'http.user_agent': 'axios/1.6.7', 'http.flavor': '1.1', 'net.transport': 'ip_tcp', - 'net.host.ip': '127.0.0.1', + 'net.host.ip': expect.any(String), 'net.host.port': expect.any(Number), - 'net.peer.ip': '127.0.0.1', + 'net.peer.ip': expect.any(String), 'net.peer.port': expect.any(Number), 'http.status_code': 200, 'http.status_text': 'OK', @@ -103,9 +103,9 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'http.target': '/test-inbound-headers', 'http.flavor': '1.1', 'net.transport': 'ip_tcp', - 'net.host.ip': '127.0.0.1', + 'net.host.ip': expect.any(String), 'net.host.port': expect.any(Number), - 'net.peer.ip': '127.0.0.1', + 'net.peer.ip': expect.any(String), 'net.peer.port': expect.any(Number), 'http.status_code': 200, 'http.status_text': 'OK', @@ -184,13 +184,13 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'net.host.name': 'localhost', 'http.method': 'GET', 'http.scheme': 'http', - 'http.target': '/test-outgoing-http', + 'http.target': '/test-outgoing-fetch', 'http.user_agent': 'axios/1.6.7', 'http.flavor': '1.1', 'net.transport': 'ip_tcp', - 'net.host.ip': '127.0.0.1', + 'net.host.ip': expect.any(String), 'net.host.port': expect.any(Number), - 'net.peer.ip': '127.0.0.1', + 'net.peer.ip': expect.any(String), 'net.peer.port': expect.any(Number), 'http.status_code': 200, 'http.status_text': 'OK', @@ -223,9 +223,9 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'http.target': '/test-inbound-headers', 'http.flavor': '1.1', 'net.transport': 'ip_tcp', - 'net.host.ip': '127.0.0.1', + 'net.host.ip': expect.any(String), 'net.host.port': expect.any(Number), - 'net.peer.ip': '127.0.0.1', + 'net.peer.ip': expect.any(String), 'net.peer.port': expect.any(Number), 'http.status_code': 200, 'http.status_text': 'OK', diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts index 9b5c8deaf987..827a6d417b27 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts @@ -20,8 +20,6 @@ test('Sends an API route transaction', async ({ baseURL }) => { const transactionEvent = await pageloadTransactionEventPromise; const transactionEventId = transactionEvent.event_id; - console.log(JSON.stringify(transactionEvent, null, 2)); - expect(transactionEvent.contexts?.trace).toEqual({ data: { 'sentry.source': 'route', @@ -40,9 +38,9 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'http.user_agent': 'axios/1.6.7', 'http.flavor': '1.1', 'net.transport': 'ip_tcp', - 'net.host.ip': '127.0.0.1', + 'net.host.ip': expect.any(String), 'net.host.port': expect.any(Number), - 'net.peer.ip': '127.0.0.1', + 'net.peer.ip': expect.any(String), 'net.peer.port': expect.any(Number), 'http.status_code': 200, 'http.status_text': 'OK', From c9dde6a0590926b1f77705801f2ec845d35edfb5 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 27 Feb 2024 09:30:51 +0000 Subject: [PATCH 06/10] fix e2e test for real? --- .../node-experimental-fastify-app/tests/propagation.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index a7e2f5a3eed8..787082f860b9 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -216,6 +216,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'http.url': 'http://localhost:3030/test-inbound-headers', + 'http.user_agent': 'undici', 'http.host': 'localhost:3030', 'net.host.name': 'localhost', 'http.method': 'GET', From a906b9a11ad5d57689b542b27191c4a647d5b6a9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 27 Feb 2024 11:10:28 +0000 Subject: [PATCH 07/10] rewrite method --- .../src/utils/dynamicSamplingContext.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry/src/utils/dynamicSamplingContext.ts b/packages/opentelemetry/src/utils/dynamicSamplingContext.ts index 4dfa8beb426b..cda72c11cac5 100644 --- a/packages/opentelemetry/src/utils/dynamicSamplingContext.ts +++ b/packages/opentelemetry/src/utils/dynamicSamplingContext.ts @@ -24,18 +24,18 @@ export function getDynamicSamplingContextFromSpan(span: AbstractSpan): Readonly< return {}; } - const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client); - const traceState = span.spanContext().traceState; const traceStateDsc = traceState?.get(SENTRY_TRACE_STATE_DSC); - if (traceStateDsc) { - const dsc = baggageHeaderToDynamicSamplingContext(traceStateDsc); - if (dsc) { - return dsc; - } + // If the span has a DSC, we want it to take precedence + const dscOnSpan = traceStateDsc ? baggageHeaderToDynamicSamplingContext(traceStateDsc) : undefined; + + if (dscOnSpan) { + return dscOnSpan; } + const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client); + const attributes = spanHasAttributes(span) ? span.attributes : {}; const sampleRate = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]; @@ -51,6 +51,7 @@ export function getDynamicSamplingContextFromSpan(span: AbstractSpan): Readonly< dsc.transaction = name; } + // TODO: Once we aligned span types, use spanIsSampled() from core instead // eslint-disable-next-line no-bitwise const sampled = Boolean(span.spanContext().traceFlags & TraceFlags.SAMPLED); dsc.sampled = String(sampled); From 39d43cc613d3fc520d51344e30d6be5dd61a1027 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 27 Feb 2024 12:04:32 +0000 Subject: [PATCH 08/10] ensure trace state is picked up form spans & add tests --- packages/opentelemetry/src/propagator.ts | 27 ++++++- packages/opentelemetry/src/spanExporter.ts | 11 +-- .../test/integration/transactions.test.ts | 70 ++++++++++++++++++- .../opentelemetry/test/propagator.test.ts | 31 +++++++- 4 files changed, 129 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 852de15eb3a3..903f9cc0585f 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -5,6 +5,7 @@ import { getClient, getDynamicSamplingContextFromClient } from '@sentry/core'; import type { DynamicSamplingContext, PropagationContext } from '@sentry/types'; import { SENTRY_BAGGAGE_KEY_PREFIX, + baggageHeaderToDynamicSamplingContext, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, propagationContextFromHeaders, @@ -13,6 +14,27 @@ import { import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER, SENTRY_TRACE_STATE_DSC } from './constants'; import { getPropagationContextFromContext, setPropagationContextOnContext } from './utils/contextData'; +function getDynamicSamplingContextFromContext(context: Context): Partial | undefined { + // If possible, we want to take the DSC from the active span + // That should take precedence over the DSC from the propagation context + const activeSpan = trace.getSpan(context); + const traceStateDsc = activeSpan?.spanContext().traceState?.get(SENTRY_TRACE_STATE_DSC); + const dscOnSpan = traceStateDsc ? baggageHeaderToDynamicSamplingContext(traceStateDsc) : undefined; + + if (dscOnSpan) { + return dscOnSpan; + } + + const propagationContext = getPropagationContextFromContext(context); + + if (propagationContext) { + const { traceId } = getSentryTraceData(context, propagationContext); + return getDynamicSamplingContext(propagationContext, traceId); + } + + return undefined; +} + /** * Injects and extracts `sentry-trace` and `baggage` headers from carriers. */ @@ -29,9 +51,8 @@ export class SentryPropagator extends W3CBaggagePropagator { const propagationContext = getPropagationContextFromContext(context); const { spanId, traceId, sampled } = getSentryTraceData(context, propagationContext); - const dynamicSamplingContext = propagationContext - ? getDynamicSamplingContext(propagationContext, traceId) - : undefined; + + const dynamicSamplingContext = getDynamicSamplingContextFromContext(context); if (dynamicSamplingContext) { baggage = Object.entries(dynamicSamplingContext).reduce((b, [dscKey, dscValue]) => { diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index aadaa9a6ace0..d541b4eca591 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -5,7 +5,7 @@ import type { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base'; import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; import type { Transaction } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, flush, getCurrentHub } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, getCurrentHub } from '@sentry/core'; import type { Scope, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types'; import { addNonEnumerableProperty, dropUndefinedKeys, logger } from '@sentry/utils'; import { startTransaction } from './custom/transaction'; @@ -74,14 +74,17 @@ export class SentrySpanExporter implements SpanExporter { /** @inheritDoc */ public shutdown(): Promise { + const forceFlush = this.forceFlush(); this._stopped = true; this._finishedSpans = []; - return this.forceFlush(); + return forceFlush; } /** @inheritDoc */ - public async forceFlush(): Promise { - await flush(); + public forceFlush(): Promise { + return new Promise(resolve => { + this.export(this._finishedSpans, () => resolve()); + }); } } diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 9377ca1bc4c7..01ac760ff927 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -1,10 +1,13 @@ +import type { SpanContext } from '@opentelemetry/api'; import { TraceFlags, context, trace } from '@opentelemetry/api'; import type { SpanProcessor } from '@opentelemetry/sdk-trace-base'; import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addBreadcrumb, getClient, setTag, withIsolationScope } from '@sentry/core'; -import type { PropagationContext, TransactionEvent } from '@sentry/types'; +import type { Event, PropagationContext, TransactionEvent } from '@sentry/types'; import { logger } from '@sentry/utils'; +import { TraceState } from '@opentelemetry/core'; import { spanToJSON } from '@sentry/core'; +import { SENTRY_TRACE_STATE_DSC } from '../../src/constants'; import { SentrySpanProcessor } from '../../src/spanProcessor'; import { startInactiveSpan, startSpan } from '../../src/trace'; import { setPropagationContextOnContext } from '../../src/utils/contextData'; @@ -14,6 +17,7 @@ import { cleanupOtel, getProvider, mockSdkInit } from '../helpers/mockSdkInit'; describe('Integration | Transactions', () => { afterEach(() => { jest.restoreAllMocks(); + jest.useRealTimers(); cleanupOtel(); }); @@ -515,4 +519,68 @@ describe('Integration | Transactions', () => { ]), ); }); + + it('uses & inherits DSC on span trace state', async () => { + const transactionEvents: Event[] = []; + const beforeSendTransaction = jest.fn(event => { + transactionEvents.push(event); + return null; + }); + + const traceId = 'd4cda95b652f4a1592b449d5929fda1b'; + const parentSpanId = '6e0c63257de34c92'; + + const dscString = `sentry-transaction=other-transaction,sentry-environment=other,sentry-release=8.0.0,sentry-public_key=public,sentry-trace_id=${traceId},sentry-sampled=true`; + + const spanContext: SpanContext = { + traceId, + spanId: parentSpanId, + isRemote: true, + traceFlags: TraceFlags.SAMPLED, + traceState: new TraceState().set(SENTRY_TRACE_STATE_DSC, dscString), + }; + + const propagationContext: PropagationContext = { + traceId, + parentSpanId, + spanId: '6e0c63257de34c93', + sampled: true, + }; + + mockSdkInit({ enableTracing: true, beforeSendTransaction }); + + const client = getClient() as TestClientInterface; + + // We simulate the correct context we'd normally get from the SentryPropagator + context.with( + trace.setSpanContext(setPropagationContextOnContext(context.active(), propagationContext), spanContext), + () => { + startSpan({ op: 'test op', name: 'test name', source: 'task', origin: 'auto.test' }, span => { + expect(span.spanContext().traceState?.get(SENTRY_TRACE_STATE_DSC)).toEqual(dscString); + + const subSpan = startInactiveSpan({ name: 'inner span 1' }); + + expect(subSpan.spanContext().traceState?.get(SENTRY_TRACE_STATE_DSC)).toEqual(dscString); + + subSpan.end(); + + startSpan({ name: 'inner span 2' }, subSpan => { + expect(subSpan.spanContext().traceState?.get(SENTRY_TRACE_STATE_DSC)).toEqual(dscString); + }); + }); + }, + ); + + await client.flush(); + + expect(transactionEvents).toHaveLength(1); + expect(transactionEvents[0]?.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({ + environment: 'other', + public_key: 'public', + release: '8.0.0', + sampled: 'true', + trace_id: traceId, + transaction: 'other-transaction', + }); + }); }); diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index 4129b6cfd0e0..27436117d885 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -6,11 +6,11 @@ import { propagation, trace, } from '@opentelemetry/api'; -import { suppressTracing } from '@opentelemetry/core'; +import { TraceState, suppressTracing } from '@opentelemetry/core'; import { addTracingExtensions, setCurrentClient } from '@sentry/core'; import type { Client, PropagationContext } from '@sentry/types'; -import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER } from '../src/constants'; +import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER, SENTRY_TRACE_STATE_DSC } from '../src/constants'; import { SentryPropagator } from '../src/propagator'; import { getPropagationContextFromContext, setPropagationContextOnContext } from '../src/utils/contextData'; @@ -77,6 +77,33 @@ describe('SentryPropagator', () => { ], 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1', ], + [ + 'works with a DSC on the span trace state', + { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.SAMPLED, + traceState: new TraceState().set( + SENTRY_TRACE_STATE_DSC, + 'sentry-transaction=other-transaction,sentry-environment=other,sentry-release=8.0.0,sentry-public_key=public,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-sampled=true', + ), + }, + { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c94', + parentSpanId: '6e0c63257de34c93', + sampled: true, + }, + [ + 'sentry-environment=other', + 'sentry-release=8.0.0', + 'sentry-public_key=public', + 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', + 'sentry-transaction=other-transaction', + 'sentry-sampled=true', + ], + 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1', + ], [ 'works with an unsampled propagation context', { From 87906263ec0f46fbd1e0e63e06e3250dccd90584 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 27 Feb 2024 12:31:09 +0000 Subject: [PATCH 09/10] unflake test... --- .../node-experimental-fastify-app/tests/propagation.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index 787082f860b9..e4ff4ec8f9fe 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -207,7 +207,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { }); expect(inboundTransaction.contexts?.trace).toEqual({ - data: { + data: expect.objectContaining({ 'sentry.source': 'route', 'sentry.origin': 'auto.http.otel.http', 'sentry.op': 'http.server', @@ -216,7 +216,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'http.url': 'http://localhost:3030/test-inbound-headers', - 'http.user_agent': 'undici', 'http.host': 'localhost:3030', 'net.host.name': 'localhost', 'http.method': 'GET', @@ -231,7 +230,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'http.status_code': 200, 'http.status_text': 'OK', 'http.route': '/test-inbound-headers', - }, + }), op: 'http.server', parent_span_id: outgoingHttpSpanId, span_id: expect.any(String), From 3073700897d945216b01facad5b67c6773188ead Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 27 Feb 2024 15:42:45 +0000 Subject: [PATCH 10/10] fix e2e test --- .../tests/propagation.test.ts | 12 ------------ .../tests/transactions.test.ts | 3 --- 2 files changed, 15 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts index e4ff4ec8f9fe..0afc4ff09b06 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts @@ -79,9 +79,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { op: 'http.server', span_id: expect.any(String), status: 'ok', - tags: { - 'http.status_code': '200', - }, trace_id: traceId, origin: 'auto.http.otel.http', }); @@ -115,9 +112,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { parent_span_id: outgoingHttpSpanId, span_id: expect.any(String), status: 'ok', - tags: { - 'http.status_code': '200', - }, trace_id: traceId, origin: 'auto.http.otel.http', }); @@ -199,9 +193,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { op: 'http.server', span_id: expect.any(String), status: 'ok', - tags: { - 'http.status_code': '200', - }, trace_id: traceId, origin: 'auto.http.otel.http', }); @@ -235,9 +226,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { parent_span_id: outgoingHttpSpanId, span_id: expect.any(String), status: 'ok', - tags: { - 'http.status_code': '200', - }, trace_id: traceId, origin: 'auto.http.otel.http', }); diff --git a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts index e8f2c6762254..4ff9b5df632c 100644 --- a/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts @@ -49,9 +49,6 @@ test('Sends an API route transaction', async ({ baseURL }) => { op: 'http.server', span_id: expect.any(String), status: 'ok', - tags: { - 'http.status_code': '200', - }, trace_id: expect.any(String), origin: 'auto.http.otel.http', });