Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Re-add TraceContext for all events #2656

Merged
merged 3 commits into from Jun 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
- [react] feat: Add @sentry/react package (#2631)
- [browser] Change XHR instrumentation order to handle `onreadystatechange` breadcrumbs correctly (#2643)
- [apm] fix: Re-add TraceContext for all events (#2656)

## 5.16.1

Expand Down
3 changes: 3 additions & 0 deletions packages/apm/src/integrations/tracing.ts
Expand Up @@ -436,6 +436,9 @@ export class Tracing implements Integration {
...transactionContext,
}) as Transaction;

// We set the transaction here on the scope so error events pick up the trace context and attach it to the error
hub.configureScope(scope => scope.setSpan(Tracing._activeTransaction));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does it for the "idle transaction" -- what about manual instrumentation / Sentry.startTransaction?

cc @mitsuhiko

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably something to document.


// The reason we do this here is because of cached responses
// If we start and transaction without an activity it would never finish since there is no activity
const id = Tracing.pushActivity('idleTransactionStarted');
Expand Down
6 changes: 6 additions & 0 deletions packages/hub/src/scope.ts
Expand Up @@ -377,6 +377,12 @@ export class Scope implements ScopeInterface {
if (this._transaction) {
event.transaction = this._transaction;
}
// We want to set the trace context for normal events only if there isn't already
// a trace context on the event. There is a product feature in place where we link
// errors with transaction and it relys on that.
if (this._span) {
event.contexts = { trace: this._span.getTraceContext(), ...event.contexts };
}

this._applyFingerprint(event);

Expand Down
32 changes: 32 additions & 0 deletions packages/hub/test/scope.test.ts
Expand Up @@ -265,6 +265,38 @@ describe('Scope', () => {
});
});

test('applyToEvent trace context', async () => {
expect.assertions(1);
const scope = new Scope();
const span = {
fake: 'span',
getTraceContext: () => ({ a: 'b' }),
} as any;
scope.setSpan(span);
const event: Event = {};
return scope.applyToEvent(event).then(processedEvent => {
expect((processedEvent!.contexts!.trace as any).a).toEqual('b');
});
});

test('applyToEvent existing trace context in event should be stronger', async () => {
expect.assertions(1);
const scope = new Scope();
const span = {
fake: 'span',
getTraceContext: () => ({ a: 'b' }),
} as any;
scope.setSpan(span);
const event: Event = {
contexts: {
trace: { a: 'c' },
},
};
return scope.applyToEvent(event).then(processedEvent => {
expect((processedEvent!.contexts!.trace as any).a).toEqual('c');
});
});

test('clear', () => {
const scope = new Scope();
scope.setExtra('a', 2);
Expand Down