From 92da34b8b8b7513289288da543fb4bc45484a809 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 11 Feb 2025 17:05:15 +0100 Subject: [PATCH 01/11] feat(core): Create types and utilities for span links --- packages/core/src/types-hoist/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/types-hoist/index.ts b/packages/core/src/types-hoist/index.ts index c1cbe5284808..48b38926e85f 100644 --- a/packages/core/src/types-hoist/index.ts +++ b/packages/core/src/types-hoist/index.ts @@ -113,6 +113,7 @@ export type { SpanContextData, TraceFlag, } from './span'; +export type { SpanLink, SpanLinkJSON } from './link'; export type { SpanStatus } from './spanStatus'; export type { TimedEvent } from './timedEvent'; export type { StackFrame } from './stackframe'; From ebfafa821f45397bfa7563b76f8a275a142adbd7 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 10:46:27 +0100 Subject: [PATCH 02/11] review suggestions --- packages/core/src/types-hoist/index.ts | 1 - packages/core/src/types-hoist/span.ts | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/core/src/types-hoist/index.ts b/packages/core/src/types-hoist/index.ts index 48b38926e85f..c1cbe5284808 100644 --- a/packages/core/src/types-hoist/index.ts +++ b/packages/core/src/types-hoist/index.ts @@ -113,7 +113,6 @@ export type { SpanContextData, TraceFlag, } from './span'; -export type { SpanLink, SpanLinkJSON } from './link'; export type { SpanStatus } from './spanStatus'; export type { TimedEvent } from './timedEvent'; export type { StackFrame } from './stackframe'; diff --git a/packages/core/src/types-hoist/span.ts b/packages/core/src/types-hoist/span.ts index 2b82aab74934..0945da7b04fe 100644 --- a/packages/core/src/types-hoist/span.ts +++ b/packages/core/src/types-hoist/span.ts @@ -257,7 +257,7 @@ export interface Span { * Prefer setting links directly when starting a span (e.g. `Sentry.startSpan()`) as some context information is only available during span creation. * @param link - The link containing the context of the span to link to and optional attributes */ - addLink(link: SpanLink): this; + addLink(link: SpanLink | unknown): this; /** * Associates this span with multiple related spans. See {@link addLink} for more details. @@ -265,7 +265,7 @@ export interface Span { * Prefer setting links directly when starting a span (e.g. `Sentry.startSpan()`) as some context information is only available during span creation. * @param links - Array of links to associate with this span */ - addLinks(links: SpanLink[]): this; + addLinks(links: SpanLink[] | unknown): this; /** * NOT USED IN SENTRY, only added for compliance with OTEL Span interface From 6736918b22ed70561bb88320413188b3195d3be0 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 11:00:35 +0100 Subject: [PATCH 03/11] remove unknown typing --- packages/core/src/types-hoist/span.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/types-hoist/span.ts b/packages/core/src/types-hoist/span.ts index 0945da7b04fe..2b82aab74934 100644 --- a/packages/core/src/types-hoist/span.ts +++ b/packages/core/src/types-hoist/span.ts @@ -257,7 +257,7 @@ export interface Span { * Prefer setting links directly when starting a span (e.g. `Sentry.startSpan()`) as some context information is only available during span creation. * @param link - The link containing the context of the span to link to and optional attributes */ - addLink(link: SpanLink | unknown): this; + addLink(link: SpanLink): this; /** * Associates this span with multiple related spans. See {@link addLink} for more details. @@ -265,7 +265,7 @@ export interface Span { * Prefer setting links directly when starting a span (e.g. `Sentry.startSpan()`) as some context information is only available during span creation. * @param links - Array of links to associate with this span */ - addLinks(links: SpanLink[] | unknown): this; + addLinks(links: SpanLink[]): this; /** * NOT USED IN SENTRY, only added for compliance with OTEL Span interface From 3ab4aa8b96bfb20b8de3ee9c0fe7b7a5cb88820c Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 12 Feb 2025 14:27:16 +0100 Subject: [PATCH 04/11] feat(node): Add `addLink(s)` to span --- .../tracing/linking/scenario-addLink.ts | 33 ++++++++ .../tracing/linking/scenario-addLinks.ts | 31 ++++++++ .../suites/tracing/linking/test.ts | 75 +++++++++++++++++++ packages/core/src/utils/spanUtils.ts | 4 +- packages/core/test/lib/tracing/trace.test.ts | 69 +++++++++++++++++ packages/opentelemetry/src/spanExporter.ts | 4 +- packages/opentelemetry/test/trace.test.ts | 70 ++++++++++++++++- 7 files changed, 283 insertions(+), 3 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/linking/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts new file mode 100644 index 000000000000..27282ffb2867 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts @@ -0,0 +1,33 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [], + transport: loggingTransport, +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'parent1' }, async parentSpan1 => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child1.1' }, async childSpan1 => { + childSpan1.addLink({ + context: parentSpan1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }); + + childSpan1.end(); + }); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child1.2' }, async childSpan2 => { + childSpan2.addLink({ + context: parentSpan1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }); + + childSpan2.end(); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts new file mode 100644 index 000000000000..216beff5c87e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts @@ -0,0 +1,31 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [], + transport: loggingTransport, +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'parent1' }, async parentSpan1 => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child1.1' }, async childSpan1 => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child2.1' }, async childSpan2 => { + childSpan2.addLinks([ + { context: parentSpan1.spanContext() }, + { + context: childSpan1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }, + ]); + + childSpan2.end(); + }); + + childSpan1.end(); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts new file mode 100644 index 000000000000..601e6c77c38b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts @@ -0,0 +1,75 @@ +import { createRunner } from '../../../utils/runner'; + +// A general note regarding this test: +// The fact that the trace_id and span_id are correctly linked is tested in a unit test + +describe('span links', () => { + test('should link spans with addLink()', done => { + createRunner(__dirname, 'scenario-addLink.ts') + .expect({ + transaction: { + transaction: 'parent1', + spans: [ + expect.objectContaining({ + description: 'child1.1', + links: [ + expect.objectContaining({ + trace_id: expect.any(String), + span_id: expect.any(String), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), + }), + ], + }), + expect.objectContaining({ + description: 'child1.2', + links: [ + expect.objectContaining({ + trace_id: expect.any(String), + span_id: expect.any(String), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), + }), + ], + }), + ], + }, + }) + .start(done); + }); + + test('should link spans with addLinks()', done => { + createRunner(__dirname, 'scenario-addLinks.ts') + .expect({ + transaction: { + transaction: 'parent1', + spans: [ + expect.objectContaining({ + description: 'child1.1', + links: [], + }), + expect.objectContaining({ + description: 'child2.1', + links: [ + expect.not.objectContaining({ attributes: expect.anything() }) && + expect.objectContaining({ + trace_id: expect.any(String), + span_id: expect.any(String), + }), + expect.objectContaining({ + trace_id: expect.any(String), + span_id: expect.any(String), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), + }), + ], + }), + ], + }, + }) + .start(done); + }); +}); diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index fcf4aa1857e3..56a24ef5ae5c 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -144,7 +144,7 @@ export function spanToJSON(span: Span): SpanJSON { // Handle a span from @opentelemetry/sdk-base-trace's `Span` class if (spanIsOpenTelemetrySdkTraceBaseSpan(span)) { - const { attributes, startTime, name, endTime, parentSpanId, status } = span; + const { attributes, startTime, name, endTime, parentSpanId, status, links } = span; return dropUndefinedKeys({ span_id, @@ -158,6 +158,7 @@ export function spanToJSON(span: Span): SpanJSON { status: getStatusMessage(status), op: attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP], origin: attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, + links: links ? convertSpanLinksForEnvelope(links) : undefined, }); } @@ -184,6 +185,7 @@ export interface OpenTelemetrySdkTraceBaseSpan extends Span { status: SpanStatus; endTime: SpanTimeInput; parentSpanId?: string; + links?: SpanLink[]; } /** diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index c33b50c01a85..9ea4f21cbee1 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -1,3 +1,4 @@ +import type { SpanLink } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -399,6 +400,40 @@ describe('startSpan', () => { }); }); + it('allows to pass span links', () => { + const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); + + // @ts-expect-error links exists on span + expect(rawSpan1?.links).toEqual([]); + + const span1JSON = spanToJSON(rawSpan1); + + startSpan({ name: '/users/:id' }, rawSpan2 => { + rawSpan2.addLink({ + context: rawSpan1.spanContext(), + attributes: { + 'sentry.link.type': 'previous_trace', + }, + }); + + const span2LinkJSON = spanToJSON(rawSpan2).links?.[0]; + + expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); + expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); + expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); + }); + }); + it('allows to force a transaction with forceTransaction=true', async () => { const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); client = new TestClient(options); @@ -900,6 +935,40 @@ describe('startSpanManual', () => { }); }); + it('allows to pass span links', () => { + const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); + + // @ts-expect-error links exists on span + expect(rawSpan1?.links).toEqual([]); + + const span1JSON = spanToJSON(rawSpan1); + + startSpanManual({ name: '/users/:id' }, rawSpan2 => { + rawSpan2.addLink({ + context: rawSpan1.spanContext(), + attributes: { + 'sentry.link.type': 'previous_trace', + }, + }); + + const span2LinkJSON = spanToJSON(rawSpan2).links?.[0]; + + expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); + expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); + expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); + }); + }); + it('allows to force a transaction with forceTransaction=true', async () => { const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); client = new TestClient(options); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index c6a838a5574f..2a0ea4854b86 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -11,6 +11,7 @@ import type { TransactionEvent, TransactionSource, } from '@sentry/core'; +import { convertSpanLinksForEnvelope } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_OP, @@ -322,7 +323,7 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS const span_id = span.spanContext().spanId; const trace_id = span.spanContext().traceId; - const { attributes, startTime, endTime, parentSpanId } = span; + const { attributes, startTime, endTime, parentSpanId, links } = span; const { op, description, data, origin = 'manual' } = getSpanData(span); const allData = dropUndefinedKeys({ @@ -347,6 +348,7 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS op, origin, measurements: timedEventsToMeasurements(span.events), + links: links ? convertSpanLinksForEnvelope(links) : undefined, }); spans.push(spanJSON); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 184b93b1e71b..d1bd223f0cd0 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -20,7 +20,7 @@ import { suppressTracing, withScope, } from '@sentry/core'; -import type { Event, Scope } from '@sentry/core'; +import type { Event, Scope, SpanLink } from '@sentry/core'; import { SEMATTRS_HTTP_METHOD } from '@opentelemetry/semantic-conventions'; import { continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../src/trace'; @@ -356,6 +356,40 @@ describe('trace', () => { }); }); + it('allows to pass span links', () => { + const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); + + // @ts-expect-error links exists on span + expect(rawSpan1?.links).toEqual([]); + + const span1JSON = spanToJSON(rawSpan1); + + startSpan({ name: '/users/:id' }, rawSpan2 => { + rawSpan2.addLink({ + context: rawSpan1.spanContext(), + attributes: { + 'sentry.link.type': 'previous_trace', + }, + }); + + const span2LinkJSON = spanToJSON(rawSpan2).links?.[0]; + + expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); + expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); + expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); + }); + }); + it('allows to force a transaction with forceTransaction=true', async () => { const client = getClient()!; const transactionEvents: Event[] = []; @@ -906,6 +940,40 @@ describe('trace', () => { }); }); + it('allows to pass span links', () => { + const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); + + // @ts-expect-error links exists on span + expect(rawSpan1?.links).toEqual([]); + + const span1JSON = spanToJSON(rawSpan1); + + startSpanManual({ name: '/users/:id' }, rawSpan2 => { + rawSpan2.addLink({ + context: rawSpan1.spanContext(), + attributes: { + 'sentry.link.type': 'previous_trace', + }, + }); + + const span2LinkJSON = spanToJSON(rawSpan2).links?.[0]; + + expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); + expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); + + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); + // @ts-expect-error links and _spanContext exist on span + expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); + expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); + }); + }); + it('allows to force a transaction with forceTransaction=true', async () => { const client = getClient()!; const transactionEvents: Event[] = []; From 5a74d31ecb8de5c06c03cdf4c6f99c8338203a1a Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 12 Feb 2025 15:40:58 +0100 Subject: [PATCH 05/11] return undefined if no links --- packages/core/src/utils/spanUtils.ts | 2 +- packages/opentelemetry/src/spanExporter.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 56a24ef5ae5c..d23a08a96808 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -158,7 +158,7 @@ export function spanToJSON(span: Span): SpanJSON { status: getStatusMessage(status), op: attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP], origin: attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, - links: links ? convertSpanLinksForEnvelope(links) : undefined, + links: convertSpanLinksForEnvelope(links), }); } diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 2a0ea4854b86..2f11b9bbfb7b 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -348,7 +348,7 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS op, origin, measurements: timedEventsToMeasurements(span.events), - links: links ? convertSpanLinksForEnvelope(links) : undefined, + links: convertSpanLinksForEnvelope(links), }); spans.push(spanJSON); From 84860b4ac6de31aa7dfa04ad41a254ece294a091 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 12 Feb 2025 17:08:07 +0100 Subject: [PATCH 06/11] fix tests -> undefined instead of [] --- packages/core/test/lib/tracing/trace.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 9ea4f21cbee1..97b9b0984ab8 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -404,7 +404,7 @@ describe('startSpan', () => { const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); // @ts-expect-error links exists on span - expect(rawSpan1?.links).toEqual([]); + expect(rawSpan1?.links).toEqual(undefined); const span1JSON = spanToJSON(rawSpan1); @@ -939,7 +939,7 @@ describe('startSpanManual', () => { const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); // @ts-expect-error links exists on span - expect(rawSpan1?.links).toEqual([]); + expect(rawSpan1?.links).toEqual(undefined); const span1JSON = spanToJSON(rawSpan1); @@ -1393,7 +1393,7 @@ describe('startInactiveSpan', () => { origin: 'manual', }, }); - expect(innerTransaction?.spans).toEqual([]); + expect(innerTransaction?.spans).toEqual(undefined); expect(innerTransaction?.transaction).toEqual('inner transaction'); expect(innerTransaction?.sdkProcessingMetadata).toEqual({ dynamicSamplingContext: { From 2fb2b9d7c6e7cff3ce08951b1d7783180c96e2e0 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 13:57:41 +0100 Subject: [PATCH 07/11] add link to trace context --- .../linking/scenario-addLink-nested.ts | 33 +++ .../tracing/linking/scenario-addLink.ts | 25 +-- .../linking/scenario-addLinks-nested.ts | 31 +++ .../tracing/linking/scenario-addLinks.ts | 29 ++- .../suites/tracing/linking/test.ts | 193 +++++++++++++----- packages/core/src/types-hoist/context.ts | 2 + packages/core/test/lib/tracing/trace.test.ts | 69 ------- packages/opentelemetry/src/spanExporter.ts | 2 + .../opentelemetry/test/spanExporter.test.ts | 29 ++- packages/opentelemetry/test/trace.test.ts | 18 +- 10 files changed, 265 insertions(+), 166 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink-nested.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks-nested.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink-nested.ts b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink-nested.ts new file mode 100644 index 000000000000..27282ffb2867 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink-nested.ts @@ -0,0 +1,33 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [], + transport: loggingTransport, +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'parent1' }, async parentSpan1 => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child1.1' }, async childSpan1 => { + childSpan1.addLink({ + context: parentSpan1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }); + + childSpan1.end(); + }); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child1.2' }, async childSpan2 => { + childSpan2.addLink({ + context: parentSpan1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }); + + childSpan2.end(); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts index 27282ffb2867..d00ae669dbd7 100644 --- a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts +++ b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLink.ts @@ -9,25 +9,12 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line @typescript-eslint/no-floating-promises -Sentry.startSpan({ name: 'parent1' }, async parentSpan1 => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Sentry.startSpan({ name: 'child1.1' }, async childSpan1 => { - childSpan1.addLink({ - context: parentSpan1.spanContext(), - attributes: { 'sentry.link.type': 'previous_trace' }, - }); +const span1 = Sentry.startInactiveSpan({ name: 'span1' }); +span1.end(); - childSpan1.end(); - }); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Sentry.startSpan({ name: 'child1.2' }, async childSpan2 => { - childSpan2.addLink({ - context: parentSpan1.spanContext(), - attributes: { 'sentry.link.type': 'previous_trace' }, - }); - - childSpan2.end(); +Sentry.startSpan({ name: 'rootSpan' }, rootSpan => { + rootSpan.addLink({ + context: span1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks-nested.ts b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks-nested.ts new file mode 100644 index 000000000000..216beff5c87e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks-nested.ts @@ -0,0 +1,31 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [], + transport: loggingTransport, +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.startSpan({ name: 'parent1' }, async parentSpan1 => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child1.1' }, async childSpan1 => { + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.startSpan({ name: 'child2.1' }, async childSpan2 => { + childSpan2.addLinks([ + { context: parentSpan1.spanContext() }, + { + context: childSpan1.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }, + ]); + + childSpan2.end(); + }); + + childSpan1.end(); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts index 216beff5c87e..1ce8a8a34a8f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts +++ b/dev-packages/node-integration-tests/suites/tracing/linking/scenario-addLinks.ts @@ -9,23 +9,18 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line @typescript-eslint/no-floating-promises -Sentry.startSpan({ name: 'parent1' }, async parentSpan1 => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Sentry.startSpan({ name: 'child1.1' }, async childSpan1 => { - // eslint-disable-next-line @typescript-eslint/no-floating-promises - Sentry.startSpan({ name: 'child2.1' }, async childSpan2 => { - childSpan2.addLinks([ - { context: parentSpan1.spanContext() }, - { - context: childSpan1.spanContext(), - attributes: { 'sentry.link.type': 'previous_trace' }, - }, - ]); +const span1 = Sentry.startInactiveSpan({ name: 'span1' }); +span1.end(); - childSpan2.end(); - }); +const span2 = Sentry.startInactiveSpan({ name: 'span2' }); +span2.end(); - childSpan1.end(); - }); +Sentry.startSpan({ name: 'rootSpan' }, rootSpan => { + rootSpan.addLinks([ + { context: span1.spanContext() }, + { + context: span2.spanContext(), + attributes: { 'sentry.link.type': 'previous_trace' }, + }, + ]); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts index 601e6c77c38b..7bd83dfd420c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts @@ -1,73 +1,164 @@ import { createRunner } from '../../../utils/runner'; -// A general note regarding this test: -// The fact that the trace_id and span_id are correctly linked is tested in a unit test - describe('span links', () => { - test('should link spans with addLink()', done => { + test('should link spans with addLink() in trace context', done => { + let span1_traceId: string, span1_spanId: string; + createRunner(__dirname, 'scenario-addLink.ts') .expect({ - transaction: { - transaction: 'parent1', - spans: [ + transaction: event => { + expect(event.transaction).toBe('span1'); + + // assigning a string to prevent "might be undefined" + span1_traceId = event.contexts?.trace?.trace_id || 'non-existent-trace-id'; + span1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + + expect(event.spans).toEqual([]); + }, + }) + .expect({ + transaction: event => { + expect(event.transaction).toBe('rootSpan'); + + expect(event.contexts?.trace?.links).toEqual([ expect.objectContaining({ - description: 'child1.1', - links: [ - expect.objectContaining({ - trace_id: expect.any(String), - span_id: expect.any(String), - attributes: expect.objectContaining({ - 'sentry.link.type': 'previous_trace', - }), - }), - ], + trace_id: expect.stringMatching(span1_traceId), + span_id: expect.stringMatching(span1_spanId), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), }), + ]); + }, + }) + .start(done); + }); + + test('should link spans with addLinks() in trace context', done => { + let span1_traceId: string, span1_spanId: string, span2_traceId: string, span2_spanId: string; + + createRunner(__dirname, 'scenario-addLinks.ts') + .expect({ + transaction: event => { + expect(event.transaction).toBe('span1'); + + // assigning a string to prevent "might be undefined" + span1_traceId = event.contexts?.trace?.trace_id || 'non-existent-trace-id'; + span1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + + expect(event.spans).toEqual([]); + }, + }) + .expect({ + transaction: event => { + expect(event.transaction).toBe('span2'); + + // assigning a string to prevent "might be undefined" + span2_traceId = event.contexts?.trace?.trace_id || 'non-existent-trace-id'; + span2_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + + expect(event.spans).toEqual([]); + }, + }) + .expect({ + transaction: event => { + expect(event.transaction).toBe('rootSpan'); + + expect(event.contexts?.trace?.links).toEqual([ + expect.not.objectContaining({ attributes: expect.anything() }) && + expect.objectContaining({ + trace_id: expect.stringMatching(span1_traceId), + span_id: expect.stringMatching(span1_spanId), + }), expect.objectContaining({ - description: 'child1.2', - links: [ - expect.objectContaining({ - trace_id: expect.any(String), - span_id: expect.any(String), - attributes: expect.objectContaining({ - 'sentry.link.type': 'previous_trace', - }), - }), - ], + trace_id: expect.stringMatching(span2_traceId), + span_id: expect.stringMatching(span2_spanId), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), }), - ], + ]); }, }) .start(done); }); - test('should link spans with addLinks()', done => { - createRunner(__dirname, 'scenario-addLinks.ts') + test('should link spans with addLink() in nested startSpan() calls', done => { + createRunner(__dirname, 'scenario-addLink-nested.ts') .expect({ - transaction: { - transaction: 'parent1', - spans: [ + transaction: event => { + expect(event.transaction).toBe('parent1'); + + // assigning a string to prevent "might be undefined" + const parent1_traceId = event.contexts?.trace?.trace_id || 'non-existent-span-id'; + const parent1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + + const spans = event.spans || []; + const child1_1 = spans.find(span => span.description === 'child1.1'); + const child1_2 = spans.find(span => span.description === 'child1.2'); + + expect(child1_1).toBeDefined(); + expect(child1_1?.links).toEqual([ + expect.objectContaining({ + trace_id: expect.stringMatching(parent1_traceId), + span_id: expect.stringMatching(parent1_spanId), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), + }), + ]); + + expect(child1_2).toBeDefined(); + expect(child1_2?.links).toEqual([ expect.objectContaining({ - description: 'child1.1', - links: [], + trace_id: expect.stringMatching(parent1_traceId), + span_id: expect.stringMatching(parent1_spanId), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), }), + ]); + }, + }) + .start(done); + }); + + test('should link spans with addLinks() in nested startSpan() calls', done => { + createRunner(__dirname, 'scenario-addLinks-nested.ts') + .expect({ + transaction: event => { + expect(event.transaction).toBe('parent1'); + + // assigning a string to prevent "might be undefined" + const parent1_traceId = event.contexts?.trace?.trace_id || 'non-existent-span-id'; + const parent1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + + const spans = event.spans || []; + const child1_1 = spans.find(span => span.description === 'child1.1'); + const child2_1 = spans.find(span => span.description === 'child2.1'); + + expect(child1_1).toBeDefined(); + + expect(child2_1).toBeDefined(); + + expect(child2_1?.links).toEqual([ + expect.not.objectContaining({ attributes: expect.anything() }) && + expect.objectContaining({ + trace_id: expect.stringMatching(parent1_traceId), + span_id: expect.stringMatching(parent1_spanId), + }), expect.objectContaining({ - description: 'child2.1', - links: [ - expect.not.objectContaining({ attributes: expect.anything() }) && - expect.objectContaining({ - trace_id: expect.any(String), - span_id: expect.any(String), - }), - expect.objectContaining({ - trace_id: expect.any(String), - span_id: expect.any(String), - attributes: expect.objectContaining({ - 'sentry.link.type': 'previous_trace', - }), - }), - ], + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error traceID is defined + trace_id: expect.stringMatching(child1_1?.trace_id), + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error spanID is defined + span_id: expect.stringMatching(child1_1?.span_id), + attributes: expect.objectContaining({ + 'sentry.link.type': 'previous_trace', + }), }), - ], + ]); }, }) .start(done); diff --git a/packages/core/src/types-hoist/context.ts b/packages/core/src/types-hoist/context.ts index 60aa60b38868..0ad6eebf6ac3 100644 --- a/packages/core/src/types-hoist/context.ts +++ b/packages/core/src/types-hoist/context.ts @@ -1,4 +1,5 @@ import type { FeatureFlag } from '../featureFlags'; +import type { SpanLinkJSON } from './link'; import type { Primitive } from './misc'; import type { SpanOrigin } from './span'; @@ -106,6 +107,7 @@ export interface TraceContext extends Record { tags?: { [key: string]: Primitive }; trace_id: string; origin?: SpanOrigin; + links?: SpanLinkJSON[]; } export interface CloudResourceContext extends Record { diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 97b9b0984ab8..40ef63a13b43 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -1,4 +1,3 @@ -import type { SpanLink } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -400,40 +399,6 @@ describe('startSpan', () => { }); }); - it('allows to pass span links', () => { - const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); - - // @ts-expect-error links exists on span - expect(rawSpan1?.links).toEqual(undefined); - - const span1JSON = spanToJSON(rawSpan1); - - startSpan({ name: '/users/:id' }, rawSpan2 => { - rawSpan2.addLink({ - context: rawSpan1.spanContext(), - attributes: { - 'sentry.link.type': 'previous_trace', - }, - }); - - const span2LinkJSON = spanToJSON(rawSpan2).links?.[0]; - - expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); - - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); - expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); - - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); - expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); - }); - }); - it('allows to force a transaction with forceTransaction=true', async () => { const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); client = new TestClient(options); @@ -935,40 +900,6 @@ describe('startSpanManual', () => { }); }); - it('allows to pass span links', () => { - const rawSpan1 = startInactiveSpan({ name: 'pageload_span' }); - - // @ts-expect-error links exists on span - expect(rawSpan1?.links).toEqual(undefined); - - const span1JSON = spanToJSON(rawSpan1); - - startSpanManual({ name: '/users/:id' }, rawSpan2 => { - rawSpan2.addLink({ - context: rawSpan1.spanContext(), - attributes: { - 'sentry.link.type': 'previous_trace', - }, - }); - - const span2LinkJSON = spanToJSON(rawSpan2).links?.[0]; - - expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); - - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); - expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); - - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); - // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); - expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); - }); - }); - it('allows to force a transaction with forceTransaction=true', async () => { const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); client = new TestClient(options); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 2f11b9bbfb7b..1c88afea0f51 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -248,6 +248,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve ...removeSentryAttributes(span.attributes), }); + const { links } = span; const { traceId: trace_id, spanId: span_id } = span.spanContext(); // If parentSpanIdFromTraceState is defined at all, we want it to take precedence @@ -267,6 +268,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve origin, op, status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined + links: convertSpanLinksForEnvelope(links), }); const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]; diff --git a/packages/opentelemetry/test/spanExporter.test.ts b/packages/opentelemetry/test/spanExporter.test.ts index 48ab8da060de..19714c2b172f 100644 --- a/packages/opentelemetry/test/spanExporter.test.ts +++ b/packages/opentelemetry/test/spanExporter.test.ts @@ -1,5 +1,5 @@ import { ATTR_HTTP_RESPONSE_STATUS_CODE } from '@opentelemetry/semantic-conventions'; -import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, startInactiveSpan } from '@sentry/core'; +import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_OP, startInactiveSpan, startSpanManual } from '@sentry/core'; import { createTransactionForOtelSpan } from '../src/spanExporter'; import { cleanupOtel, mockSdkInit } from './helpers/mockSdkInit'; @@ -108,4 +108,31 @@ describe('createTransactionForOtelSpan', () => { transaction_info: { source: 'custom' }, }); }); + + it('adds span link to the trace context when adding with addLink()', () => { + const span = startInactiveSpan({ name: 'parent1' }); + span.end(); + + startSpanManual({ name: 'rootSpan' }, rootSpan => { + rootSpan.addLink({ context: span.spanContext(), attributes: { 'sentry.link.type': 'previous_trace' } }); + rootSpan.end(); + + const prevTraceId = span.spanContext().traceId; + const prevSpanId = span.spanContext().spanId; + const event = createTransactionForOtelSpan(rootSpan as any); + + expect(event.contexts?.trace).toEqual( + expect.objectContaining({ + links: [ + expect.objectContaining({ + attributes: { 'sentry.link.type': 'previous_trace' }, + sampled: true, + trace_id: expect.stringMatching(prevTraceId), + span_id: expect.stringMatching(prevSpanId), + }), + ], + }), + ); + }); + }); }); diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index d1bd223f0cd0..ba1adbb74031 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -20,7 +20,7 @@ import { suppressTracing, withScope, } from '@sentry/core'; -import type { Event, Scope, SpanLink } from '@sentry/core'; +import type { Event, Scope } from '@sentry/core'; import { SEMATTRS_HTTP_METHOD } from '@opentelemetry/semantic-conventions'; import { continueTrace, startInactiveSpan, startSpan, startSpanManual } from '../src/trace'; @@ -377,15 +377,15 @@ describe('trace', () => { expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); + expect(rawSpan2?.links?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); + expect(rawSpan2?.links?.[0].context.traceId).toEqual(span1JSON.trace_id); expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); + expect(rawSpan2?.links?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); + expect(rawSpan2?.links?.[0].context.spanId).toEqual(span1JSON.span_id); expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); }); }); @@ -961,15 +961,15 @@ describe('trace', () => { expect(span2LinkJSON?.attributes?.['sentry.link.type']).toBe('previous_trace'); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); + expect(rawSpan2?.links?.[0].context.traceId).toEqual(rawSpan1._spanContext.traceId); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.traceId).toEqual(span1JSON.trace_id); + expect(rawSpan2?.links?.[0].context.traceId).toEqual(span1JSON.trace_id); expect(span2LinkJSON?.trace_id).toBe(span1JSON.trace_id); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); + expect(rawSpan2?.links?.[0].context.spanId).toEqual(rawSpan1?._spanContext.spanId); // @ts-expect-error links and _spanContext exist on span - expect((rawSpan2?.links as SpanLink[])?.[0].context.spanId).toEqual(span1JSON.span_id); + expect(rawSpan2?.links?.[0].context.spanId).toEqual(span1JSON.span_id); expect(span2LinkJSON?.span_id).toBe(span1JSON.span_id); }); }); From adf1cf8ddd53e61619b25e654d5e1c141b965119 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 14:05:22 +0100 Subject: [PATCH 08/11] fix test --- packages/core/test/lib/tracing/trace.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 40ef63a13b43..c33b50c01a85 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -1324,7 +1324,7 @@ describe('startInactiveSpan', () => { origin: 'manual', }, }); - expect(innerTransaction?.spans).toEqual(undefined); + expect(innerTransaction?.spans).toEqual([]); expect(innerTransaction?.transaction).toEqual('inner transaction'); expect(innerTransaction?.sdkProcessingMetadata).toEqual({ dynamicSamplingContext: { From 160fc563c52d57fcfbf368029b041ba2140c56d6 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 14:46:53 +0100 Subject: [PATCH 09/11] type-cast tests --- .../suites/tracing/linking/test.ts | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts index 7bd83dfd420c..9bd501517f0a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts @@ -9,9 +9,8 @@ describe('span links', () => { transaction: event => { expect(event.transaction).toBe('span1'); - // assigning a string to prevent "might be undefined" - span1_traceId = event.contexts?.trace?.trace_id || 'non-existent-trace-id'; - span1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + span1_traceId = event.contexts?.trace?.trace_id as string; + span1_spanId = event.contexts?.trace?.span_id as string; expect(event.spans).toEqual([]); }, @@ -42,9 +41,8 @@ describe('span links', () => { transaction: event => { expect(event.transaction).toBe('span1'); - // assigning a string to prevent "might be undefined" - span1_traceId = event.contexts?.trace?.trace_id || 'non-existent-trace-id'; - span1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + span1_traceId = event.contexts?.trace?.trace_id as string; + span1_spanId = event.contexts?.trace?.span_id as string; expect(event.spans).toEqual([]); }, @@ -53,9 +51,8 @@ describe('span links', () => { transaction: event => { expect(event.transaction).toBe('span2'); - // assigning a string to prevent "might be undefined" - span2_traceId = event.contexts?.trace?.trace_id || 'non-existent-trace-id'; - span2_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + span2_traceId = event.contexts?.trace?.trace_id as string; + span2_spanId = event.contexts?.trace?.span_id as string; expect(event.spans).toEqual([]); }, @@ -89,9 +86,8 @@ describe('span links', () => { transaction: event => { expect(event.transaction).toBe('parent1'); - // assigning a string to prevent "might be undefined" - const parent1_traceId = event.contexts?.trace?.trace_id || 'non-existent-span-id'; - const parent1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + const parent1_traceId = event.contexts?.trace?.trace_id as string; + const parent1_spanId = event.contexts?.trace?.span_id as string; const spans = event.spans || []; const child1_1 = spans.find(span => span.description === 'child1.1'); @@ -129,9 +125,8 @@ describe('span links', () => { transaction: event => { expect(event.transaction).toBe('parent1'); - // assigning a string to prevent "might be undefined" - const parent1_traceId = event.contexts?.trace?.trace_id || 'non-existent-span-id'; - const parent1_spanId = event.contexts?.trace?.span_id || 'non-existent-span-id'; + const parent1_traceId = event.contexts?.trace?.trace_id as string; + const parent1_spanId = event.contexts?.trace?.span_id as string; const spans = event.spans || []; const child1_1 = spans.find(span => span.description === 'child1.1'); From 91fde6dcd0d6742b199da63413767220b639b226 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 14:48:40 +0100 Subject: [PATCH 10/11] change size limit --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index c6e86836fd4c..157c1243021e 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -54,7 +54,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '68 KB', + limit: '70 KB', modifyWebpackConfig: function (config) { const webpack = require('webpack'); const TerserPlugin = require('terser-webpack-plugin'); From ff296f7e8ca310a6540a70ac900cfc404fec9148 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 13 Feb 2025 17:02:22 +0100 Subject: [PATCH 11/11] change test match --- .../node-integration-tests/suites/tracing/linking/test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts index 9bd501517f0a..1c4e518a4f74 100644 --- a/dev-packages/node-integration-tests/suites/tracing/linking/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/linking/test.ts @@ -143,12 +143,8 @@ describe('span links', () => { span_id: expect.stringMatching(parent1_spanId), }), expect.objectContaining({ - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error traceID is defined - trace_id: expect.stringMatching(child1_1?.trace_id), - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error spanID is defined - span_id: expect.stringMatching(child1_1?.span_id), + trace_id: expect.stringMatching(child1_1?.trace_id || 'non-existent-id-fallback'), + span_id: expect.stringMatching(child1_1?.span_id || 'non-existent-id-fallback'), attributes: expect.objectContaining({ 'sentry.link.type': 'previous_trace', }),