From aebc06570c9107e02f116f900e1d1fda326e12b0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 11:50:56 +0100 Subject: [PATCH 1/6] feat(core): Deprecate `Span.origin` in favor of `sentry.origin` attribute --- packages/core/src/semanticAttributes.ts | 5 ++++ packages/core/src/tracing/span.ts | 31 ++++++++++++++++++------- packages/types/src/span.ts | 7 ++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index 884eb0e5e9ef..afd0d123090f 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -14,3 +14,8 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; * Use this attribute to represent the operation of a span. */ export const SEMANTIC_ATTRIBUTE_SENTRY_OP = 'sentry.op'; + +/** + * Use this attribute to represent the origin of a span. + */ +export const SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN = 'sentry.origin'; diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 6371435e2d58..8ddfddb3b353 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -16,7 +16,7 @@ import type { import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { getRootSpan } from '../utils/getRootSpan'; import { TRACE_FLAG_NONE, @@ -100,11 +100,6 @@ export class Span implements SpanInterface { */ public instrumenter: Instrumenter; - /** - * The origin of the span, giving context about what created the span. - */ - public origin?: SpanOrigin; - protected _traceId: string; protected _spanId: string; protected _parentSpanId?: string; @@ -138,7 +133,9 @@ export class Span implements SpanInterface { this._attributes = spanContext.attributes ? { ...spanContext.attributes } : {}; // eslint-disable-next-line deprecation/deprecation this.instrumenter = spanContext.instrumenter || 'sentry'; - this.origin = spanContext.origin || 'manual'; + + this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanContext.origin || 'manual'); + // eslint-disable-next-line deprecation/deprecation this._name = spanContext.name || spanContext.description; @@ -346,6 +343,24 @@ export class Span implements SpanInterface { this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op); } + /** + * The origin of the span, giving context about what created the span. + * + * @deprecated Use `spanToJSON().origin` to read the origin instead. + */ + public get origin(): SpanOrigin | undefined { + return this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined; + } + + /** + * The origin of the span, giving context about what created the span. + * + * @deprecated Use `startSpan()` functions to set the origin instead. + */ + public set origin(origin: SpanOrigin | undefined) { + this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, origin); + } + /* eslint-enable @typescript-eslint/member-ordering */ /** @inheritdoc */ @@ -611,7 +626,7 @@ export class Span implements SpanInterface { tags: Object.keys(this.tags).length > 0 ? this.tags : undefined, timestamp: this._endTime, trace_id: this._traceId, - origin: this.origin, + origin: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, }); } diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index f4eb56c88194..28acf68fdcfd 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -258,6 +258,13 @@ export interface Span extends Omit { */ status?: string; + /** + * The origin of the span, giving context about what created the span. + * + * @deprecated Use `startSpan` function to set and `spanToJSON(span).origin` to read the origin instead. + */ + origin?: SpanOrigin; + /** * Get context data for this span. * This includes the spanId & the traceId. From d1d21aa190d36b7a3f815444a5825156efdaf886 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 13:13:45 +0100 Subject: [PATCH 2/6] fix a bunch of tests --- .../nextjs-app-dir/tests/middleware.test.ts | 1 + .../tests/propagation.test.ts | 4 ++ .../tests/transactions.test.ts | 4 ++ packages/core/test/lib/tracing/span.test.ts | 51 +++++++++++++++---- .../core/test/lib/utils/spanUtils.test.ts | 4 ++ .../test/client/tracingFetch.test.ts | 7 +-- .../test/custom/transaction.test.ts | 6 +++ .../test/integration/scope.test.ts | 5 +- .../test/integration/transactions.test.ts | 36 ++++++++++--- .../test/browser/request.test.ts | 1 + packages/tracing/test/hub.test.ts | 3 +- packages/tracing/test/span.test.ts | 24 +++++++-- packages/tracing/test/transaction.test.ts | 16 ++++-- packages/types/src/span.ts | 2 +- 14 files changed, 135 insertions(+), 29 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts index b2346db58450..69fe5a8670c8 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -69,6 +69,7 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru type: 'fetch', url: 'http://localhost:3030/', 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.wintercg_fetch', }, description: 'GET http://localhost:3030/', op: 'http.client', 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 4c9310228eb3..9b83e882a739 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 @@ -64,6 +64,7 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', span_id: expect.any(String), @@ -87,6 +88,7 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', parent_span_id: outgoingHttpSpanId, @@ -159,6 +161,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', span_id: expect.any(String), @@ -182,6 +185,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', parent_span_id: outgoingHttpSpanId, 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 e29637a87df8..d2ab9e3d664f 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 @@ -29,6 +29,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'otel.kind': 'SERVER', 'http.response.status_code': 200, 'sentry.op': 'http.server', + 'sentry.origin': 'auto.http.otel.http', }, op: 'http.server', span_id: expect.any(String), @@ -48,6 +49,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'fastify.type': 'request_handler', 'http.route': '/test-transaction', 'otel.kind': 'INTERNAL', + 'sentry.origin': 'auto.http.otel.fastify', }, description: 'request handler - fastify -> app-auto-0', parent_span_id: expect.any(String), @@ -61,6 +63,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { { data: { 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', }, description: 'test-span', parent_span_id: expect.any(String), @@ -74,6 +77,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { { data: { 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', }, description: 'child-span', parent_span_id: expect.any(String), diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/span.test.ts index 2896ae494ad1..b3c08987d4b2 100644 --- a/packages/core/test/lib/tracing/span.test.ts +++ b/packages/core/test/lib/tracing/span.test.ts @@ -84,6 +84,8 @@ describe('span', () => { strArray: ['aa', 'bb'], boolArray: [true, false], arrayWithUndefined: [1, undefined, 2], + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', }); }); @@ -92,11 +94,12 @@ describe('span', () => { span.setAttribute('str', 'bar'); - expect(Object.keys(span['_attributes']).length).toEqual(1); + // length 2 because `sentry.origin` is always set by default + expect(Object.keys(span['_attributes']).length).toEqual(2); span.setAttribute('str', undefined); - expect(Object.keys(span['_attributes']).length).toEqual(0); + expect(Object.keys(span['_attributes']).length).toEqual(1); }); it('disallows invalid attribute types', () => { @@ -119,7 +122,10 @@ describe('span', () => { const initialAttributes = span['_attributes']; - expect(initialAttributes).toEqual({}); + expect(initialAttributes).toEqual({ + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); const newAttributes = { str: 'bar', @@ -145,6 +151,7 @@ describe('span', () => { strArray: ['aa', 'bb'], boolArray: [true, false], arrayWithUndefined: [1, undefined, 2], + 'sentry.origin': 'manual', }); expect(span['_attributes']).not.toBe(newAttributes); @@ -164,6 +171,7 @@ describe('span', () => { strArray: ['aa', 'bb'], boolArray: [true, false], arrayWithUndefined: [1, undefined, 2], + 'sentry.origin': 'manual', }); }); @@ -172,11 +180,12 @@ describe('span', () => { span.setAttribute('str', 'bar'); - expect(Object.keys(span['_attributes']).length).toEqual(1); + // length 2 because `sentry.origin` is always set by default + expect(Object.keys(span['_attributes']).length).toEqual(2); span.setAttributes({ str: undefined }); - expect(Object.keys(span['_attributes']).length).toEqual(0); + expect(Object.keys(span['_attributes']).length).toEqual(1); }); }); @@ -275,7 +284,10 @@ describe('span', () => { it('works without data & attributes', () => { const span = new Span(); - expect(span['_getData']()).toEqual(undefined); + expect(span['_getData']()).toEqual({ + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); }); it('works with data only', () => { @@ -283,16 +295,27 @@ describe('span', () => { // eslint-disable-next-line deprecation/deprecation span.setData('foo', 'bar'); - expect(span['_getData']()).toEqual({ foo: 'bar' }); - // eslint-disable-next-line deprecation/deprecation - expect(span['_getData']()).toBe(span.data); + expect(span['_getData']()).toEqual({ + foo: 'bar', + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); + expect(span['_getData']()).toStrictEqual({ + // eslint-disable-next-line deprecation/deprecation + ...span.data, + 'sentry.origin': 'manual', + }); }); it('works with attributes only', () => { const span = new Span(); span.setAttribute('foo', 'bar'); - expect(span['_getData']()).toEqual({ foo: 'bar' }); + expect(span['_getData']()).toEqual({ + foo: 'bar', + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).toBe(span.attributes); }); @@ -306,7 +329,13 @@ describe('span', () => { // eslint-disable-next-line deprecation/deprecation span.setData('baz', 'baz'); - expect(span['_getData']()).toEqual({ foo: 'foo', bar: 'bar', baz: 'baz' }); + expect(span['_getData']()).toEqual({ + foo: 'foo', + bar: 'bar', + baz: 'baz', + // origin is set by default to 'manual' in the Span constructor + 'sentry.origin': 'manual', + }); // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).not.toBe(span.attributes); // eslint-disable-next-line deprecation/deprecation diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts index 5699fa391c2b..0afb59530623 100644 --- a/packages/core/test/lib/utils/spanUtils.test.ts +++ b/packages/core/test/lib/utils/spanUtils.test.ts @@ -55,6 +55,9 @@ describe('spanToJSON', () => { trace_id: span.spanContext().traceId, origin: 'manual', start_timestamp: span['_startTime'], + data: { + 'sentry.origin': 'manual', + }, }); }); @@ -89,6 +92,7 @@ describe('spanToJSON', () => { timestamp: 456, data: { 'sentry.op': 'test op', + 'sentry.origin': 'auto', }, }); }); diff --git a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts index 579ccf4ca547..8ae20200edad 100644 --- a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { Transaction } from '@sentry/types'; +import { SerializedEvent } from '@sentry/types'; import { countEnvelopes, getMultipleSentryEnvelopeRequests } from './utils/helpers'; test('should correctly instrument `fetch` for performance tracing', async ({ page }) => { @@ -12,7 +12,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag }); }); - const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { + const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { url: '/fetch', envelopeType: 'transaction', }); @@ -27,7 +27,6 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag }, }); - // @ts-expect-error - We know that `spans` is inside Transaction envelopes expect(transaction[0].spans).toEqual( expect.arrayContaining([ expect.objectContaining({ @@ -38,6 +37,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag 'http.response_content_length': expect.any(Number), 'http.response.status_code': 200, 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.client', }, description: 'GET http://example.com', op: 'http.client', @@ -47,6 +47,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag timestamp: expect.any(Number), trace_id: expect.any(String), status: expect.any(String), + origin: 'auto.http.client', }), ]), ); diff --git a/packages/opentelemetry/test/custom/transaction.test.ts b/packages/opentelemetry/test/custom/transaction.test.ts index b949ee82d91d..cb0cd2efd332 100644 --- a/packages/opentelemetry/test/custom/transaction.test.ts +++ b/packages/opentelemetry/test/custom/transaction.test.ts @@ -28,6 +28,9 @@ describe('NodeExperimentalTransaction', () => { expect.objectContaining({ contexts: { trace: { + data: { + 'sentry.origin': 'manual', + }, span_id: expect.any(String), trace_id: expect.any(String), origin: 'manual', @@ -109,6 +112,9 @@ describe('NodeExperimentalTransaction', () => { expect.objectContaining({ contexts: { trace: { + data: { + 'sentry.origin': 'manual', + }, span_id: expect.any(String), trace_id: expect.any(String), origin: 'manual', diff --git a/packages/opentelemetry/test/integration/scope.test.ts b/packages/opentelemetry/test/integration/scope.test.ts index f313dcba352e..6ab7c6d35621 100644 --- a/packages/opentelemetry/test/integration/scope.test.ts +++ b/packages/opentelemetry/test/integration/scope.test.ts @@ -94,7 +94,10 @@ describe('Integration | Scope', () => { expect.objectContaining({ contexts: expect.objectContaining({ trace: { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, span_id: spanId, status: 'ok', trace_id: traceId, diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index cf172d048b59..a87816691a45 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -83,7 +83,11 @@ describe('Integration | Transactions', () => { }, }, trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -146,6 +150,7 @@ describe('Integration | Transactions', () => { { data: { 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', }, description: 'inner span 1', origin: 'manual', @@ -157,7 +162,11 @@ describe('Integration | Transactions', () => { trace_id: expect.any(String), }, { - data: { 'otel.kind': 'INTERNAL', 'test.inner': 'test value' }, + data: { + 'otel.kind': 'INTERNAL', + 'test.inner': 'test value', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), @@ -238,7 +247,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -280,7 +293,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op b' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op b', + 'sentry.origin': 'manual', + }, op: 'test op b', span_id: expect.any(String), status: 'ok', @@ -360,6 +377,7 @@ describe('Integration | Transactions', () => { data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', }, op: 'test op', span_id: expect.any(String), @@ -398,7 +416,10 @@ describe('Integration | Transactions', () => { // This is the same behavior as for the "regular" SDKs expect(spans.map(span => spanToJSON(span))).toEqual([ { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 1', origin: 'manual', parent_span_id: expect.any(String), @@ -409,7 +430,10 @@ describe('Integration | Transactions', () => { trace_id: traceId, }, { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 6641be671bfc..782c0890e156 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -264,6 +264,7 @@ describe('callbacks', () => { type: 'fetch', url: 'http://dogs.are.great/', 'sentry.op': 'http.client', + 'sentry.origin': 'auto.http.browser', }); expect(finishedSpan.op).toBe('http.client'); }); diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index a7fbf5e86b62..7b73d9bb2d51 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -305,7 +305,8 @@ describe('Hub', () => { makeMain(hub); hub.startTransaction({ name: 'dogpark', parentSampled: true }); - expect(Transaction.prototype.setAttribute).toHaveBeenCalledTimes(0); + // length 1 because `sentry.origin` is set on span initialization + expect(Transaction.prototype.setAttribute).toHaveBeenCalledTimes(1); }); it('should record sampling method and rate when sampling decision comes from traceSampleRate', () => { diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 1cf33c7c435f..2d6db4c21707 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,7 +1,7 @@ /* eslint-disable deprecation/deprecation */ import { BrowserClient } from '@sentry/browser'; import { Hub, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, Scope, makeMain, spanToJSON } from '@sentry/core'; -import type { BaseTransportOptions, ClientOptions, TransactionSource } from '@sentry/types'; +import type { BaseTransportOptions, ClientOptions, SpanOrigin, TransactionSource } from '@sentry/types'; import { Span, TRACEPARENT_REGEXP, Transaction } from '../src'; import { getDefaultBrowserClientOptions } from './testutils'; @@ -173,6 +173,9 @@ describe('Span', () => { span_id: 'd', trace_id: 'c', origin: 'manual', + data: { + 'sentry.origin': 'manual', + }, }); }); }); @@ -474,6 +477,9 @@ describe('Span', () => { expect(context).toStrictEqual({ span_id: 'd', trace_id: 'c', + data: { + 'sentry.origin': 'manual', + }, origin: 'manual', }); }); @@ -481,7 +487,13 @@ describe('Span', () => { describe('toContext and updateWithContext', () => { test('toContext should return correct context', () => { - const originalContext = { traceId: 'a', spanId: 'b', sampled: false, description: 'test', op: 'op' }; + const originalContext = { + traceId: 'a', + spanId: 'b', + sampled: false, + description: 'test', + op: 'op', + }; const span = new Span(originalContext); const newContext = span.toContext(); @@ -494,6 +506,7 @@ describe('Span', () => { traceId: expect.any(String), data: { 'sentry.op': 'op', + 'sentry.origin': 'manual', }, }); }); @@ -562,7 +575,12 @@ describe('Span', () => { expect(span.op).toBe('new-op'); expect(span.sampled).toBe(true); expect(span.tags).toStrictEqual({ tag1: 'bye' }); - expect(span.data).toStrictEqual({ data0: 'foo', data1: 'bar', 'sentry.op': 'op' }); + expect(span.data).toStrictEqual({ + data0: 'foo', + data1: 'bar', + 'sentry.op': 'op', + 'sentry.origin': 'manual', + }); }); }); diff --git a/packages/tracing/test/transaction.test.ts b/packages/tracing/test/transaction.test.ts index 3d048c9a3c3f..f9e950dd9cb9 100644 --- a/packages/tracing/test/transaction.test.ts +++ b/packages/tracing/test/transaction.test.ts @@ -1,6 +1,10 @@ /* eslint-disable deprecation/deprecation */ import { BrowserClient, Hub } from '@sentry/browser'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; import { Transaction, addExtensionMethods } from '../src'; import { getDefaultBrowserClientOptions } from './testutils'; @@ -163,7 +167,10 @@ describe('`Transaction` class', () => { contexts: { foo: { key: 'val' }, trace: { - data: { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1 }, + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', + }, span_id: transaction.spanId, trace_id: transaction.traceId, origin: 'manual', @@ -191,7 +198,10 @@ describe('`Transaction` class', () => { expect.objectContaining({ contexts: { trace: { - data: { [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1 }, + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual', + }, span_id: transaction.spanId, trace_id: transaction.traceId, origin: 'manual', diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 28acf68fdcfd..e34940b35d65 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -166,7 +166,7 @@ export interface SpanContext { } /** Span holding trace_id, span_id */ -export interface Span extends Omit { +export interface Span extends Omit { /** * Human-readable identifier for the span. Identical to span.description. * @deprecated Use `spanToJSON(span).description` instead. From f21afb3d27326a0f65555473bf406d0435a9a1f4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 13:23:53 +0100 Subject: [PATCH 3/6] fix node experimental tests --- .../test/integration/scope.test.ts | 2 +- .../test/integration/transactions.test.ts | 45 +++++++++++++++---- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/packages/node-experimental/test/integration/scope.test.ts b/packages/node-experimental/test/integration/scope.test.ts index d1170dd636e8..6dfcad34ff81 100644 --- a/packages/node-experimental/test/integration/scope.test.ts +++ b/packages/node-experimental/test/integration/scope.test.ts @@ -88,7 +88,7 @@ describe('Integration | Scope', () => { expect.objectContaining({ contexts: expect.objectContaining({ trace: { - data: { 'otel.kind': 'INTERNAL' }, + data: { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual' }, span_id: spanId, status: 'ok', trace_id: traceId, diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts index 8a9464c1fd03..b1038a53a276 100644 --- a/packages/node-experimental/test/integration/transactions.test.ts +++ b/packages/node-experimental/test/integration/transactions.test.ts @@ -84,7 +84,11 @@ describe('Integration | Transactions', () => { }, runtime: { name: 'node', version: expect.any(String) }, trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -148,7 +152,10 @@ describe('Integration | Transactions', () => { // This is the same behavior as for the "regular" SDKs expect(spans.map(span => spanToJSON(span))).toEqual([ { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 1', origin: 'manual', parent_span_id: expect.any(String), @@ -159,7 +166,11 @@ describe('Integration | Transactions', () => { trace_id: expect.any(String), }, { - data: { 'otel.kind': 'INTERNAL', 'test.inner': 'test value' }, + data: { + 'otel.kind': 'INTERNAL', + 'test.inner': 'test value', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), @@ -244,7 +255,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), status: 'ok', @@ -286,7 +301,11 @@ describe('Integration | Transactions', () => { }, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op b' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op b', + 'sentry.origin': 'manual', + }, op: 'test op b', span_id: expect.any(String), status: 'ok', @@ -363,7 +382,11 @@ describe('Integration | Transactions', () => { attributes: {}, }), trace: { - data: { 'otel.kind': 'INTERNAL', 'sentry.op': 'test op' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.op': 'test op', + 'sentry.origin': 'auto.test', + }, op: 'test op', span_id: expect.any(String), parent_span_id: parentSpanId, @@ -401,7 +424,10 @@ describe('Integration | Transactions', () => { // This is the same behavior as for the "regular" SDKs expect(spans.map(span => spanToJSON(span))).toEqual([ { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 1', origin: 'manual', parent_span_id: expect.any(String), @@ -412,7 +438,10 @@ describe('Integration | Transactions', () => { trace_id: traceId, }, { - data: { 'otel.kind': 'INTERNAL' }, + data: { + 'otel.kind': 'INTERNAL', + 'sentry.origin': 'manual', + }, description: 'inner span 2', origin: 'manual', parent_span_id: expect.any(String), From c12e7f53751ac6460849af367c1bd96dd09feca1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 14:19:24 +0100 Subject: [PATCH 4/6] fix more tests --- packages/opentelemetry-node/test/spanprocessor.test.ts | 7 ++++++- packages/tracing/test/span.test.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 23818b17442e..0bd7d852f7a5 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -344,7 +344,8 @@ describe('SentrySpanProcessor', () => { const sentrySpan = getSpanForOtelSpan(child); - expect(spanToJSON(sentrySpan!).data).toEqual(undefined); + // origin is set by default to 'manual' + expect(spanToJSON(sentrySpan!).data).toEqual({ 'sentry.origin': 'manual' }); child.end(); @@ -354,6 +355,7 @@ describe('SentrySpanProcessor', () => { 'test-attribute-2': [1, 2, 3], 'test-attribute-3': 0, 'test-attribute-4': false, + 'sentry.origin': 'manual', }); }); @@ -592,6 +594,7 @@ describe('SentrySpanProcessor', () => { 'otel.kind': 'INTERNAL', url: 'http://example.com/my/route/123', 'sentry.op': 'http', + 'sentry.origin': 'manual', }); parentOtelSpan.end(); @@ -622,6 +625,7 @@ describe('SentrySpanProcessor', () => { 'otel.kind': 'INTERNAL', url: 'http://example.com/my/route/123', 'sentry.op': 'http', + 'sentry.origin': 'manual', }); expect(op).toBe('http'); @@ -655,6 +659,7 @@ describe('SentrySpanProcessor', () => { 'http.query': '?what=123', 'http.fragment': '#myHash', 'sentry.op': 'http', + 'sentry.origin': 'manual', }); expect(op).toBe('http'); diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 2d6db4c21707..4a7e1537f394 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,7 +1,7 @@ /* eslint-disable deprecation/deprecation */ import { BrowserClient } from '@sentry/browser'; import { Hub, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, Scope, makeMain, spanToJSON } from '@sentry/core'; -import type { BaseTransportOptions, ClientOptions, SpanOrigin, TransactionSource } from '@sentry/types'; +import type { BaseTransportOptions, ClientOptions, TransactionSource } from '@sentry/types'; import { Span, TRACEPARENT_REGEXP, Transaction } from '../src'; import { getDefaultBrowserClientOptions } from './testutils'; From 84429c7c9218ee940a9b51d043a00cc998969b2d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 14:44:10 +0100 Subject: [PATCH 5/6] integration test --- .../suites/public-api/startSpan/basic/test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts index 9df0e24f3a38..2567f5c018d3 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts @@ -24,11 +24,11 @@ sentryTest('should report finished spans as children of the root transaction', a const url = await getLocalTestPath({ testDir: __dirname }); const transaction = await getFirstSentryEnvelopeRequest(page, url); - const rootSpanId = transaction?.contexts?.trace?.span_id; - expect(transaction.spans).toHaveLength(1); const span_1 = transaction.spans?.[0]; expect(span_1?.description).toBe('child_span'); - expect(span_1?.parent_span_id).toEqual(rootSpanId); + expect(span_1?.parent_span_id).toEqual(transaction?.contexts?.trace?.span_id); + expect(span_1?.origin).toEqual('manual'); + expect(span_1?.data?.['sentry.origin']).toEqual('manual'); }); From 500c3ed356e9f6de50781b55214f23b8257f0330 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jan 2024 14:42:56 +0100 Subject: [PATCH 6/6] fix nextjs test pls --- .../nextjs/test/integration/test/client/tracingFetch.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts index 8ae20200edad..8517b4ab0fce 100644 --- a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts @@ -37,7 +37,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag 'http.response_content_length': expect.any(Number), 'http.response.status_code': 200, 'sentry.op': 'http.client', - 'sentry.origin': 'auto.http.client', + 'sentry.origin': 'auto.http.browser', }, description: 'GET http://example.com', op: 'http.client', @@ -47,7 +47,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag timestamp: expect.any(Number), trace_id: expect.any(String), status: expect.any(String), - origin: 'auto.http.client', + origin: 'auto.http.browser', }), ]), );