From 1d77692693f936ad391e7a8ce2e4b4b4b64ddd9a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 10 Dec 2024 10:16:24 +0100 Subject: [PATCH 1/3] feat(opentelemetry): Set `response` context for http.server spans --- .../core/test/lib/tracing/sentrySpan.test.ts | 88 ++++++++++---- packages/opentelemetry/src/spanExporter.ts | 23 +++- .../opentelemetry/test/spanExporter.test.ts | 111 ++++++++++++++++++ 3 files changed, 193 insertions(+), 29 deletions(-) create mode 100644 packages/opentelemetry/test/spanExporter.test.ts diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts index 9a1c9f69255c..9b2cad83f2e1 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -1,4 +1,4 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, setCurrentClient, timestampInSeconds } from '../../../src'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getCurrentScope, setCurrentClient, timestampInSeconds } from '../../../src'; import { SentrySpan } from '../../../src/tracing/sentrySpan'; import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus'; import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON } from '../../../src/utils/spanUtils'; @@ -95,7 +95,7 @@ describe('SentrySpan', () => { }); }); - describe('finish', () => { + describe('end', () => { test('simple', () => { const span = new SentrySpan({}); expect(spanToJSON(span).timestamp).toBeUndefined(); @@ -103,6 +103,22 @@ describe('SentrySpan', () => { expect(spanToJSON(span).timestamp).toBeGreaterThan(1); }); + test('with endTime in seconds', () => { + const span = new SentrySpan({}); + expect(spanToJSON(span).timestamp).toBeUndefined(); + const endTime = Date.now() / 1000; + span.end(endTime); + expect(spanToJSON(span).timestamp).toBe(endTime); + }); + + test('with endTime in milliseconds', () => { + const span = new SentrySpan({}); + expect(spanToJSON(span).timestamp).toBeUndefined(); + const endTime = Date.now(); + span.end(endTime); + expect(spanToJSON(span).timestamp).toBe(endTime / 1000); + }); + test('uses sampled config for standalone span', () => { const client = new TestClient( getDefaultTestClientOptions({ @@ -136,7 +152,7 @@ describe('SentrySpan', () => { expect(mockSend).toHaveBeenCalledTimes(1); }); - test('sends the span if `beforeSendSpan` does not modify the span ', () => { + test('sends the span if `beforeSendSpan` does not modify the span', () => { const beforeSendSpan = jest.fn(span => span); const client = new TestClient( getDefaultTestClientOptions({ @@ -194,30 +210,54 @@ describe('SentrySpan', () => { ); consoleWarnSpy.mockRestore(); }); - }); - describe('end', () => { - test('simple', () => { - const span = new SentrySpan({}); - expect(spanToJSON(span).timestamp).toBeUndefined(); - span.end(); - expect(spanToJSON(span).timestamp).toBeGreaterThan(1); - }); + test('build TransactionEvent for basic root span', () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://username@domain/123', + }), + ); + setCurrentClient(client); - test('with endTime in seconds', () => { - const span = new SentrySpan({}); - expect(spanToJSON(span).timestamp).toBeUndefined(); - const endTime = Date.now() / 1000; - span.end(endTime); - expect(spanToJSON(span).timestamp).toBe(endTime); - }); + const scope = getCurrentScope(); + const captureEventSpy = jest.spyOn(scope, 'captureEvent').mockImplementation(() => 'testId'); - test('with endTime in milliseconds', () => { - const span = new SentrySpan({}); - expect(spanToJSON(span).timestamp).toBeUndefined(); - const endTime = Date.now(); - span.end(endTime); - expect(spanToJSON(span).timestamp).toBe(endTime / 1000); + const span = new SentrySpan({ + name: 'test', + startTimestamp: 1, + sampled: true, + }); + span.end(2); + + expect(captureEventSpy).toHaveBeenCalledTimes(1); + expect(captureEventSpy).toHaveBeenCalledWith({ + _metrics_summary: undefined, + contexts: { + trace: { + data: { + 'sentry.origin': 'manual', + }, + origin: 'manual', + span_id: expect.stringMatching(/^[a-f0-9]{16}$/), + trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), + }, + }, + sdkProcessingMetadata: { + capturedSpanIsolationScope: undefined, + capturedSpanScope: undefined, + dynamicSamplingContext: { + environment: 'production', + public_key: 'username', + trace_id: expect.stringMatching(/^[a-f0-9]{32}$/), + transaction: 'test', + }, + }, + spans: [], + start_timestamp: 1, + timestamp: 2, + transaction: 'test', + type: 'transaction', + }); }); }); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index ad044372e0df..0d836d060159 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -1,8 +1,16 @@ +/* eslint-disable max-lines */ import type { Span } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import { ATTR_HTTP_RESPONSE_STATUS_CODE, SEMATTRS_HTTP_STATUS_CODE } from '@opentelemetry/semantic-conventions'; -import type { SpanJSON, SpanOrigin, TraceContext, TransactionEvent, TransactionSource } from '@sentry/core'; +import type { + SpanAttributes, + SpanJSON, + SpanOrigin, + TraceContext, + TransactionEvent, + TransactionSource, +} from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -221,13 +229,14 @@ function parseSpan(span: ReadableSpan): { op?: string; origin?: SpanOrigin; sour return { origin, op, source }; } -function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { +/** Exported only for tests. */ +export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { const { op, description, data, origin = 'manual', source } = getSpanData(span); const capturedSpanScopes = getCapturedScopesOnSpan(span as unknown as Span); const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined; - const attributes = dropUndefinedKeys({ + const attributes: SpanAttributes = dropUndefinedKeys({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate, [SEMANTIC_ATTRIBUTE_SENTRY_OP]: op, @@ -257,12 +266,16 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined }); - const transactionEvent: TransactionEvent = { + const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]; + const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined; + + const transactionEvent: TransactionEvent = dropUndefinedKeys({ contexts: { trace: traceContext, otel: { resource: span.resource.attributes, }, + ...responseContext, }, spans: [], start_timestamp: spanTimeInputToSeconds(span.startTime), @@ -283,7 +296,7 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { }, }), _metrics_summary: getMetricSummaryJsonForSpan(span as unknown as Span), - }; + }); return transactionEvent; } diff --git a/packages/opentelemetry/test/spanExporter.test.ts b/packages/opentelemetry/test/spanExporter.test.ts new file mode 100644 index 000000000000..184d89d6f645 --- /dev/null +++ b/packages/opentelemetry/test/spanExporter.test.ts @@ -0,0 +1,111 @@ +import { ATTR_HTTP_RESPONSE_STATUS_CODE } from '@opentelemetry/semantic-conventions/*'; +import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, startInactiveSpan } from '@sentry/core'; +import { createTransactionForOtelSpan } from '../src/spanExporter'; +import { cleanupOtel, mockSdkInit } from './helpers/mockSdkInit'; + +describe('createTransactionForOtelSpan', () => { + beforeEach(() => { + mockSdkInit({ + enableTracing: true, + }); + }); + + afterEach(() => { + cleanupOtel(); + }); + + it('works with a basic span', () => { + const span = startInactiveSpan({ name: 'test', startTime: 1733821670000 }); + span.end(1733821672000); + + const event = createTransactionForOtelSpan(span as any); + // we do not care about this here + delete event.sdkProcessingMetadata; + + expect(event).toEqual({ + contexts: { + trace: { + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + }, + origin: 'manual', + status: 'ok', + }, + otel: { + resource: { + 'service.name': 'opentelemetry-test', + 'telemetry.sdk.language': 'nodejs', + 'telemetry.sdk.name': 'opentelemetry', + 'telemetry.sdk.version': expect.any(String), + 'service.namespace': 'sentry', + 'service.version': SDK_VERSION, + }, + }, + }, + spans: [], + start_timestamp: 1733821670, + timestamp: 1733821672, + transaction: 'test', + type: 'transaction', + transaction_info: { source: 'custom' }, + }); + }); + + it('works with a http.server span', () => { + const span = startInactiveSpan({ + name: 'test', + startTime: 1733821670000, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200, + }, + }); + span.end(1733821672000); + + const event = createTransactionForOtelSpan(span as any); + // we do not care about this here + delete event.sdkProcessingMetadata; + + expect(event).toEqual({ + contexts: { + trace: { + span_id: expect.stringMatching(/[a-f0-9]{16}/), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + 'sentry.op': 'http.server', + 'http.response.status_code': 200, + }, + origin: 'manual', + status: 'ok', + op: 'http.server', + }, + otel: { + resource: { + 'service.name': 'opentelemetry-test', + 'telemetry.sdk.language': 'nodejs', + 'telemetry.sdk.name': 'opentelemetry', + 'telemetry.sdk.version': expect.any(String), + 'service.namespace': 'sentry', + 'service.version': SDK_VERSION, + }, + }, + response: { + status_code: 200, + }, + }, + spans: [], + start_timestamp: 1733821670, + timestamp: 1733821672, + transaction: 'test', + type: 'transaction', + transaction_info: { source: 'custom' }, + }); + }); +}); From f12f5c642656e019724d3a2e61a6321ed0f847b3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 10 Dec 2024 10:51:41 +0100 Subject: [PATCH 2/3] fix test --- packages/opentelemetry/test/spanExporter.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry/test/spanExporter.test.ts b/packages/opentelemetry/test/spanExporter.test.ts index 184d89d6f645..737171da8908 100644 --- a/packages/opentelemetry/test/spanExporter.test.ts +++ b/packages/opentelemetry/test/spanExporter.test.ts @@ -1,4 +1,4 @@ -import { ATTR_HTTP_RESPONSE_STATUS_CODE } from '@opentelemetry/semantic-conventions/*'; +import { ATTR_HTTP_RESPONSE_STATUS_CODE } from '@opentelemetry/semantic-conventions'; import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, startInactiveSpan } from '@sentry/core'; import { createTransactionForOtelSpan } from '../src/spanExporter'; import { cleanupOtel, mockSdkInit } from './helpers/mockSdkInit'; From af1dae2e9f20846633d0717371e0f8983acf3899 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 10 Dec 2024 10:53:51 +0100 Subject: [PATCH 3/3] add some tests --- .../test-applications/nestjs-8/tests/transactions.test.ts | 4 ++++ .../test-applications/node-express/tests/transactions.test.ts | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts index cab90847df50..be0e03cdbc97 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts @@ -46,6 +46,10 @@ test('Sends an API route transaction', async ({ baseURL }) => { origin: 'auto.http.otel.http', }); + expect(transactionEvent.contexts?.response).toEqual({ + status_code: 200, + }); + expect(transactionEvent).toEqual( expect.objectContaining({ spans: expect.arrayContaining([ diff --git a/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts index 3b51ae415b03..7f9b18b4cc50 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts @@ -46,6 +46,10 @@ test('Sends an API route transaction', async ({ baseURL }) => { origin: 'auto.http.otel.http', }); + expect(transactionEvent.contexts?.response).toEqual({ + status_code: 200, + }); + expect(transactionEvent).toEqual( expect.objectContaining({ transaction: 'GET /test-transaction',