-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
test(browser): Add test for current DSC transaction name updating behavior #13953
Merged
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
18c058a
test(browser): Add test for current DSC transaction name updating beh…
Lms24 efcaaf8
request and error after span end
Lms24 2be6891
add tests for requests and spans after initial pageload span ended
Lms24 eab1cd3
biome
Lms24 75af20f
test(node): Add tests for current DSC transaction name updating behavior
Lms24 079417d
fix failing tests
Lms24 3ba108b
cleanup
Lms24 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
11 changes: 11 additions & 0 deletions
11
dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/init.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
integrations: [Sentry.browserTracingIntegration({ instrumentNavigation: false, instrumentPageLoad: false })], | ||
tracesSampleRate: 1, | ||
tracePropagationTargets: ['example.com'], | ||
release: '1.1.1', | ||
}); |
36 changes: 36 additions & 0 deletions
36
dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/subject.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
const btnStartSpan = document.getElementById('btnStartSpan'); | ||
const btnUpdateName = document.getElementById('btnUpdateName'); | ||
const btnMakeRequest = document.getElementById('btnMakeRequest'); | ||
const btnCaptureError = document.getElementById('btnCaptureError'); | ||
const btnEndSpan = document.getElementById('btnEndSpan'); | ||
|
||
btnStartSpan.addEventListener('click', () => { | ||
Sentry.startSpanManual( | ||
{ name: 'test-root-span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } }, | ||
async span => { | ||
console.log('Root span started'); | ||
window.__traceId = span.spanContext().traceId; | ||
await new Promise(resolve => { | ||
btnEndSpan.addEventListener('click', resolve); | ||
}); | ||
span.end(); | ||
console.log('Root span ended'); | ||
}, | ||
); | ||
}); | ||
|
||
let updateCnt = 0; | ||
btnUpdateName.addEventListener('click', () => { | ||
const span = Sentry.getActiveSpan(); | ||
const rootSpan = Sentry.getRootSpan(span); | ||
rootSpan.updateName(`updated-root-span-${++updateCnt}`); | ||
rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); | ||
}); | ||
|
||
btnMakeRequest.addEventListener('click', () => { | ||
fetch('https://example.com/api'); | ||
}); | ||
|
||
btnCaptureError.addEventListener('click', () => { | ||
Sentry.captureException(new Error('test-error')); | ||
}); |
5 changes: 5 additions & 0 deletions
5
dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/template.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<button id="btnStartSpan">Start Span</button> | ||
<button id="btnUpdateName">Update Name</button> | ||
<button id="btnMakeRequest">Make Request</button> | ||
<button id="btnCaptureError">Capture Error</button> | ||
<button id="btnEndSpan">End Span</button> |
164 changes: 164 additions & 0 deletions
164
dev-packages/browser-integration-tests/suites/tracing/dsc-txn-name-update/test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
import { expect } from '@playwright/test'; | ||
import type { Page } from '@playwright/test'; | ||
import type { DynamicSamplingContext } from '@sentry/types'; | ||
import { sentryTest } from '../../../utils/fixtures'; | ||
import type { EventAndTraceHeader } from '../../../utils/helpers'; | ||
import { | ||
eventAndTraceHeaderRequestParser, | ||
getMultipleSentryEnvelopeRequests, | ||
shouldSkipTracingTest, | ||
} from '../../../utils/helpers'; | ||
|
||
sentryTest('updates the DSC when the txn name is updated and high-quality', async ({ getLocalTestUrl, page }) => { | ||
if (shouldSkipTracingTest()) { | ||
sentryTest.skip(); | ||
} | ||
|
||
const url = await getLocalTestUrl({ testDir: __dirname }); | ||
|
||
await page.goto(url); | ||
|
||
/* | ||
Test Steps: | ||
1. Start new span with LQ txn name (source: url) | ||
2. Make request and check that baggage has no transaction name | ||
3. Capture error and check that envelope trace header has no transaction name | ||
4. Update span name and source to HQ (source: route) | ||
5. Make request and check that baggage has HQ txn name | ||
6. Capture error and check that envelope trace header has HQ txn name | ||
7. Update span name again with HQ name (source: route) | ||
8. Make request and check that baggage has updated HQ txn name | ||
9. Capture error and check that envelope trace header has updated HQ txn name | ||
10. End span and check that envelope trace header has updated HQ txn name | ||
*/ | ||
|
||
// 1 | ||
await page.locator('#btnStartSpan').click(); | ||
const traceId = await page.evaluate(() => { | ||
return (window as any).__traceId; | ||
}); | ||
|
||
expect(traceId).toMatch(/^[0-9a-f]{32}$/); | ||
|
||
// 2 | ||
const baggageItems = await makeRequestAndGetBaggageItems(page); | ||
expect(baggageItems).toEqual([ | ||
'sentry-environment=production', | ||
'sentry-public_key=public', | ||
'sentry-release=1.1.1', | ||
'sentry-sample_rate=1', | ||
'sentry-sampled=true', | ||
`sentry-trace_id=${traceId}`, | ||
]); | ||
|
||
// 3 | ||
const errorEnvelopeTraceHeader = await captureErrorAndGetEnvelopeTraceHeader(page); | ||
expect(errorEnvelopeTraceHeader).toEqual({ | ||
environment: 'production', | ||
public_key: 'public', | ||
release: '1.1.1', | ||
sample_rate: '1', | ||
sampled: 'true', | ||
trace_id: traceId, | ||
}); | ||
|
||
// 4 | ||
await page.locator('#btnUpdateName').click(); | ||
|
||
// 5 | ||
const baggageItemsAfterUpdate = await makeRequestAndGetBaggageItems(page); | ||
expect(baggageItemsAfterUpdate).toEqual([ | ||
'sentry-environment=production', | ||
'sentry-public_key=public', | ||
'sentry-release=1.1.1', | ||
'sentry-sample_rate=1', | ||
'sentry-sampled=true', | ||
`sentry-trace_id=${traceId}`, | ||
'sentry-transaction=updated-root-span-1', | ||
]); | ||
|
||
// 6 | ||
const errorEnvelopeTraceHeaderAfterUpdate = await captureErrorAndGetEnvelopeTraceHeader(page); | ||
expect(errorEnvelopeTraceHeaderAfterUpdate).toEqual({ | ||
environment: 'production', | ||
public_key: 'public', | ||
release: '1.1.1', | ||
sample_rate: '1', | ||
sampled: 'true', | ||
trace_id: traceId, | ||
transaction: 'updated-root-span-1', | ||
}); | ||
|
||
// 7 | ||
await page.locator('#btnUpdateName').click(); | ||
|
||
// 8 | ||
const baggageItemsAfterSecondUpdate = await makeRequestAndGetBaggageItems(page); | ||
expect(baggageItemsAfterSecondUpdate).toEqual([ | ||
'sentry-environment=production', | ||
'sentry-public_key=public', | ||
'sentry-release=1.1.1', | ||
'sentry-sample_rate=1', | ||
'sentry-sampled=true', | ||
`sentry-trace_id=${traceId}`, | ||
'sentry-transaction=updated-root-span-2', | ||
]); | ||
|
||
// 9 | ||
const errorEnvelopeTraceHeaderAfterSecondUpdate = await captureErrorAndGetEnvelopeTraceHeader(page); | ||
expect(errorEnvelopeTraceHeaderAfterSecondUpdate).toEqual({ | ||
environment: 'production', | ||
public_key: 'public', | ||
release: '1.1.1', | ||
sample_rate: '1', | ||
sampled: 'true', | ||
trace_id: traceId, | ||
transaction: 'updated-root-span-2', | ||
}); | ||
|
||
// 10 | ||
const txnEventPromise = getMultipleSentryEnvelopeRequests<EventAndTraceHeader>( | ||
page, | ||
1, | ||
{ envelopeType: 'transaction' }, | ||
eventAndTraceHeaderRequestParser, | ||
); | ||
|
||
await page.locator('#btnEndSpan').click(); | ||
|
||
const [txnEvent, txnEnvelopeTraceHeader] = (await txnEventPromise)[0]; | ||
expect(txnEnvelopeTraceHeader).toEqual({ | ||
environment: 'production', | ||
public_key: 'public', | ||
release: '1.1.1', | ||
sample_rate: '1', | ||
sampled: 'true', | ||
trace_id: traceId, | ||
transaction: 'updated-root-span-2', | ||
}); | ||
|
||
expect(txnEvent.transaction).toEqual('updated-root-span-2'); | ||
}); | ||
|
||
async function makeRequestAndGetBaggageItems(page: Page): Promise<string[]> { | ||
const requestPromise = page.waitForRequest('https://example.com/*'); | ||
await page.locator('#btnMakeRequest').click(); | ||
const request = await requestPromise; | ||
|
||
const baggage = await request.headerValue('baggage'); | ||
return baggage?.split(',').sort() ?? []; | ||
} | ||
|
||
async function captureErrorAndGetEnvelopeTraceHeader(page: Page): Promise<DynamicSamplingContext | undefined> { | ||
const errorEventPromise = getMultipleSentryEnvelopeRequests<EventAndTraceHeader>( | ||
page, | ||
1, | ||
{ envelopeType: 'event' }, | ||
eventAndTraceHeaderRequestParser, | ||
); | ||
|
||
await page.locator('#btnCaptureError').click(); | ||
|
||
const [, errorEnvelopeTraceHeader] = (await errorEventPromise)[0]; | ||
return errorEnvelopeTraceHeader; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
can we add 11. to send a request after the span has ended, and make sure what is sent then? as we keep the trace alive after that (but not on the active span anymore, but the propagation context) this is also a scenario, right?
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, good point. Let me add that
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.
Hmm so it looks like at the moment, the DSC is also re-generated and no span information is added at all, meaning we not only loose
transaction
name but alsosample_rate
andsampled
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.
argh this is also problematic for the
sentry-trace
header. In the same scenario, we omit the sampling decision:(first 3 requests are happening while the span is active, the last one afterwards. Same traceId but different sampled flags)
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.
let's reflect this in this PR I'd say and then see if we can fix/improve this in a follow up...?
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.
@mydea I think the behavior in this test is caused by the constructed scenario of us starting the trace by creating a custom span as opposed to a pageload or navigation span. I will look into what exactly is causing the transaction info to be missing while the traceId remains the same in this scenario. However, I added tests for the usual pageload transaction scenario where after the transaction ends we make requests/send additional spans. They have consistent DSC data from the initial pageload transaction.
==> we should have consistent behaviour today as long as browserTracingIntegration is used with default options for pageload/navigation.