Skip to content

Commit 776e302

Browse files
committed
rewrite httpContext integration to use processSpan client hook
1 parent 587e3e8 commit 776e302

File tree

10 files changed

+185
-65
lines changed

10 files changed

+185
-65
lines changed

dev-packages/browser-integration-tests/suites/span-first/pageload/test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,18 @@ sentryTest('sends a span v2 envelope for the pageload', async ({ getLocalTestUrl
106106
type: 'string',
107107
value: 'url',
108108
},
109+
'sentry.idle_span_finish_reason': {
110+
type: 'string',
111+
value: 'idleTimeout',
112+
},
113+
'url.full': {
114+
type: 'string',
115+
value: 'http://sentry-test.io/index.html',
116+
},
117+
'http.request.header.user_agent': {
118+
type: 'string',
119+
value: expect.any(String),
120+
},
109121
}),
110122
trace_id: expect.stringMatching(/^[a-f\d]{32}$/),
111123
span_id: expect.stringMatching(/^[a-f\d]{16}$/),
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
traceLifecycle: 'stream',
8+
integrations: [Sentry.browserTracingIntegration(), Sentry.spanStreamingIntegration()],
9+
tracePropagationTargets: ['http://sentry-test-site.example'],
10+
tracesSampleRate: 1,
11+
sendDefaultPii: true,
12+
debug: true,
13+
});
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<div>Rendered</div>
8+
</body>
9+
</html>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../utils/fixtures';
3+
import { shouldSkipTracingTest } from '../../../../utils/helpers';
4+
import { getSpanOp, waitForV2Spans } from '../../../../utils/spanFirstUtils';
5+
6+
sentryTest('captures TTFB web vital', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipTracingTest()) {
8+
sentryTest.skip();
9+
}
10+
const pageloadSpansPromise = waitForV2Spans(page, spans => !!spans.find(span => getSpanOp(span) === 'pageload'));
11+
12+
const url = await getLocalTestUrl({ testDir: __dirname });
13+
await page.goto(url);
14+
15+
const pageloadSpan = (await pageloadSpansPromise).find(span => getSpanOp(span) === 'pageload');
16+
17+
expect(pageloadSpan).toBeDefined();
18+
19+
// If responseStart === 0, ttfb is not reported
20+
// This seems to happen somewhat randomly, so we just ignore this in that case
21+
const responseStart = await page.evaluate("performance.getEntriesByType('navigation')[0].responseStart;");
22+
if (responseStart !== 0) {
23+
expect(pageloadSpan!.attributes?.['ui.web_vital.ttfb']).toEqual({
24+
type: expect.stringMatching(/^double$/),
25+
value: expect.any(Number),
26+
});
27+
}
28+
29+
expect(pageloadSpan!.attributes?.['ui.web_vital.ttfb.requestTime']).toEqual({
30+
type: expect.stringMatching(/^integer|double$/),
31+
value: expect.any(Number),
32+
});
33+
});

packages/browser/src/integrations/httpcontext.ts

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,47 @@
1-
import { defineIntegration } from '@sentry/core';
1+
import {
2+
defineIntegration,
3+
httpHeadersToSpanAttributes,
4+
safeSetSpanAttributes,
5+
SEMANTIC_ATTRIBUTE_URL_FULL,
6+
} from '@sentry/core';
27
import { getHttpRequestData, WINDOW } from '../helpers';
38

