Skip to content

Commit

Permalink
feat(tracing): Add PropagationContext to scope (#8421)
Browse files Browse the repository at this point in the history
ref #8352

For more details about PropagationContext, see
https://www.notion.so/sentry/Tracing-without-performance-efab307eb7f64e71a04f09dc72722530

Building off of work in both
#8403 and
#8418, this PR adds
`PropagationContext` and uses that to always set a trace context on
outgoing error events.

Currently if there is an active span on the scope, we automatically
attach that span's trace context to all outgoing events. Now, we want to
rely on either the active span or fallback to the propagation context to
ensure that there is always a trace being generated and propagated.

Next up we'll work on updating the node/browser SDKs to update the
propagation context. For example, we should update the propagation
context for node based on the incoming sentry-trace/baggage headers.
  • Loading branch information
AbhiPrasad committed Jun 29, 2023
1 parent 7de917e commit f1ede57
Show file tree
Hide file tree
Showing 16 changed files with 188 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT
version: SDK_VERSION,
name: 'sentry.javascript.browser',
},
sdkProcessingMetadata: {},
request: {
url: expect.stringContaining('/dist/index.html'),
headers: {
Expand Down Expand Up @@ -94,7 +93,6 @@ sentryTest('should capture replays (@sentry/browser export)', async ({ getLocalT
version: SDK_VERSION,
name: 'sentry.javascript.browser',
},
sdkProcessingMetadata: {},
request: {
url: expect.stringContaining('/dist/index.html'),
headers: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ sentryTest('should capture replays (@sentry/replay export)', async ({ getLocalTe
version: SDK_VERSION,
name: 'sentry.javascript.browser',
},
sdkProcessingMetadata: {},
request: {
url: expect.stringContaining('/dist/index.html'),
headers: {
Expand Down Expand Up @@ -94,7 +93,6 @@ sentryTest('should capture replays (@sentry/replay export)', async ({ getLocalTe
version: SDK_VERSION,
name: 'sentry.javascript.browser',
},
sdkProcessingMetadata: {},
request: {
url: expect.stringContaining('/dist/index.html'),
headers: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const DEFAULT_REPLAY_EVENT = {
version: SDK_VERSION,
name: 'sentry.javascript.browser',
},
sdkProcessingMetadata: {},
request: {
url: expect.stringContaining('/dist/index.html'),
headers: {
Expand Down
47 changes: 46 additions & 1 deletion packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
Integration,
IntegrationClass,
Outcome,
PropagationContext,
SdkMetadata,
Session,
SessionAggregates,
Expand All @@ -29,6 +30,7 @@ import {
addItemToEnvelope,
checkOrSetAlreadyCaught,
createAttachmentEnvelopeItem,
dropUndefinedKeys,
isPlainObject,
isPrimitive,
isThenable,
Expand All @@ -41,6 +43,7 @@ import {
} from '@sentry/utils';

import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
import { DEFAULT_ENVIRONMENT } from './constants';
import { createEventEnvelope, createSessionEnvelope } from './envelope';
import type { IntegrationIndex } from './integration';
import { setupIntegration, setupIntegrations } from './integration';
Expand Down Expand Up @@ -507,7 +510,49 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
if (!hint.integrations && integrations.length > 0) {
hint.integrations = integrations;
}
return prepareEvent(options, event, hint, scope);
return prepareEvent(options, event, hint, scope).then(evt => {
if (evt === null) {
return evt;
}

// If a trace context is not set on the event, we use the propagationContext set on the event to
// generate a trace context. If the propagationContext does not have a dynamic sampling context, we
// also generate one for it.
const { propagationContext } = evt.sdkProcessingMetadata || {};
const trace = evt.contexts && evt.contexts.trace;
if (!trace && propagationContext) {
const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext as PropagationContext;
evt.contexts = {
trace: {
trace_id,
span_id: spanId,
parent_span_id: parentSpanId,
},
...evt.contexts,
};

const { publicKey: public_key } = this.getDsn() || {};
const { segment: user_segment } = (scope && scope.getUser()) || {};

let dynamicSamplingContext = dsc;
if (!dsc) {
dynamicSamplingContext = dropUndefinedKeys({
environment: options.environment || DEFAULT_ENVIRONMENT,
release: options.release,
user_segment,
public_key,
trace_id,
});
this.emit && this.emit('createDsc', dynamicSamplingContext);
}

evt.sdkProcessingMetadata = {
dynamicSamplingContext,
...evt.sdkProcessingMetadata,
};
}
return evt;
});
}

/**
Expand Down
36 changes: 35 additions & 1 deletion packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
Extra,
Extras,
Primitive,
PropagationContext,
RequestSession,
Scope as ScopeInterface,
ScopeContext,
Expand All @@ -29,6 +30,7 @@ import {
isThenable,
logger,
SyncPromise,
uuid4,
} from '@sentry/utils';

import { updateSession } from './session';
Expand Down Expand Up @@ -70,6 +72,9 @@ export class Scope implements ScopeInterface {
/** Attachments */
protected _attachments: Attachment[];

/** Propagation Context for distributed tracing */
protected _propagationContext: PropagationContext;

/**
* A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get
* sent to Sentry
Expand Down Expand Up @@ -108,6 +113,7 @@ export class Scope implements ScopeInterface {
this._extra = {};
this._contexts = {};
this._sdkProcessingMetadata = {};
this._propagationContext = generatePropagationContext();
}

/**
Expand All @@ -131,6 +137,7 @@ export class Scope implements ScopeInterface {
newScope._requestSession = scope._requestSession;
newScope._attachments = [...scope._attachments];
newScope._sdkProcessingMetadata = { ...scope._sdkProcessingMetadata };
newScope._propagationContext = { ...scope._propagationContext };
}
return newScope;
}
Expand Down Expand Up @@ -347,6 +354,9 @@ export class Scope implements ScopeInterface {
if (captureContext._requestSession) {
this._requestSession = captureContext._requestSession;
}
if (captureContext._propagationContext) {
this._propagationContext = captureContext._propagationContext;
}
} else if (isPlainObject(captureContext)) {
// eslint-disable-next-line no-param-reassign
captureContext = captureContext as ScopeContext;
Expand All @@ -365,6 +375,9 @@ export class Scope implements ScopeInterface {
if (captureContext.requestSession) {
this._requestSession = captureContext.requestSession;
}
if (captureContext.propagationContext) {
this._propagationContext = captureContext.propagationContext;
}
}

return this;
Expand All @@ -387,6 +400,7 @@ export class Scope implements ScopeInterface {
this._session = undefined;
this._notifyScopeListeners();
this._attachments = [];
this._propagationContext = generatePropagationContext();
return this;
}

Expand Down Expand Up @@ -500,7 +514,11 @@ export class Scope implements ScopeInterface {
event.breadcrumbs = [...(event.breadcrumbs || []), ...this._breadcrumbs];
event.breadcrumbs = event.breadcrumbs.length > 0 ? event.breadcrumbs : undefined;

event.sdkProcessingMetadata = { ...event.sdkProcessingMetadata, ...this._sdkProcessingMetadata };
event.sdkProcessingMetadata = {
...event.sdkProcessingMetadata,
...this._sdkProcessingMetadata,
propagationContext: this._propagationContext,
};

return this._notifyEventProcessors([...getGlobalEventProcessors(), ...this._eventProcessors], event, hint);
}
Expand All @@ -514,6 +532,14 @@ export class Scope implements ScopeInterface {
return this;
}

/**
* @inheritdoc
*/
public setPropagationContext(context: PropagationContext): this {
this._propagationContext = context;
return this;
}

/**
* This will be called after {@link applyToEvent} is finished.
*/
Expand Down Expand Up @@ -598,3 +624,11 @@ function getGlobalEventProcessors(): EventProcessor[] {
export function addGlobalEventProcessor(callback: EventProcessor): void {
getGlobalEventProcessors().push(callback);
}

function generatePropagationContext(): PropagationContext {
return {
traceId: uuid4(),
spanId: uuid4().substring(16),
sampled: false,
};
}
22 changes: 22 additions & 0 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,28 @@ describe('BaseClient', () => {
);
});

test('it adds a trace context all events', () => {
expect.assertions(1);

const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
const client = new TestClient(options);
const scope = new Scope();

client.captureEvent({ message: 'message' }, { event_id: 'wat' }, scope);

expect(TestClient.instance!.event!).toEqual(
expect.objectContaining({
contexts: {
trace: {
parent_span_id: undefined,
span_id: expect.any(String),
trace_id: expect.any(String),
},
},
}),
);
});

test('adds `event_id` from hint if available', () => {
expect.assertions(1);

Expand Down
54 changes: 52 additions & 2 deletions packages/hub/test/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@ describe('Scope', () => {
GLOBAL_OBJ.__SENTRY__.globalEventProcessors = undefined;
});

describe('init', () => {
test('it creates a propagation context', () => {
const scope = new Scope();

// @ts-ignore asserting on private properties
expect(scope._propagationContext).toEqual({
traceId: expect.any(String),
spanId: expect.any(String),
sampled: false,
dsc: undefined,
parentSpanId: undefined,
});
});
});

describe('attributes modification', () => {
test('setFingerprint', () => {
const scope = new Scope();
Expand Down Expand Up @@ -193,6 +208,14 @@ describe('Scope', () => {
expect(parentScope.getRequestSession()).toEqual({ status: 'ok' });
expect(scope.getRequestSession()).toEqual({ status: 'ok' });
});

test('should clone propagation context', () => {
const parentScope = new Scope();
const scope = Scope.clone(parentScope);

// @ts-ignore accessing private property for test
expect(scope._propagationContext).toEqual(parentScope._propagationContext);
});
});

describe('applyToEvent', () => {
Expand Down Expand Up @@ -220,7 +243,11 @@ describe('Scope', () => {
expect(processedEvent!.transaction).toEqual('/abc');
expect(processedEvent!.breadcrumbs![0]).toHaveProperty('message', 'test');
expect(processedEvent!.contexts).toEqual({ os: { id: '1' } });
expect(processedEvent!.sdkProcessingMetadata).toEqual({ dogs: 'are great!' });
expect(processedEvent!.sdkProcessingMetadata).toEqual({
dogs: 'are great!',
// @ts-expect-error accessing private property for test
propagationContext: scope._propagationContext,
});
});
});

Expand Down Expand Up @@ -339,7 +366,7 @@ describe('Scope', () => {
scope.setSpan(span);
const event: Event = {
contexts: {
trace: { a: 'c' },
trace: { a: 'c' } as any,
},
};
return scope.applyToEvent(event).then(processedEvent => {
Expand Down Expand Up @@ -383,6 +410,8 @@ describe('Scope', () => {

test('clear', () => {
const scope = new Scope();
// @ts-expect-error accessing private property
const oldPropagationContext = scope._propagationContext;
scope.setExtra('a', 2);
scope.setTag('a', 'b');
scope.setUser({ id: '1' });
Expand All @@ -393,6 +422,14 @@ describe('Scope', () => {
scope.clear();
expect((scope as any)._extra).toEqual({});
expect((scope as any)._requestSession).toEqual(undefined);
// @ts-expect-error accessing private property
expect(scope._propagationContext).toEqual({
traceId: expect.any(String),
spanId: expect.any(String),
sampled: false,
});
// @ts-expect-error accessing private property
expect(scope._propagationContext).not.toEqual(oldPropagationContext);
});

test('clearBreadcrumbs', () => {
Expand Down Expand Up @@ -486,6 +523,8 @@ describe('Scope', () => {
expect(updatedScope._level).toEqual('warning');
expect(updatedScope._fingerprint).toEqual(['bar']);
expect(updatedScope._requestSession.status).toEqual('ok');
// @ts-ignore accessing private property for test
expect(updatedScope._propagationContext).toEqual(localScope._propagationContext);
});

test('given an empty instance of Scope, it should preserve all the original scope data', () => {
Expand Down Expand Up @@ -518,7 +557,13 @@ describe('Scope', () => {
tags: { bar: '3', baz: '4' },
user: { id: '42' },
requestSession: { status: 'errored' as RequestSessionStatus },
propagationContext: {
traceId: '8949daf83f4a4a70bee4c1eb9ab242ed',
spanId: 'a024ad8fea82680e',
sampled: true,
},
};

const updatedScope = scope.update(localAttributes) as any;

expect(updatedScope._tags).toEqual({
Expand All @@ -540,6 +585,11 @@ describe('Scope', () => {
expect(updatedScope._level).toEqual('warning');
expect(updatedScope._fingerprint).toEqual(['bar']);
expect(updatedScope._requestSession).toEqual({ status: 'errored' });
expect(updatedScope._propagationContext).toEqual({
traceId: '8949daf83f4a4a70bee4c1eb9ab242ed',
spanId: 'a024ad8fea82680e',
sampled: true,
});
});
});

Expand Down
11 changes: 2 additions & 9 deletions packages/node/test/async/domain.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getCurrentHub, Hub, runWithAsyncContext, setAsyncContextStrategy } from '@sentry/core';
import * as domain from 'domain';
import type { Hub } from '@sentry/core';
import { getCurrentHub, runWithAsyncContext, setAsyncContextStrategy } from '@sentry/core';

import { setDomainAsyncContextStrategy } from '../../src/async/domain';

Expand All @@ -9,13 +9,6 @@ describe('domains', () => {
setAsyncContextStrategy(undefined);
});

test('without domain', () => {
// @ts-ignore property active does not exist on domain
expect(domain.active).toBeFalsy();
const hub = getCurrentHub();
expect(hub).toEqual(new Hub());
});

test('hub scope inheritance', () => {
setDomainAsyncContextStrategy();

Expand Down
Loading

0 comments on commit f1ede57

Please sign in to comment.