Skip to content

feat(tracing): Add user data to tracestate header #3343

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 2, 2021
Merged

feat(tracing): Add user data to tracestate header #3343

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ba466c9
add metadata to Transaction type and segment to User type
lobsterkatie Mar 24, 2021
86ac6a3
move to lazy creation of tracestate value
lobsterkatie Mar 24, 2021
a2ee549
include user data in tracestate
lobsterkatie Mar 24, 2021
2572ff4
when creating tracestate, get hub from transaction if possible
lobsterkatie Mar 24, 2021
ba14834
add tests
lobsterkatie Mar 24, 2021
2a7b830
change schema to match spec
lobsterkatie Mar 24, 2021
7cc1d39
fix typo
lobsterkatie Mar 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
include user data in tracestate
  • Loading branch information
@lobsterkatie
lobsterkatie committed Mar 24, 2021
commit a2ee549b95d9665bf9bb1296b13f8702b012646b
7 changes: 5 additions & 2 deletions packages/tracing/src/span.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable max-lines */
import { getCurrentHub } from '@sentry/hub';
import { Hub, Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types';
import { getCurrentHub, Hub } from '@sentry/hub';
import { Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types';
Comment on lines +2 to +3
Copy link

@RaduW RaduW Mar 24, 2021

Choose a reason for hiding this comment

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

What's going on here ? Doesn't seem to be related to the PR.

Copy link
Member Author

@lobsterkatie lobsterkatie Mar 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It actually is, though. The change here is from importing the type to importing the class. Later on in the file I'm calling Hub.getScope(), a method which exists on the class but isn't in the type, so if I don't make this change, it won't compile. The alternative to doing this is to add the method to the type, which I'm happy to do instead if you think that's better.

Copy link

@RaduW RaduW Mar 26, 2021

Choose a reason for hiding this comment

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

Understood, it's probably me (I'm not so familiar with this code) but it further raises the question why we don't have getScope in the interface ( since we do have pushScope, popScope, withScope).

Anyway, not the right place to discuss general architecture of the API.

import { dropUndefinedKeys, logger, timestampWithMs, uuid4 } from '@sentry/utils';

import { SpanStatus } from './spanstatus';
Expand Down Expand Up @@ -367,6 +367,7 @@ export class Span implements SpanInterface {
*/
protected _getNewTracestate(hub: Hub = getCurrentHub()): string | undefined {
const client = hub.getClient();
const { id: userId, segment: userSegment } = hub.getScope()?.getUser() || {};
const dsn = client?.getDsn();

if (!client || !dsn) {
Expand All @@ -385,6 +386,8 @@ export class Span implements SpanInterface {
release,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
publicKey: dsn.publicKey!,
userId,
userSegment,
})}`;
}

Expand Down
6 changes: 5 additions & 1 deletion packages/tracing/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ type SentryTracestateData = {
environment: string | undefined | null;
release: string | undefined | null;
publicKey: string;
userId: string | undefined | null;
userSegment: string | undefined | null;
};

/**
Expand All @@ -159,9 +161,11 @@ type SentryTracestateData = {
*/
export function computeTracestateValue(data: SentryTracestateData): string {
// `JSON.stringify` will drop keys with undefined values, but not ones with null values, so this prevents
// `environment` and `release` from being dropped if they haven't been set by `Sentry.init`
// these values from being dropped if they haven't been set by `Sentry.init`
data.environment = data.environment || null;
data.release = data.release || null;
data.userId = data.userId || null;
data.userSegment = data.userSegment || null;

// See https://www.w3.org/TR/trace-context/#tracestate-header-field-values
// The spec for tracestate header values calls for a string of the form
Expand Down
5 changes: 5 additions & 0 deletions packages/tracing/test/span.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ describe('Span', () => {
const release = 'off.leash.trail';
const environment = 'dogpark';
const traceId = '12312012123120121231201212312012';
const user = { id: '1121', segment: 'bigs' };

const computedTracestate = `sentry=${computeTracestateValue({ traceId, environment, release, publicKey })}`;
const thirdpartyData = 'maisey=silly,charlie=goofy';
Expand All @@ -121,6 +122,10 @@ describe('Span', () => {
}),
);

hub.configureScope(scope => {
scope.setUser(user);
});

test('no third-party data', () => {
const transaction = new Transaction({ name: 'FETCH /ball', traceId }, hub);
const span = transaction.startChild({ op: 'dig.hole' });
Expand Down