From 67654bcfcc5b923948c49ef2e4a25631a0d57773 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 18 Apr 2024 18:15:49 +0200 Subject: [PATCH 1/8] feat(browser): Start standalone (segment) spans via `start*Span` APIs separate standalone and segment tmp add test helper (no need after rebase) add tests --- .../public-api/startSpan/standalone/init.js | 8 +++ .../standalone/segment-false/subject.js | 1 + .../standalone/segment-false/test.ts | 47 +++++++++++++ .../standalone/segment-true/subject.js | 1 + .../startSpan/standalone/segment-true/test.ts | 48 +++++++++++++ .../standalone/segment-undefined/subject.js | 1 + .../standalone/segment-undefined/test.ts | 46 +++++++++++++ .../utils/helpers.ts | 7 ++ packages/core/src/tracing/sentrySpan.ts | 48 +++++++++++-- packages/core/src/tracing/trace.ts | 19 ++++-- packages/core/test/lib/tracing/trace.test.ts | 67 +++++++++++++++++++ packages/types/src/span.ts | 21 ++++++ packages/types/src/startSpanOptions.ts | 33 +++++++++ 13 files changed, 334 insertions(+), 13 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/init.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/init.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/init.js new file mode 100644 index 000000000000..f568455a877c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1.0, +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js new file mode 100644 index 000000000000..e2034026cce4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js @@ -0,0 +1 @@ +Sentry.startSpan({ name: 'standalone_segment_span', experimental: { standalone: true, segment: false } }, () => {}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts new file mode 100644 index 000000000000..61a7a2e230aa --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts @@ -0,0 +1,47 @@ +import { expect } from '@playwright/test'; +import type { SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../../utils/helpers'; + +sentryTest('sends a span envelope with is_segment: false', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const spanEnvelope = await getFirstSentryEnvelopeRequest(page, url, properFullEnvelopeRequestParser); + + const headers = spanEnvelope[0]; + const item = spanEnvelope[1][0]; + + const itemHeader = item[0]; + const spanJson = item[1]; + + expect(headers).toMatchObject({ + sent_at: expect.any(String), + }); + + expect(itemHeader).toEqual({ + type: 'span', + }); + + expect(spanJson).toEqual({ + data: { + 'sentry.origin': 'manual', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + }, + description: 'standalone_segment_span', + origin: 'manual', + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + is_segment: false, + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js new file mode 100644 index 000000000000..0c257c679faa --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js @@ -0,0 +1 @@ +Sentry.startSpan({ name: 'standalone_segment_span', experimental: { standalone: true, segment: true } }, () => {}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts new file mode 100644 index 000000000000..3e08786a5d94 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts @@ -0,0 +1,48 @@ +import { expect } from '@playwright/test'; +import type { SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../../utils/helpers'; + +sentryTest('sends a segment span envelope', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const spanEnvelope = await getFirstSentryEnvelopeRequest(page, url, properFullEnvelopeRequestParser); + + const headers = spanEnvelope[0]; + const item = spanEnvelope[1][0]; + + const itemHeader = item[0]; + const spanJson = item[1]; + + expect(headers).toMatchObject({ + sent_at: expect.any(String), + }); + + expect(itemHeader).toEqual({ + type: 'span', + }); + + expect(spanJson).toEqual({ + data: { + 'sentry.origin': 'manual', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + }, + description: 'standalone_segment_span', + origin: 'manual', + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + is_segment: true, + segment_id: spanJson.span_id, + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/subject.js new file mode 100644 index 000000000000..4ce33ad3b8c4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/subject.js @@ -0,0 +1 @@ +Sentry.startSpan({ name: 'standalone_segment_span', experimental: { standalone: true } }, () => {}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts new file mode 100644 index 000000000000..d7e8423de639 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts @@ -0,0 +1,46 @@ +import { expect } from '@playwright/test'; +import type { SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../../utils/helpers'; + +sentryTest('sends a segment-less span envelope', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const spanEnvelope = await getFirstSentryEnvelopeRequest(page, url, properFullEnvelopeRequestParser); + + const headers = spanEnvelope[0]; + const item = spanEnvelope[1][0]; + + const itemHeader = item[0]; + const spanJson = item[1]; + + expect(headers).toMatchObject({ + sent_at: expect.any(String), + }); + + expect(itemHeader).toEqual({ + type: 'span', + }); + + expect(spanJson).toEqual({ + data: { + 'sentry.origin': 'manual', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + }, + description: 'standalone_segment_span', + origin: 'manual', + span_id: expect.stringMatching(/^[0-9a-f]{16}$/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), + }); +}); diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index d1e27f194e12..b4fe7d2607ff 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -74,6 +74,13 @@ export const properEnvelopeRequestParser = (request: Request | null, return properEnvelopeParser(request)[0][envelopeIndex] as T; }; +export const properFullEnvelopeRequestParser = (request: Request | null): T => { + // https://develop.sentry.dev/sdk/envelopes/ + const envelope = request?.postData() || ''; + + return parseEnvelope(envelope) as T; +}; + export const envelopeHeaderRequestParser = (request: Request | null): EventEnvelopeHeaders => { // https://develop.sentry.dev/sdk/envelopes/ const envelope = request?.postData() || ''; diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 31fd35bc4b68..1ceef2a98fde 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -4,6 +4,7 @@ import type { SpanAttributeValue, SpanAttributes, SpanContextData, + SpanEnvelope, SpanJSON, SpanOrigin, SpanStatus, @@ -16,6 +17,7 @@ import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/ut import { getClient, getCurrentScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; +import { createSpanEnvelope } from '../envelope'; import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary'; import { SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, @@ -58,6 +60,12 @@ export class SentrySpan implements Span { /** The timed events added to this span. */ protected _events: TimedEvent[]; + /** if true, treat span as a standalone span (not part of a transaction) */ + private _isStandaloneSpan?: boolean; + + /** send standalone span as segment span */ + private _isSegmentSpan?: boolean; + /** * You should never call the constructor manually, always use `Sentry.startSpan()` * or other span methods. @@ -96,6 +104,9 @@ export class SentrySpan implements Span { if (this._endTime) { this._onSpanEnded(); } + + this._isStandaloneSpan = spanContext.isStandalone; + this._isSegmentSpan = spanContext.isStandalone && spanContext.isSegment; } /** @inheritdoc */ @@ -188,6 +199,8 @@ export class SentrySpan implements Span { profile_id: this._attributes[SEMANTIC_ATTRIBUTE_PROFILE_ID] as string | undefined, exclusive_time: this._attributes[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME] as number | undefined, measurements: timedEventsToMeasurements(this._events), + is_segment: !this._isStandaloneSpan ? undefined : this._isSegmentSpan, + segment_id: this._isStandaloneSpan && this._isSegmentSpan ? this._spanId : undefined, }); } @@ -227,13 +240,20 @@ export class SentrySpan implements Span { client.emit('spanEnd', this); } - // If this is a root span, send it when it is endedf - if (this === getRootSpan(this)) { - const transactionEvent = this._convertSpanToTransaction(); - if (transactionEvent) { - const scope = getCapturedScopesOnSpan(this).scope || getCurrentScope(); - scope.captureEvent(transactionEvent); - } + // If this is not a root span, we're done, otherwise, we send it when it is ended + if (this !== getRootSpan(this)) { + return; + } + + if (this._isStandaloneSpan) { + sendSpanEnvelope(createSpanEnvelope([this])); + return; + } + + const transactionEvent = this._convertSpanToTransaction(); + if (transactionEvent) { + const scope = getCapturedScopesOnSpan(this).scope || getCurrentScope(); + scope.captureEvent(transactionEvent); } } @@ -318,3 +338,17 @@ function isSpanTimeInput(value: undefined | SpanAttributes | SpanTimeInput): val function isFullFinishedSpan(input: Partial): input is SpanJSON { return !!input.start_timestamp && !!input.timestamp && !!input.span_id && !!input.trace_id; } + +function sendSpanEnvelope(envelope: SpanEnvelope): void { + const client = getClient(); + if (!client) { + return; + } + + const transport = client.getTransport(); + if (transport) { + transport.send(envelope).then(null, reason => { + DEBUG_BUILD && logger.error('Error while sending span:', reason); + }); + } +} diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 065403c78aed..16ee0e053eae 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -45,7 +45,7 @@ export function startSpan(context: StartSpanOptions, callback: (span: Span) = const shouldSkipSpan = context.onlyIfParent && !parentSpan; const activeSpan = shouldSkipSpan ? new SentryNonRecordingSpan() - : createChildSpanOrTransaction({ + : createChildOrRootSpan({ parentSpan, spanContext, forceTransaction: context.forceTransaction, @@ -92,7 +92,7 @@ export function startSpanManual(context: StartSpanOptions, callback: (span: S const shouldSkipSpan = context.onlyIfParent && !parentSpan; const activeSpan = shouldSkipSpan ? new SentryNonRecordingSpan() - : createChildSpanOrTransaction({ + : createChildOrRootSpan({ parentSpan, spanContext, forceTransaction: context.forceTransaction, @@ -144,7 +144,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span { return new SentryNonRecordingSpan(); } - return createChildSpanOrTransaction({ + return createChildOrRootSpan({ parentSpan, spanContext, forceTransaction: context.forceTransaction, @@ -212,7 +212,7 @@ export function suppressTracing(callback: () => T): T { }); } -function createChildSpanOrTransaction({ +function createChildOrRootSpan({ parentSpan, spanContext, forceTransaction, @@ -291,14 +291,21 @@ function createChildSpanOrTransaction({ * Eventually the StartSpanOptions will be more aligned with OpenTelemetry. */ function normalizeContext(context: StartSpanOptions): SentrySpanArguments { + const exp = context.experimental || {}; + const initialCtx: SentrySpanArguments = { + isStandalone: exp.standalone, + isSegment: exp.standalone && exp.segment, + ...context, + }; + if (context.startTime) { - const ctx: SentrySpanArguments & { startTime?: SpanTimeInput } = { ...context }; + const ctx: SentrySpanArguments & { startTime?: SpanTimeInput } = { ...initialCtx }; ctx.startTimestamp = spanTimeInputToSeconds(context.startTime); delete ctx.startTime; return ctx; } - return context; + return initialCtx; } function getAcs(): AsyncContextStrategy { diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 110a90d01f6f..4ed48ea61999 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -551,6 +551,73 @@ describe('startSpan', () => { expect(result).toBe('aha'); }); + + describe('[experimental] standalone and segment', () => { + it('starts a standalone segment span if both options are set', () => { + const span = startSpan( + { + name: 'test span', + experimental: { standalone: true, segment: true }, + }, + span => { + return span; + }, + ); + + const spanJson = spanToJSON(span); + expect(spanJson.is_segment).toBe(true); + expect(spanJson.segment_id).toBe(spanJson.span_id); + expect(spanJson.segment_id).toMatch(/^[a-f0-9]{16}$/); + }); + + it('starts a non-segment span if standalone is set and segment is false', () => { + const span = startSpan( + { + name: 'test span', + experimental: { standalone: true, segment: false }, + }, + span => { + return span; + }, + ); + + const spanJson = spanToJSON(span); + expect(spanJson.is_segment).toBe(false); + expect(spanJson.segment_id).toBeUndefined(); + }); + + it('starts a standalone span (no data about segment) if only standalone is set ', () => { + const span = startSpan( + { + name: 'test span', + experimental: { standalone: true }, + }, + span => { + return span; + }, + ); + + const spanJson = spanToJSON(span); + expect(spanJson.is_segment).toBeUndefined(); + expect(spanJson.segment_id).toBeUndefined(); + }); + + it.each([undefined, false])('ignores segment if standalone is falsy (%s)', standalone => { + const span = startSpan( + { + name: 'test span', + experimental: { standalone, segment: true }, + }, + span => { + return span; + }, + ); + + const spanJson = spanToJSON(span); + expect(spanJson.is_segment).toBeUndefined(); + expect(spanJson.segment_id).toBeUndefined(); + }); + }); }); describe('startSpanManual', () => { diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 066d8e7bf1f3..da620254a144 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -58,6 +58,8 @@ export interface SpanJSON { profile_id?: string; exclusive_time?: number; measurements?: Measurements; + is_segment?: boolean; + segment_id?: string; } // These are aligned with OpenTelemetry trace flags @@ -187,6 +189,25 @@ export interface SentrySpanArguments { * Timestamp in seconds (epoch time) indicating when the span ended. */ endTimestamp?: number | undefined; + + /** + * Set to `true` if this span should be sent as a standalone span as + * opposed to a transaction. + * + * @experimental this option is currently experimental and should only be + * used within SDK code. It might be removed or changed in the future. + */ + isStandalone?: boolean | undefined; + + /** + * Set to `true` if this span is a segment span. For now, this is used for + * standalone single spans that are not part of a transaction but should + * show up in the Sentry UI (as opposed to a non-segment standalone span). + * + * @experimental this option is currently experimental and should only be + * used within SDK code. It might be removed or changed in the future. + */ + isSegment?: boolean | undefined; } /** diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts index 31874ffb6734..cb2348e67ce6 100644 --- a/packages/types/src/startSpanOptions.ts +++ b/packages/types/src/startSpanOptions.ts @@ -26,4 +26,37 @@ export interface StartSpanOptions { /** Attributes for the span. */ attributes?: SpanAttributes; + + /** + * Experimental option without any stability guarantees. Use with caution! + */ + experimental?: { + /** + * If set to true, always start a standalone span which will be sent as a + * standalone span envelope instead of a transaction envelope. + * + * @experimental this option is currently experimental and should only be + * used within SDK code. It might be removed or changed in the future. + * The payload ("envelope") of the resulting request sending the span to + * Sentry might change at any time. + * + * @private + * @hidden + */ + standalone?: boolean; + + /** + * If set to true and `standalone` is also set to `true`, the span will be + * sent as a standalone segment span (with `is_segment: true` and `segment_id`). + * + * If `standalone` is not set to `true`, this option has no effect. + * + * @experimental this option is currently experimental and should only be + * used within SDK code. It might be removed or changed in the future. + * + * @private + * @hidden + */ + segment?: boolean; + }; } From 63ce8c72f63228a01de1c0498a491c0efd737101 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Apr 2024 11:29:38 +0200 Subject: [PATCH 2/8] cleanup --- .../public-api/startSpan/standalone/segment-false/test.ts | 2 +- .../suites/public-api/startSpan/standalone/segment-true/test.ts | 2 +- .../public-api/startSpan/standalone/segment-undefined/test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts index 61a7a2e230aa..ec939865eb34 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts @@ -22,7 +22,7 @@ sentryTest('sends a span envelope with is_segment: false', async ({ getLocalTest const itemHeader = item[0]; const spanJson = item[1]; - expect(headers).toMatchObject({ + expect(headers).toEqual({ sent_at: expect.any(String), }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts index 3e08786a5d94..aa08bfa1dfe2 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts @@ -22,7 +22,7 @@ sentryTest('sends a segment span envelope', async ({ getLocalTestPath, page }) = const itemHeader = item[0]; const spanJson = item[1]; - expect(headers).toMatchObject({ + expect(headers).toEqual({ sent_at: expect.any(String), }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts index d7e8423de639..a9914732e588 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts @@ -22,7 +22,7 @@ sentryTest('sends a segment-less span envelope', async ({ getLocalTestPath, page const itemHeader = item[0]; const spanJson = item[1]; - expect(headers).toMatchObject({ + expect(headers).toEqual({ sent_at: expect.any(String), }); From 8b3627a932dfa22a43b58d42592ad637c7d754d6 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Apr 2024 11:57:44 +0200 Subject: [PATCH 3/8] jsdoc --- packages/types/src/startSpanOptions.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts index cb2348e67ce6..5fd9d3cca620 100644 --- a/packages/types/src/startSpanOptions.ts +++ b/packages/types/src/startSpanOptions.ts @@ -35,12 +35,11 @@ export interface StartSpanOptions { * If set to true, always start a standalone span which will be sent as a * standalone span envelope instead of a transaction envelope. * - * @experimental this option is currently experimental and should only be + * @internal this option is currently experimental and should only be * used within SDK code. It might be removed or changed in the future. * The payload ("envelope") of the resulting request sending the span to * Sentry might change at any time. * - * @private * @hidden */ standalone?: boolean; @@ -51,10 +50,9 @@ export interface StartSpanOptions { * * If `standalone` is not set to `true`, this option has no effect. * - * @experimental this option is currently experimental and should only be + * @internal this option is currently experimental and should only be * used within SDK code. It might be removed or changed in the future. * - * @private * @hidden */ segment?: boolean; From 922ccf33013907993c2cd80b3bb6c77fb04d497f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 23 Apr 2024 17:21:22 +0200 Subject: [PATCH 4/8] reduce to `standalone: boolean`; remove `segment` --- .../standalone/segment-false/subject.js | 1 - .../standalone/segment-false/test.ts | 47 ------------------- .../standalone/segment-true/subject.js | 1 - .../standalone/segment-undefined/test.ts | 46 ------------------ .../{segment-undefined => }/subject.js | 0 .../standalone/{segment-true => }/test.ts | 4 +- packages/core/src/tracing/sentrySpan.ts | 12 ++--- packages/core/src/tracing/trace.ts | 1 - packages/core/test/lib/tracing/trace.test.ts | 42 ++--------------- packages/types/src/span.ts | 14 +----- packages/types/src/startSpanOptions.ts | 15 +----- 11 files changed, 15 insertions(+), 168 deletions(-) delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts rename dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/{segment-undefined => }/subject.js (100%) rename dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/{segment-true => }/test.ts (92%) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js deleted file mode 100644 index e2034026cce4..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/subject.js +++ /dev/null @@ -1 +0,0 @@ -Sentry.startSpan({ name: 'standalone_segment_span', experimental: { standalone: true, segment: false } }, () => {}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts deleted file mode 100644 index ec939865eb34..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-false/test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { expect } from '@playwright/test'; -import type { SpanEnvelope } from '@sentry/types'; - -import { sentryTest } from '../../../../../utils/fixtures'; -import { - getFirstSentryEnvelopeRequest, - properFullEnvelopeRequestParser, - shouldSkipTracingTest, -} from '../../../../../utils/helpers'; - -sentryTest('sends a span envelope with is_segment: false', async ({ getLocalTestPath, page }) => { - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } - - const url = await getLocalTestPath({ testDir: __dirname }); - const spanEnvelope = await getFirstSentryEnvelopeRequest(page, url, properFullEnvelopeRequestParser); - - const headers = spanEnvelope[0]; - const item = spanEnvelope[1][0]; - - const itemHeader = item[0]; - const spanJson = item[1]; - - expect(headers).toEqual({ - sent_at: expect.any(String), - }); - - expect(itemHeader).toEqual({ - type: 'span', - }); - - expect(spanJson).toEqual({ - data: { - 'sentry.origin': 'manual', - 'sentry.sample_rate': 1, - 'sentry.source': 'custom', - }, - description: 'standalone_segment_span', - origin: 'manual', - span_id: expect.stringMatching(/^[0-9a-f]{16}$/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), - is_segment: false, - }); -}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js deleted file mode 100644 index 0c257c679faa..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/subject.js +++ /dev/null @@ -1 +0,0 @@ -Sentry.startSpan({ name: 'standalone_segment_span', experimental: { standalone: true, segment: true } }, () => {}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts deleted file mode 100644 index a9914732e588..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { expect } from '@playwright/test'; -import type { SpanEnvelope } from '@sentry/types'; - -import { sentryTest } from '../../../../../utils/fixtures'; -import { - getFirstSentryEnvelopeRequest, - properFullEnvelopeRequestParser, - shouldSkipTracingTest, -} from '../../../../../utils/helpers'; - -sentryTest('sends a segment-less span envelope', async ({ getLocalTestPath, page }) => { - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } - - const url = await getLocalTestPath({ testDir: __dirname }); - const spanEnvelope = await getFirstSentryEnvelopeRequest(page, url, properFullEnvelopeRequestParser); - - const headers = spanEnvelope[0]; - const item = spanEnvelope[1][0]; - - const itemHeader = item[0]; - const spanJson = item[1]; - - expect(headers).toEqual({ - sent_at: expect.any(String), - }); - - expect(itemHeader).toEqual({ - type: 'span', - }); - - expect(spanJson).toEqual({ - data: { - 'sentry.origin': 'manual', - 'sentry.sample_rate': 1, - 'sentry.source': 'custom', - }, - description: 'standalone_segment_span', - origin: 'manual', - span_id: expect.stringMatching(/^[0-9a-f]{16}$/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: expect.stringMatching(/^[0-9a-f]{32}$/), - }); -}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/subject.js similarity index 100% rename from dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-undefined/subject.js rename to dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/subject.js diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts similarity index 92% rename from dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts rename to dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts index aa08bfa1dfe2..aab881e34580 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/segment-true/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone/test.ts @@ -1,12 +1,12 @@ import { expect } from '@playwright/test'; import type { SpanEnvelope } from '@sentry/types'; -import { sentryTest } from '../../../../../utils/fixtures'; +import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, properFullEnvelopeRequestParser, shouldSkipTracingTest, -} from '../../../../../utils/helpers'; +} from '../../../../utils/helpers'; sentryTest('sends a segment span envelope', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 1ceef2a98fde..300c16afc6b4 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -63,9 +63,6 @@ export class SentrySpan implements Span { /** if true, treat span as a standalone span (not part of a transaction) */ private _isStandaloneSpan?: boolean; - /** send standalone span as segment span */ - private _isSegmentSpan?: boolean; - /** * You should never call the constructor manually, always use `Sentry.startSpan()` * or other span methods. @@ -106,7 +103,6 @@ export class SentrySpan implements Span { } this._isStandaloneSpan = spanContext.isStandalone; - this._isSegmentSpan = spanContext.isStandalone && spanContext.isSegment; } /** @inheritdoc */ @@ -199,8 +195,8 @@ export class SentrySpan implements Span { profile_id: this._attributes[SEMANTIC_ATTRIBUTE_PROFILE_ID] as string | undefined, exclusive_time: this._attributes[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME] as number | undefined, measurements: timedEventsToMeasurements(this._events), - is_segment: !this._isStandaloneSpan ? undefined : this._isSegmentSpan, - segment_id: this._isStandaloneSpan && this._isSegmentSpan ? this._spanId : undefined, + is_segment: this._isStandaloneSpan || undefined, + segment_id: this._isStandaloneSpan ? this._spanId : undefined, }); } @@ -240,7 +236,8 @@ export class SentrySpan implements Span { client.emit('spanEnd', this); } - // If this is not a root span, we're done, otherwise, we send it when it is ended + // If this is not a root span (== segment span) and we don't want to send it standalone, we're done. + // otherwise, we send it if (this !== getRootSpan(this)) { return; } @@ -289,6 +286,7 @@ export class SentrySpan implements Span { // The transaction span itself should be filtered out const finishedSpans = getSpanDescendants(this).filter(span => span !== this); + // TODO: filter out standalone spans! const spans = finishedSpans.map(span => spanToJSON(span)).filter(isFullFinishedSpan); const source = this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionSource | undefined; diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 16ee0e053eae..4d910f54e996 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -294,7 +294,6 @@ function normalizeContext(context: StartSpanOptions): SentrySpanArguments { const exp = context.experimental || {}; const initialCtx: SentrySpanArguments = { isStandalone: exp.standalone, - isSegment: exp.standalone && exp.segment, ...context, }; diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 4ed48ea61999..f2aa8460dba4 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -552,12 +552,12 @@ describe('startSpan', () => { expect(result).toBe('aha'); }); - describe('[experimental] standalone and segment', () => { - it('starts a standalone segment span if both options are set', () => { + describe('[experimental] standalone spans', () => { + it('starts a standalone segment span if standalone is set', () => { const span = startSpan( { name: 'test span', - experimental: { standalone: true, segment: true }, + experimental: { standalone: true }, }, span => { return span; @@ -570,43 +570,11 @@ describe('startSpan', () => { expect(spanJson.segment_id).toMatch(/^[a-f0-9]{16}$/); }); - it('starts a non-segment span if standalone is set and segment is false', () => { - const span = startSpan( - { - name: 'test span', - experimental: { standalone: true, segment: false }, - }, - span => { - return span; - }, - ); - - const spanJson = spanToJSON(span); - expect(spanJson.is_segment).toBe(false); - expect(spanJson.segment_id).toBeUndefined(); - }); - - it('starts a standalone span (no data about segment) if only standalone is set ', () => { - const span = startSpan( - { - name: 'test span', - experimental: { standalone: true }, - }, - span => { - return span; - }, - ); - - const spanJson = spanToJSON(span); - expect(spanJson.is_segment).toBeUndefined(); - expect(spanJson.segment_id).toBeUndefined(); - }); - - it.each([undefined, false])('ignores segment if standalone is falsy (%s)', standalone => { + it.each([undefined, false])("doesn't set segment properties if standalone is falsy (%s)", standalone => { const span = startSpan( { name: 'test span', - experimental: { standalone, segment: true }, + experimental: { standalone }, }, span => { return span; diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index da620254a144..cc5cb45213b9 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -191,23 +191,13 @@ export interface SentrySpanArguments { endTimestamp?: number | undefined; /** - * Set to `true` if this span should be sent as a standalone span as - * opposed to a transaction. + * Set to `true` if this span should be sent as a standalone segment span + * as opposed to a transaction. * * @experimental this option is currently experimental and should only be * used within SDK code. It might be removed or changed in the future. */ isStandalone?: boolean | undefined; - - /** - * Set to `true` if this span is a segment span. For now, this is used for - * standalone single spans that are not part of a transaction but should - * show up in the Sentry UI (as opposed to a non-segment standalone span). - * - * @experimental this option is currently experimental and should only be - * used within SDK code. It might be removed or changed in the future. - */ - isSegment?: boolean | undefined; } /** diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts index 5fd9d3cca620..84e4fc287ddf 100644 --- a/packages/types/src/startSpanOptions.ts +++ b/packages/types/src/startSpanOptions.ts @@ -33,7 +33,7 @@ export interface StartSpanOptions { experimental?: { /** * If set to true, always start a standalone span which will be sent as a - * standalone span envelope instead of a transaction envelope. + * standalone segment span envelope instead of a transaction envelope. * * @internal this option is currently experimental and should only be * used within SDK code. It might be removed or changed in the future. @@ -43,18 +43,5 @@ export interface StartSpanOptions { * @hidden */ standalone?: boolean; - - /** - * If set to true and `standalone` is also set to `true`, the span will be - * sent as a standalone segment span (with `is_segment: true` and `segment_id`). - * - * If `standalone` is not set to `true`, this option has no effect. - * - * @internal this option is currently experimental and should only be - * used within SDK code. It might be removed or changed in the future. - * - * @hidden - */ - segment?: boolean; }; } From 46182e94c6d6c0246678b4dc385116ce5c6ed549 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 24 Apr 2024 17:46:05 +0200 Subject: [PATCH 5/8] ensure standalone spans are filtered out when transaction is active --- .../standalone-mixed-transaction/init.js | 8 ++ .../standalone-mixed-transaction/subject.js | 4 + .../standalone-mixed-transaction/test.ts | 125 ++++++++++++++++++ packages/core/src/tracing/sentrySpan.ts | 58 +++++++- 4 files changed, 188 insertions(+), 7 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/init.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/init.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/init.js new file mode 100644 index 000000000000..f568455a877c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1.0, +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/subject.js new file mode 100644 index 000000000000..e504cc75b843 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/subject.js @@ -0,0 +1,4 @@ +Sentry.startSpan({ name: 'outer' }, () => { + Sentry.startSpan({ name: 'inner' }, () => {}); + Sentry.startSpan({ name: 'standalone', experimental: { standalone: true } }, () => {}); +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts new file mode 100644 index 000000000000..02392b5fead8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts @@ -0,0 +1,125 @@ +import { expect } from '@playwright/test'; +import type { Envelope, Event, EventEnvelope, EventItem, SpanEnvelope, TransactionEvent } from '@sentry/types'; + +import exp from 'constants'; +import { sentryTest } from '../../../../utils/fixtures'; +import { + getMultipleSentryEnvelopeRequests, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +sentryTest( + 'sends a transaction and a span envelope if a standalone span is created as a child of an ongoing span tree', + async ({ getLocalTestPath, page }) => { + page.on('console', msg => console.log('PAGE LOG:', msg.text())); + + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const envelopes = await getMultipleSentryEnvelopeRequests( + page, + 2, + { url, envelopeType: ['transaction', 'span'] }, + properFullEnvelopeRequestParser, + ); + + const spanEnvelope = envelopes.find(envelope => envelope[1][0][0].type === 'span') as SpanEnvelope; + const transactionEnvelope = envelopes.find(envelope => envelope[1][0][0].type === 'transaction') as EventEnvelope; + + const spanEnvelopeHeader = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + const transactionEnvelopeHeader = transactionEnvelope[0]; + const transactionEnvelopeItem = transactionEnvelope[1][0][1] as TransactionEvent; + + const traceId = transactionEnvelopeHeader.trace!.trace_id!; + const parentSpanId = transactionEnvelopeItem.contexts?.trace?.span_id; + + expect(traceId).toMatch(/[a-f0-9]{32}/); + expect(parentSpanId).toMatch(/[a-f0-9]{16}/); + + // TODO: the span envelope also needs to contain the `trace` header (follow-up PR) + expect(spanEnvelopeHeader).toEqual({ + sent_at: expect.any(String), + }); + + expect(transactionEnvelopeHeader).toEqual({ + event_id: expect.any(String), + sdk: { + name: 'sentry.javascript.browser', + version: expect.any(String), + }, + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + transaction: 'outer', + }, + }); + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.origin': 'manual', + }, + description: 'standalone', + segment_id: spanEnvelopeItem.span_id, + is_segment: true, + parent_span_id: parentSpanId, + origin: 'manual', + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: traceId, + }); + + expect(transactionEnvelopeItem).toEqual({ + contexts: { + trace: { + data: { + 'sentry.origin': 'manual', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + }, + origin: 'manual', + span_id: parentSpanId, + trace_id: traceId, + }, + }, + environment: 'production', + event_id: expect.any(String), + platform: 'javascript', + request: { + headers: expect.any(Object), + url: expect.any(String), + }, + sdk: expect.any(Object), + spans: [ + { + data: { + 'sentry.origin': 'manual', + }, + description: 'inner', + origin: 'manual', + parent_span_id: parentSpanId, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: traceId, + }, + ], + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: 'outer', + transaction_info: { + source: 'custom', + }, + type: 'transaction', + }); + }, +); diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 300c16afc6b4..8e2592fae7bb 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -13,7 +13,7 @@ import type { TransactionEvent, TransactionSource, } from '@sentry/types'; -import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; +import { consoleSandbox, dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { getClient, getCurrentScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; @@ -229,6 +229,18 @@ export class SentrySpan implements Span { return this; } + /** + * This method should generally not be used, + * but for now we need a way to publicly check if the `_isStandaloneSpan` flag is set. + * USE THIS WITH CAUTION! + * @internal + * @hidden + * @experimental + */ + public isStandaloneSpan(): boolean { + return !!this._isStandaloneSpan; + } + /** Emit `spanEnd` when the span is ended. */ private _onSpanEnded(): void { const client = getClient(); @@ -236,17 +248,36 @@ export class SentrySpan implements Span { client.emit('spanEnd', this); } - // If this is not a root span (== segment span) and we don't want to send it standalone, we're done. - // otherwise, we send it - if (this !== getRootSpan(this)) { + // A segment span is basically the root span of a local span tree. + // So for now, this is either what we previously refer to as the root span, + // or a standalone span. + const isSegmentSpan = this._isStandaloneSpan || this === getRootSpan(this); + consoleSandbox(() => { + console.log('isSegmentSpan', isSegmentSpan); + }); + + // If this is not a root span (== segment span) and we don't want to send it + if (!isSegmentSpan) { + consoleSandbox(() => { + console.log('early ret'); + }); return; } + // if this is a standalone span, we send it immediately if (this._isStandaloneSpan) { + consoleSandbox(() => { + console.log('calling sendSpanEnvelope'); + }); + sendSpanEnvelope(createSpanEnvelope([this])); return; } + consoleSandbox(() => { + console.log('converting to transaction'); + }); + const transactionEvent = this._convertSpanToTransaction(); if (transactionEvent) { const scope = getCapturedScopesOnSpan(this).scope || getCurrentScope(); @@ -283,10 +314,9 @@ export class SentrySpan implements Span { return undefined; } - // The transaction span itself should be filtered out - const finishedSpans = getSpanDescendants(this).filter(span => span !== this); + // The transaction span itself as well as any potential standalone spans should be filtered out + const finishedSpans = getSpanDescendants(this).filter(span => span !== this && !isStandaloneSpan(span)); - // TODO: filter out standalone spans! const spans = finishedSpans.map(span => spanToJSON(span)).filter(isFullFinishedSpan); const source = this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionSource | undefined; @@ -337,14 +367,28 @@ function isFullFinishedSpan(input: Partial): input is SpanJSON { return !!input.start_timestamp && !!input.timestamp && !!input.span_id && !!input.trace_id; } +/** `SentrySpan`s can be sent as a standalone span rather than belonging to a transaction */ +function isStandaloneSpan(span: Span): boolean { + return span instanceof SentrySpan && span.isStandaloneSpan(); +} + function sendSpanEnvelope(envelope: SpanEnvelope): void { + consoleSandbox(() => { + console.log('sendSpanEnvelope', envelope); + }); const client = getClient(); if (!client) { + consoleSandbox(() => { + console.log('no client'); + }); return; } const transport = client.getTransport(); if (transport) { + consoleSandbox(() => { + console.log('transport send'); + }); transport.send(envelope).then(null, reason => { DEBUG_BUILD && logger.error('Error while sending span:', reason); }); From 02b19475d89d623d4c067ec41eb681d2b1048a44 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 25 Apr 2024 09:36:40 +0200 Subject: [PATCH 6/8] cleanup --- .../standalone-mixed-transaction/test.ts | 5 +--- packages/core/src/tracing/sentrySpan.ts | 26 +------------------ 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts index 02392b5fead8..3399bcf4be66 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts @@ -1,7 +1,6 @@ import { expect } from '@playwright/test'; -import type { Envelope, Event, EventEnvelope, EventItem, SpanEnvelope, TransactionEvent } from '@sentry/types'; +import type { Envelope, EventEnvelope, SpanEnvelope, TransactionEvent } from '@sentry/types'; -import exp from 'constants'; import { sentryTest } from '../../../../utils/fixtures'; import { getMultipleSentryEnvelopeRequests, @@ -12,8 +11,6 @@ import { sentryTest( 'sends a transaction and a span envelope if a standalone span is created as a child of an ongoing span tree', async ({ getLocalTestPath, page }) => { - page.on('console', msg => console.log('PAGE LOG:', msg.text())); - if (shouldSkipTracingTest()) { sentryTest.skip(); } diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 8e2592fae7bb..4886ee0b28bc 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -13,7 +13,7 @@ import type { TransactionEvent, TransactionSource, } from '@sentry/types'; -import { consoleSandbox, dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; +import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { getClient, getCurrentScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; @@ -252,32 +252,17 @@ export class SentrySpan implements Span { // So for now, this is either what we previously refer to as the root span, // or a standalone span. const isSegmentSpan = this._isStandaloneSpan || this === getRootSpan(this); - consoleSandbox(() => { - console.log('isSegmentSpan', isSegmentSpan); - }); - // If this is not a root span (== segment span) and we don't want to send it if (!isSegmentSpan) { - consoleSandbox(() => { - console.log('early ret'); - }); return; } // if this is a standalone span, we send it immediately if (this._isStandaloneSpan) { - consoleSandbox(() => { - console.log('calling sendSpanEnvelope'); - }); - sendSpanEnvelope(createSpanEnvelope([this])); return; } - consoleSandbox(() => { - console.log('converting to transaction'); - }); - const transactionEvent = this._convertSpanToTransaction(); if (transactionEvent) { const scope = getCapturedScopesOnSpan(this).scope || getCurrentScope(); @@ -373,22 +358,13 @@ function isStandaloneSpan(span: Span): boolean { } function sendSpanEnvelope(envelope: SpanEnvelope): void { - consoleSandbox(() => { - console.log('sendSpanEnvelope', envelope); - }); const client = getClient(); if (!client) { - consoleSandbox(() => { - console.log('no client'); - }); return; } const transport = client.getTransport(); if (transport) { - consoleSandbox(() => { - console.log('transport send'); - }); transport.send(envelope).then(null, reason => { DEBUG_BUILD && logger.error('Error while sending span:', reason); }); From 5731baea5b9de740d94ee97b052a3d8ad90d9539 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 25 Apr 2024 09:38:21 +0200 Subject: [PATCH 7/8] cleanup --- packages/types/src/startSpanOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts index 84e4fc287ddf..36e5f56355c9 100644 --- a/packages/types/src/startSpanOptions.ts +++ b/packages/types/src/startSpanOptions.ts @@ -28,7 +28,7 @@ export interface StartSpanOptions { attributes?: SpanAttributes; /** - * Experimental option without any stability guarantees. Use with caution! + * Experimental options without any stability guarantees. Use with caution! */ experimental?: { /** From a2184083832c16113888334374e296cc52b58a5d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 25 Apr 2024 15:51:43 +0200 Subject: [PATCH 8/8] change segment_id and is_segment in getSpanJson --- .../public-api/startSpan/standalone-mixed-transaction/test.ts | 3 +-- packages/core/src/tracing/sentrySpan.ts | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts index 3399bcf4be66..36aab28d2a77 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/standalone-mixed-transaction/test.ts @@ -65,8 +65,7 @@ sentryTest( 'sentry.origin': 'manual', }, description: 'standalone', - segment_id: spanEnvelopeItem.span_id, - is_segment: true, + segment_id: transactionEnvelopeItem.contexts?.trace?.span_id, parent_span_id: parentSpanId, origin: 'manual', span_id: expect.stringMatching(/[a-f0-9]{16}/), diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 4886ee0b28bc..a57305632c99 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -195,8 +195,8 @@ export class SentrySpan implements Span { profile_id: this._attributes[SEMANTIC_ATTRIBUTE_PROFILE_ID] as string | undefined, exclusive_time: this._attributes[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME] as number | undefined, measurements: timedEventsToMeasurements(this._events), - is_segment: this._isStandaloneSpan || undefined, - segment_id: this._isStandaloneSpan ? this._spanId : undefined, + is_segment: (this._isStandaloneSpan && getRootSpan(this) === this) || undefined, + segment_id: this._isStandaloneSpan ? getRootSpan(this).spanContext().spanId : undefined, }); }