-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tracing): Add additional Dynamic Sampling Context items to baggage and envelope headers #5292
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
Changes from all commits
a550b9d
cd78621
a47cb46
8054398
3f40e64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { expect } from '@playwright/test'; | ||
import { EventEnvelopeHeaders } from '@sentry/types'; | ||
|
||
import { sentryTest } from '../../../utils/fixtures'; | ||
import { envelopeHeaderRequestParser, getFirstSentryEnvelopeRequest } from '../../../utils/helpers'; | ||
|
||
sentryTest( | ||
'should send dynamic sampling context data in transaction envelope header', | ||
async ({ getLocalTestPath, page }) => { | ||
const url = await getLocalTestPath({ testDir: __dirname }); | ||
|
||
const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser); | ||
|
||
expect(envHeader.trace).toBeDefined(); | ||
expect(envHeader.trace).toEqual({ | ||
environment: 'production', | ||
transaction: expect.stringContaining('index.html'), | ||
user: { | ||
id: 'user123', | ||
segment: 'segmentB', | ||
}, | ||
sample_rate: '1', | ||
trace_id: expect.any(String), | ||
public_key: 'public', | ||
}); | ||
}, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,7 +77,10 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n | |
expect(response).toMatchObject({ | ||
test_data: { | ||
host: 'somewhere.not.sentry', | ||
baggage: 'sentry-environment=prod,sentry-release=1.0', | ||
baggage: expect.stringContaining( | ||
'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-samplerate=1,' + | ||
'sentry-publickey=public,sentry-traceid=', | ||
Comment on lines
+81
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't really matter here and it's very subjective but I personally avoid breaking up strings this way, simply because it's less "searchable". No action required though - just wanted to get this thought out ^^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup but I also don't like super long strings that heavily exceed the characters-per-line limit. Very annoying when working with split editor windows 😅 |
||
), | ||
}, | ||
}); | ||
}); | ||
|
@@ -93,7 +96,11 @@ test('Should populate Sentry and propagate 3rd party content if sentry-trace hea | |
expect(response).toMatchObject({ | ||
test_data: { | ||
host: 'somewhere.not.sentry', | ||
baggage: 'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0', | ||
// TraceId changes, hence we only expect that the string contains the traceid key | ||
baggage: expect.stringContaining( | ||
'foo=bar,bar=baz,sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,' + | ||
'sentry-samplerate=1,sentry-publickey=public,sentry-traceid=', | ||
), | ||
}, | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, having this in the
sdkProcessingMetadata
makes so much more sense.