Skip to content

Commit 7473fb2

Browse files
committed
rewrite pipeline to just always use spanJSonV2 because thanks OTel
1 parent 571a961 commit 7473fb2

File tree

11 files changed

+133
-68
lines changed

11 files changed

+133
-68
lines changed

.vscode/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,6 @@
2828
"editor.defaultFormatter": "esbenp.prettier-vscode",
2929
"[typescript]": {
3030
"editor.defaultFormatter": "esbenp.prettier-vscode"
31-
}
31+
},
32+
"angular.enable-strict-mode-prompt": false
3233
}

packages/browser/src/integrations/httpcontext.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {
22
defineIntegration,
33
httpHeadersToSpanAttributes,
4-
safeSetSpanAttributes,
4+
safeSetSpanJSONAttributes,
55
SEMANTIC_ATTRIBUTE_URL_FULL,
66
} from '@sentry/core';
77
import { getHttpRequestData, WINDOW } from '../helpers';
@@ -21,19 +21,19 @@ export const httpContextIntegration = defineIntegration(() => {
2121
}
2222

2323
if (client.getOptions().traceLifecycle === 'stream') {
24-
client.on('processSpan', (span, { readOnlySpan }) => {
25-
if (readOnlySpan.is_segment) {
24+
client.on('processSpan', spanJSON => {
25+
if (spanJSON.is_segment) {
2626
const { url, headers } = getHttpRequestData();
2727

2828
const attributeHeaders = httpHeadersToSpanAttributes(headers);
2929

30-
safeSetSpanAttributes(
31-
span,
30+
safeSetSpanJSONAttributes(
31+
spanJSON,
3232
{
3333
[SEMANTIC_ATTRIBUTE_URL_FULL]: url,
3434
...attributeHeaders,
3535
},
36-
readOnlySpan.attributes,
36+
spanJSON.attributes,
3737
);
3838
}
3939
});

packages/browser/src/integrations/spanstreaming.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
import type { Client, IntegrationFn, Span, SpanV2JSON } from '@sentry/core';
1+
import type { Client, IntegrationFn, Span, SpanV2JSON, SpanV2JSONWithSegmentRef } from '@sentry/core';
22
import {
33
captureSpan,
44
createSpanV2Envelope,
55
debug,
66
defineIntegration,
77
getDynamicSamplingContextFromSpan,
8-
INTERNAL_getSegmentSpan,
98
isV2BeforeSendSpanCallback,
109
showSpanDropWarning,
11-
spanToV2JSON,
1210
} from '@sentry/core';
1311
import { DEBUG_BUILD } from '../debug-build';
1412

@@ -35,7 +33,7 @@ export const spanStreamingIntegration = defineIntegration(((userOptions?: Partia
3533
};
3634

3735
// key: traceId-segmentSpanId
38-
const spanTreeMap = new Map<string, Set<Span>>();
36+
const spanTreeMap = new Map<string, Set<SpanV2JSONWithSegmentRef>>();
3937

4038
return {
4139
name: 'SpanStreaming',
@@ -57,13 +55,13 @@ export const spanStreamingIntegration = defineIntegration(((userOptions?: Partia
5755
return;
5856
}
5957

60-
client.on('enqueueSpan', span => {
61-
const spanTreeMapKey = getSpanTreeMapKey(span);
58+
client.on('enqueueSpan', spanJSON => {
59+
const spanTreeMapKey = getSpanTreeMapKey(spanJSON as SpanV2JSONWithSegmentRef);
6260
const spanBuffer = spanTreeMap.get(spanTreeMapKey);
6361
if (spanBuffer) {
64-
spanBuffer.add(span);
62+
spanBuffer.add(spanJSON as SpanV2JSONWithSegmentRef);
6563
} else {
66-
spanTreeMap.set(spanTreeMapKey, new Set([span]));
64+
spanTreeMap.set(spanTreeMapKey, new Set([spanJSON as SpanV2JSONWithSegmentRef]));
6765
}
6866
});
6967

@@ -87,37 +85,42 @@ export const spanStreamingIntegration = defineIntegration(((userOptions?: Partia
8785

8886
interface SpanProcessingOptions {
8987
client: Client;
90-
spanTreeMap: Map<string, Set<Span>>;
88+
spanTreeMap: Map<string, Set<SpanV2JSONWithSegmentRef>>;
9189
batchLimit: number;
9290
beforeSendSpan: ((span: SpanV2JSON) => SpanV2JSON) | undefined;
9391
}
9492

9593
/**
9694
* Just the traceid alone isn't enough because there can be multiple span trees with the same traceid.
9795
*/
98-
function getSpanTreeMapKey(span: Span): string {
99-
return `${span.spanContext().traceId}-${INTERNAL_getSegmentSpan(span).spanContext().spanId}`;
96+
function getSpanTreeMapKey(spanJSON: SpanV2JSONWithSegmentRef): string {
97+
return `${spanJSON.trace_id}-${spanJSON._segmentSpan?.spanContext().spanId || spanJSON.span_id}`;
10098
}
10199

102100
function sendSegment(
103101
segmentSpan: Span,
104102
{ client, spanTreeMap, batchLimit, beforeSendSpan }: SpanProcessingOptions,
105103
): void {
106104
const traceId = segmentSpan.spanContext().traceId;
107-
const spanTreeMapKey = getSpanTreeMapKey(segmentSpan);
105+
const segmentSpanId = segmentSpan.spanContext().spanId;
106+
const spanTreeMapKey = `${traceId}-${segmentSpanId}`;
108107
const spansOfTrace = spanTreeMap.get(spanTreeMapKey);
109108

110109
if (!spansOfTrace?.size) {
111110
spanTreeMap.delete(spanTreeMapKey);
112111
return;
113112
}
114113

115-
const finalSpans = Array.from(spansOfTrace).map(span => {
116-
const spanJson = spanToV2JSON(span);
114+
// Apply beforeSendSpan callback and clean up segment span references
115+
const finalSpans = Array.from(spansOfTrace).map(spanJSON => {
116+
// Remove the segment span reference before processing
117+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
118+
const { _segmentSpan, ...cleanSpanJSON } = spanJSON;
119+
117120
if (beforeSendSpan) {
118-
return applyBeforeSendSpanCallback(spanJson, beforeSendSpan);
121+
return applyBeforeSendSpanCallback(cleanSpanJSON, beforeSendSpan);
119122
}
120-
return spanJson;
123+
return cleanSpanJSON;
121124
});
122125

123126
const batches: SpanV2JSON[][] = [];
@@ -127,6 +130,7 @@ function sendSegment(
127130

128131
DEBUG_BUILD && debug.log(`Sending trace ${traceId} in ${batches.length} batch${batches.length === 1 ? '' : 'es'}`);
129132

133+
// Compute DSC from the segment span (passed as parameter)
130134
const dsc = getDynamicSamplingContextFromSpan(segmentSpan);
131135

132136
for (const batch of batches) {

packages/core/src/client.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -618,13 +618,13 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
618618
*/
619619
public on(hook: 'afterSegmentSpanEnd', callback: (span: Span) => void): () => void;
620620
/**
621-
* Register a callback for when the span is ready to be enqueued into the span buffer.
621+
* Register a callback for when the span JSON is ready to be enqueued into the span buffer.
622622
*/
623-
public on(hook: 'enqueueSpan', callback: (span: Span) => void): () => void;
623+
public on(hook: 'enqueueSpan', callback: (spanJSON: SpanV2JSON) => void): () => void;
624624
/**
625-
* Register a callback for when a span is processed, to add some attributes to the span.
625+
* Register a callback for when a span JSON is processed, to add some attributes to the span JSON.
626626
*/
627-
public on(hook: 'processSpan', callback: (span: Span, hint: { readOnlySpan: SpanV2JSON }) => void): () => void;
627+
public on(hook: 'processSpan', callback: (spanJSON: SpanV2JSON, hint: { readOnlySpan: Span }) => void): () => void;
628628

629629
/**
630630
* Register a callback for when an idle span is allowed to auto-finish.
@@ -901,12 +901,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
901901
// Hooks reserved for Span-First span processing:
902902
/** Fire a hook after the `spanEnd` hook */
903903
public emit(hook: 'afterSpanEnd', span: Span): void;
904-
/** Fire a hook after a span is processed, to add some attributes to the span. */
905-
public emit(hook: 'processSpan', span: Span, hint: { readOnlySpan: SpanV2JSON }): void;
904+
/** Fire a hook after a span is processed, to add some attributes to the span JSON. */
905+
public emit(hook: 'processSpan', spanJSON: SpanV2JSON, hint: { readOnlySpan: Span }): void;
906906
/** Fire a hook after the `segmentSpanEnd` hook is fired. */
907907
public emit(hook: 'afterSegmentSpanEnd', span: Span): void;
908908
/** Fire a hook after a span ready to be enqueued into the span buffer. */
909-
public emit(hook: 'enqueueSpan', span: Span): void;
909+
public emit(hook: 'enqueueSpan', spanJSON: SpanV2JSON): void;
910910

911911
/**
912912
* Fire a hook indicating that an idle span is allowed to auto finish.

packages/core/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ export {
9090
showSpanDropWarning,
9191
} from './utils/spanUtils';
9292
export { captureSpan } from './spans/captureSpan';
93-
export { safeSetSpanAttributes } from './spans/spanFirstUtils';
93+
export type { SpanV2JSONWithSegmentRef } from './spans/captureSpan';
94+
export { safeSetSpanAttributes, safeSetSpanJSONAttributes } from './spans/spanFirstUtils';
9495
export { attributesFromObject } from './utils/attributes';
9596
export { _setSpanForScope as _INTERNAL_setSpanForScope } from './utils/spanOnScope';
9697
export { parseSampleRate } from './utils/parseSampleRate';

packages/core/src/spans/captureSpan.ts

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,32 @@ import type { Span, SpanV2JSON } from '../types-hoist/span';
2020
import { mergeScopeData } from '../utils/applyScopeDataToEvent';
2121
import { debug } from '../utils/debug-logger';
2222
import { INTERNAL_getSegmentSpan, spanToV2JSON } from '../utils/spanUtils';
23-
import { safeSetSpanAttributes } from './spanFirstUtils';
23+
import { safeSetSpanJSONAttributes } from './spanFirstUtils';
2424

2525
/**
26-
* Captures a span and returns it to the caller, to be enqueued for sending.
26+
* A SpanV2JSON with an attached reference to the segment span.
27+
* This reference is used to compute dynamic sampling context before sending.
28+
* The reference MUST be removed before sending the span envelope.
2729
*/
28-
export function captureSpan(span: Span, client = getClient()): Span | void {
30+
export interface SpanV2JSONWithSegmentRef extends SpanV2JSON {
31+
_segmentSpan: Span;
32+
}
33+
34+
/**
35+
* Captures a span and returns a JSON representation to be enqueued for sending.
36+
*
37+
* IMPORTANT: This function converts the span to JSON immediately to avoid writing
38+
* to an already-ended OTel span instance (which is blocked by the OTel Span class).
39+
*/
40+
export function captureSpan(span: Span, client = getClient()): SpanV2JSONWithSegmentRef | void {
2941
if (!client) {
3042
DEBUG_BUILD && debug.warn('No client available to capture span.');
3143
return;
3244
}
3345

46+
// Convert to JSON FIRST - we cannot write to an already-ended span
47+
const spanJSON = spanToV2JSON(span) as SpanV2JSONWithSegmentRef;
48+
3449
const segmentSpan = INTERNAL_getSegmentSpan(span);
3550
const serializedSegmentSpan = spanToV2JSON(segmentSpan);
3651

@@ -39,44 +54,39 @@ export function captureSpan(span: Span, client = getClient()): Span | void {
3954

4055
const originalAttributes = serializedSegmentSpan.attributes ?? {};
4156

42-
applyCommonSpanAttributes(span, serializedSegmentSpan, client, finalScopeData, originalAttributes);
57+
applyCommonSpanAttributes(spanJSON, serializedSegmentSpan, client, finalScopeData, originalAttributes);
4358

4459
if (span === segmentSpan) {
45-
applyScopeToSegmentSpan(span, finalScopeData, originalAttributes);
60+
applyScopeToSegmentSpan(spanJSON, finalScopeData, originalAttributes);
4661
}
4762

48-
// Allow integrations to add additional data to span. Pass in a serialized
49-
// span to avoid having to potentially serialize the span in every integration
50-
// (for improved performance).
51-
client.emit('processSpan', span, { readOnlySpan: spanToV2JSON(span) });
52-
53-
// Wondering where we apply the beforeSendSpan callback?
54-
// We apply it directly before sending the span,
55-
// so whenever the buffer this span gets enqueued in is being flushed.
56-
// Why? Because we have to enqueue the span instance itself, not a JSON object.
57-
// We could temporarily convert to JSON here but this means that we'd then again
58-
// have to mutate the `span` instance (doesn't work for every kind of object mutation)
59-
// or construct a fully new span object. The latter is risky because users (or we) could hold
60-
// references to the original span instance.
61-
client.emit('enqueueSpan', span);
62-
63-
return span;
63+
// Attach segment span reference for DSC generation at send time
64+
spanJSON._segmentSpan = segmentSpan;
65+
66+
// Allow integrations to add additional data to the span JSON
67+
client.emit('processSpan', spanJSON, { readOnlySpan: span });
68+
69+
// Enqueue the JSON representation for sending
70+
// Note: We now enqueue JSON instead of the span instance to avoid mutating ended spans
71+
client.emit('enqueueSpan', spanJSON);
72+
73+
return spanJSON;
6474
}
6575

6676
function applyScopeToSegmentSpan(
67-
segmentSpan: Span,
77+
segmentSpanJSON: SpanV2JSON,
6878
scopeData: ScopeData,
6979
originalAttributes: SerializedAttributes,
7080
): void {
7181
// TODO: Apply all scope data from auto instrumentation (contexts, request) to segment span
7282
const { attributes } = scopeData;
7383
if (attributes) {
74-
safeSetSpanAttributes(segmentSpan, attributes, originalAttributes);
84+
safeSetSpanJSONAttributes(segmentSpanJSON, attributes, originalAttributes);
7585
}
7686
}
7787

7888
function applyCommonSpanAttributes(
79-
span: Span,
89+
spanJSON: SpanV2JSON,
8090
serializedSegmentSpan: SpanV2JSON,
8191
client: Client,
8292
scopeData: ScopeData,
@@ -86,8 +96,8 @@ function applyCommonSpanAttributes(
8696
const { release, environment, sendDefaultPii } = client.getOptions();
8797

8898
// avoid overwriting any previously set attributes (from users or potentially our SDK instrumentation)
89-
safeSetSpanAttributes(
90-
span,
99+
safeSetSpanJSONAttributes(
100+
spanJSON,
91101
{
92102
[SEMANTIC_ATTRIBUTE_SENTRY_RELEASE]: release,
93103
[SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT]: environment,

packages/core/src/spans/spanFirstUtils.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { RawAttributes } from '../attributes';
22
import { isAttributeObject } from '../attributes';
33
import type { SerializedAttributes } from '../types-hoist/attributes';
4-
import type { Span } from '../types-hoist/span';
4+
import type { Span, SpanV2JSON } from '../types-hoist/span';
5+
import { attributeValueToSerializedAttribute } from '../utils/attributes';
56

67
/**
78
* Only set a span attribute if it is not already set.
@@ -18,6 +19,26 @@ export function safeSetSpanAttributes(
1819
});
1920
}
2021

22+
/**
23+
* Only set a span JSON attribute if it is not already set.
24+
* This is used to safely set attributes on JSON objects without mutating already-ended span instances.
25+
*/
26+
export function safeSetSpanJSONAttributes(
27+
spanJSON: SpanV2JSON,
28+
newAttributes: RawAttributes<Record<string, unknown>>,
29+
originalAttributeKeys: SerializedAttributes | undefined,
30+
): void {
31+
if (!spanJSON.attributes) {
32+
spanJSON.attributes = {};
33+
}
34+
35+
Object.keys(newAttributes).forEach(key => {
36+
if (!originalAttributeKeys?.[key]) {
37+
setAttributeOnSpanJSONWithMaybeUnit(spanJSON, key, newAttributes[key]);
38+
}
39+
});
40+
}
41+
2142
function setAttributeOnSpanWithMaybeUnit(span: Span, attributeKey: string, attributeValue: unknown): void {
2243
if (isAttributeObject(attributeValue)) {
2344
const { value, unit } = attributeValue;
@@ -34,6 +55,35 @@ function setAttributeOnSpanWithMaybeUnit(span: Span, attributeKey: string, attri
3455
}
3556
}
3657

58+
function setAttributeOnSpanJSONWithMaybeUnit(
59+
spanJSON: SpanV2JSON,
60+
attributeKey: string,
61+
attributeValue: unknown,
62+
): void {
63+
// Ensure attributes object exists (it's initialized in safeSetSpanJSONAttributes)
64+
if (!spanJSON.attributes) {
65+
return;
66+
}
67+
68+
if (isAttributeObject(attributeValue)) {
69+
const { value, unit } = attributeValue;
70+
71+
if (isSupportedSerializableType(value)) {
72+
spanJSON.attributes[attributeKey] = attributeValueToSerializedAttribute(value);
73+
}
74+
75+
if (unit) {
76+
spanJSON.attributes[`${attributeKey}.unit`] = attributeValueToSerializedAttribute(unit);
77+
}
78+
} else if (isSupportedSerializableType(attributeValue)) {
79+
spanJSON.attributes[attributeKey] = attributeValueToSerializedAttribute(attributeValue);
80+
}
81+
}
82+
3783
function isSupportedAttributeType(value: unknown): value is Parameters<Span['setAttribute']>[1] {
3884
return ['string', 'number', 'boolean'].includes(typeof value) || Array.isArray(value);
3985
}
86+
87+
function isSupportedSerializableType(value: unknown): boolean {
88+
return ['string', 'number', 'boolean'].includes(typeof value) || Array.isArray(value);
89+
}

packages/node-core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ export {
136136
wrapMcpServerWithSentry,
137137
featureFlagsIntegration,
138138
metrics,
139+
withStreamSpan,
139140
} from '@sentry/core';
140141

141142
export type {

packages/node/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,4 +191,5 @@ export {
191191
cron,
192192
NODE_VERSION,
193193
validateOpenTelemetrySetup,
194+
withStreamSpan,
194195
} from '@sentry/node-core';

0 commit comments

Comments
 (0)