49
/**
510
* Collects information about HTTP request headers and
611
* attaches them to the event.
712
*/
813
export const httpContextIntegration = defineIntegration(() => {
14+
const inBrowserEnvironment = WINDOW.navigator || WINDOW.location || WINDOW.document;
15+
916
return {
1017
name: 'HttpContext',
11-
// TODO (span-streaming): probably fine to omit this in favour of us globally
12-
// already adding request context data but should double-check this
18+
setup(client) {
19+
if (!inBrowserEnvironment) {
20+
return;
21+
}
22+
23+
if (client.getOptions().traceLifecycle === 'stream') {
24+
client.on('processSpan', (span, { readOnlySpan }) => {
25+
if (readOnlySpan.is_segment) {
26+
const { url, headers } = getHttpRequestData();
27+
28+
const attributeHeaders = httpHeadersToSpanAttributes(headers);
29+
30+
safeSetSpanAttributes(
31+
span,
32+
{
33+
[SEMANTIC_ATTRIBUTE_URL_FULL]: url,
34+
...attributeHeaders,
35+
},
36+
readOnlySpan.attributes,
37+
);
38+
}
39+
});
40+
}
41+
},
1342
preprocessEvent(event) {
1443
// if none of the information we want exists, don't bother
15-
if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) {
44+
if (!inBrowserEnvironment) {
1645
return;
1746
}
1847

packages/core/src/client.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import type { RequestEventData } from './types-hoist/request';
3131
import type { SdkMetadata } from './types-hoist/sdkmetadata';
3232
import type { Session, SessionAggregates } from './types-hoist/session';
3333
import type { SeverityLevel } from './types-hoist/severity';
34-
import type { Span, SpanAttributes, SpanContextData, SpanJSON } from './types-hoist/span';
34+
import type { Span, SpanAttributes, SpanContextData, SpanJSON, SpanV2JSON } from './types-hoist/span';
3535
import type { StartSpanOptions } from './types-hoist/startSpanOptions';
3636
import type { Transport, TransportMakeRequestResponse } from './types-hoist/transport';
3737
import { isV2BeforeSendSpanCallback } from './utils/beforeSendSpan';
@@ -621,6 +621,10 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
621621
* Register a callback for when the span is ready to be enqueued into the span buffer.
622622
*/
623623
public on(hook: 'enqueueSpan', callback: (span: Span) => void): () => void;
624+
/**
625+
* Register a callback for when a span is processed, to add some attributes to the span.
626+
*/
627+
public on(hook: 'processSpan', callback: (span: Span, hint: { readOnlySpan: SpanV2JSON }) => void): () => void;
624628

625629
/**
626630
* Register a callback for when an idle span is allowed to auto-finish.
@@ -897,6 +901,8 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
897901
// Hooks reserved for Span-First span processing:
898902
/** Fire a hook after the `spanEnd` hook */
899903
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;
900906
/** Fire a hook after the `segmentSpanEnd` hook is fired. */
901907
public emit(hook: 'afterSegmentSpanEnd', span: Span): void;
902908
/** Fire a hook after a span ready to be enqueued into the span buffer. */

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export {
9090
showSpanDropWarning,
9191
} from './utils/spanUtils';
9292
export { captureSpan } from './spans/captureSpan';
93+
export { safeSetSpanAttributes } from './spans/spanFirstUtils';
9394
export { attributesFromObject } from './utils/attributes';
9495
export { _setSpanForScope as _INTERNAL_setSpanForScope } from './utils/spanOnScope';
9596
export { parseSampleRate } from './utils/parseSampleRate';
Lines changed: 37 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { type RawAttributes, isAttributeObject } from '../attributes';
21
import type { Client } from '../client';
32
import { getClient, getGlobalScope } from '../currentScopes';
43
import { DEBUG_BUILD } from '../debug-build';
@@ -16,10 +15,12 @@ import {
1615
SEMANTIC_ATTRIBUTE_USER_USERNAME,
1716
} from '../semanticAttributes';
1817
import { getCapturedScopesOnSpan } from '../tracing/utils';
18+
import type { SerializedAttributes } from '../types-hoist/attributes';
1919
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';
2324

2425
/**
2526
* Captures a span and returns it to the caller, to be enqueued for sending.
@@ -36,14 +37,19 @@ export function captureSpan(span: Span, client = getClient()): void {
3637
const { isolationScope: spanIsolationScope, scope: spanScope } = getCapturedScopesOnSpan(span);
3738
const finalScopeData = getFinalScopeData(spanIsolationScope, spanScope);
3839

39-
const originalAttributeKeys = Object.keys(serializedSegmentSpan.attributes ?? {});
40+
const originalAttributes = serializedSegmentSpan.attributes ?? {};
4041

41-
applyCommonSpanAttributes(span, serializedSegmentSpan, client, finalScopeData, originalAttributeKeys);
42+
applyCommonSpanAttributes(span, serializedSegmentSpan, client, finalScopeData, originalAttributes);
4243

4344
if (span === segmentSpan) {
44-
applyScopeToSegmentSpan(span, finalScopeData, originalAttributeKeys);
45+
applyScopeToSegmentSpan(span, finalScopeData, originalAttributes);
4546
}
4647

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+
4753
// Wondering where we apply the beforeSendSpan callback?
4854
// We apply it directly before sending the span,
4955
// so whenever the buffer this span gets enqueued in is being flushed.
@@ -55,11 +61,15 @@ export function captureSpan(span: Span, client = getClient()): void {
5561
client.emit('enqueueSpan', span);
5662
}
5763

58-
function applyScopeToSegmentSpan(segmentSpan: Span, scopeData: ScopeData, originalAttributeKeys: string[]): void {
64+
function applyScopeToSegmentSpan(
65+
segmentSpan: Span,
66+
scopeData: ScopeData,
67+
originalAttributes: SerializedAttributes,
68+
): void {
5969
// TODO: Apply all scope data from auto instrumentation (contexts, request) to segment span
6070
const { attributes } = scopeData;
6171
if (attributes) {
62-
setAttributesIfNotPresent(segmentSpan, originalAttributeKeys, attributes);
72+
safeSetSpanAttributes(segmentSpan, attributes, originalAttributes);
6373
}
6474
}
6575

@@ -68,28 +78,32 @@ function applyCommonSpanAttributes(
6878
serializedSegmentSpan: SpanV2JSON,
6979
client: Client,
7080
scopeData: ScopeData,
71-
originalAttributeKeys: string[],
81+
originalAttributes: SerializedAttributes,
7282
): void {
7383
const sdk = client.getSdkMetadata();
7484
const { release, environment, sendDefaultPii } = client.getOptions();
7585

7686
// avoid overwriting any previously set attributes (from users or potentially our SDK instrumentation)
77-
setAttributesIfNotPresent(span, originalAttributeKeys, {
78-
[SEMANTIC_ATTRIBUTE_SENTRY_RELEASE]: release,
79-
[SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT]: environment,
80-
[SEMANTIC_ATTRIBUTE_SENTRY_SEGMENT_NAME]: serializedSegmentSpan.name,
81-
[SEMANTIC_ATTRIBUTE_SENTRY_SEGMENT_ID]: serializedSegmentSpan.span_id,
82-
[SEMANTIC_ATTRIBUTE_SENTRY_SDK_NAME]: sdk?.sdk?.name,
83-
[SEMANTIC_ATTRIBUTE_SENTRY_SDK_VERSION]: sdk?.sdk?.version,
84-
...(sendDefaultPii
85-
? {
86-
[SEMANTIC_ATTRIBUTE_USER_ID]: scopeData.user?.id,
87-
[SEMANTIC_ATTRIBUTE_USER_EMAIL]: scopeData.user?.email,
88-
[SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS]: scopeData.user?.ip_address ?? undefined,
89-
[SEMANTIC_ATTRIBUTE_USER_USERNAME]: scopeData.user?.username,
90-
}
91-
: {}),
92-
});
87+
safeSetSpanAttributes(
88+
span,
89+
{
90+
[SEMANTIC_ATTRIBUTE_SENTRY_RELEASE]: release,
91+
[SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT]: environment,
92+
[SEMANTIC_ATTRIBUTE_SENTRY_SEGMENT_NAME]: serializedSegmentSpan.name,
93+
[SEMANTIC_ATTRIBUTE_SENTRY_SEGMENT_ID]: serializedSegmentSpan.span_id,
94+
[SEMANTIC_ATTRIBUTE_SENTRY_SDK_NAME]: sdk?.sdk?.name,
95+
[SEMANTIC_ATTRIBUTE_SENTRY_SDK_VERSION]: sdk?.sdk?.version,
96+
...(sendDefaultPii
97+
? {
98+
[SEMANTIC_ATTRIBUTE_USER_ID]: scopeData.user?.id,
99+
[SEMANTIC_ATTRIBUTE_USER_EMAIL]: scopeData.user?.email,
100+
[SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS]: scopeData.user?.ip_address ?? undefined,
101+
[SEMANTIC_ATTRIBUTE_USER_USERNAME]: scopeData.user?.username,
102+
}
103+
: {}),
104+
},
105+
originalAttributes,
106+
);
93107
}
94108

95109
// TODO: Extract this to a helper in core. It's used in multiple places.
@@ -103,35 +117,3 @@ function getFinalScopeData(isolationScope: Scope | undefined, scope: Scope | und
103117
}
104118
return finalScopeData;
105119
}
106-
107-
function setAttributesIfNotPresent(
108-
span: Span,
109-
originalAttributeKeys: string[],
110-
newAttributes: RawAttributes<Record<string, unknown>>,
111-
): void {
112-
Object.keys(newAttributes).forEach(key => {
113-
if (!originalAttributeKeys.includes(key)) {
114-
setAttributeOnSpanWithMaybeUnit(span, key, newAttributes[key]);
115-
}
116-
});
117-
}
118-
119-
function setAttributeOnSpanWithMaybeUnit(span: Span, attributeKey: string, attributeValue: unknown): void {
120-
if (isAttributeObject(attributeValue)) {
121-
const { value, unit } = attributeValue;
122-
123-
if (isSupportedAttributeType(value)) {
124-
span.setAttribute(attributeKey, value);
125-
}
126-
127-
if (unit) {
128-
span.setAttribute(`${attributeKey}.unit`, unit);
129-
}
130-
} else if (isSupportedAttributeType(attributeValue)) {
131-
span.setAttribute(attributeKey, attributeValue);
132-
}
133-
}
134-
135-
function isSupportedAttributeType(value: unknown): value is Parameters<Span['setAttribute']>[1] {
136-
return ['string', 'number', 'boolean'].includes(typeof value) || Array.isArray(value);
137-
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import type { RawAttributes } from '../attributes';
2+
import { isAttributeObject } from '../attributes';
3+
import type { SerializedAttributes } from '../types-hoist/attributes';
4+
import type { Span } from '../types-hoist/span';
5+
6+
/**
7+
* Only set a span attribute if it is not already set.
8+
*/
9+
export function safeSetSpanAttributes(
10+
span: Span,
11+
newAttributes: RawAttributes<Record<string, unknown>>,
12+
originalAttributeKeys: SerializedAttributes | undefined,
13+
): void {
14+
Object.keys(newAttributes).forEach(key => {
15+
if (!originalAttributeKeys?.[key]) {
16+
setAttributeOnSpanWithMaybeUnit(span, key, newAttributes[key]);
17+
}
18+
});
19+
}
20+
21+
function setAttributeOnSpanWithMaybeUnit(span: Span, attributeKey: string, attributeValue: unknown): void {
22+
if (isAttributeObject(attributeValue)) {
23+
const { value, unit } = attributeValue;
24+
25+
if (isSupportedAttributeType(value)) {
26+
span.setAttribute(attributeKey, value);
27+
}
28+
29+
if (unit) {
30+
span.setAttribute(`${attributeKey}.unit`, unit);
31+
}
32+
} else if (isSupportedAttributeType(attributeValue)) {
33+
span.setAttribute(attributeKey, attributeValue);
34+
}
35+
}
36+
37+
function isSupportedAttributeType(value: unknown): value is Parameters<Span['setAttribute']>[1] {
38+
return ['string', 'number', 'boolean'].includes(typeof value) || Array.isArray(value);
39+
}

packages/core/src/utils/spanUtils.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,7 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef
314314
* Convert the various statuses to the ones expected by Sentry ('ok' is default)
315315
*/
316316
export function getV2StatusMessage(status: SpanStatus | undefined): 'ok' | 'error' {
317-
return !status ||
318-
status.code === SPAN_STATUS_UNSET ||
319-
(status.code === SPAN_STATUS_ERROR && status.message === 'unknown_error')
320-
? 'ok'
321-
: 'error';
317+
return !status || status.code === SPAN_STATUS_OK || status.code === SPAN_STATUS_UNSET ? 'ok' : 'error';
322318
}
323319

324320
/**

0 commit comments

Comments
 (0)