test(browser): Add span streaming integration tests#19581
Open
Lms24 wants to merge 15 commits intolms/feat-span-firstfrom
Open
test(browser): Add span streaming integration tests#19581Lms24 wants to merge 15 commits intolms/feat-span-firstfrom
Lms24 wants to merge 15 commits intolms/feat-span-firstfrom
Conversation
Contributor
size-limit report 📦
|
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
Lms24
force-pushed
the
lms/test-span-streaming-browser-integration-tests
branch
from
c2b23bf to
e4011e4
Compare
Contributor
Codecov Results 📊Generated by Codecov Action |
Lms24
force-pushed
the
lms/feat-span-first
branch
from
24015b1 to
e5c1208
Compare
…nsampled traces being sent
…SpanActiveInBrowser
Lms24
force-pushed
the
lms/test-span-streaming-browser-integration-tests
branch
from
4dbb4e3 to
9f490f4
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
...ser-integration-tests/suites/tracing/browserTracingIntegration/http-timings-streamed/test.ts
Outdated
Show resolved
Hide resolved
JPeer264
approved these changes
Mar 5, 2026
Member
JPeer264
left a comment
JPeer264
left a comment
There was a problem hiding this comment.
LGTM.
FWIW I didn't go over every test in detail, but picked a couple and they looked fine. I also went over with Cursor and randomly changed span names in the tests and commented out code to verify if all tests were actually expecting the correct things - also this check passed.
| */ | ||
| export function getSimpleStatusMessage(status: SpanStatus | undefined): 'ok' | 'error' { | ||
| return !status || status.code === SPAN_STATUS_OK || status.code === SPAN_STATUS_UNSET ? 'ok' : 'error'; | ||
| if ( |
Member
There was a problem hiding this comment.
l: Any reason why this moved away from a ternary? I think this adds now a few bytes
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR adds browser integration to test testing span streaming:
waitForStreamedSpan: Returns a promise of a single matching spanwaitForStreamedSpans: Returns a promise of all spans in an array whenever the callback returns truewaitForStreamedSpanEnvelope: Returns an entire streamed span (v2) envelope (including headers)observeStreamedSpan: Can be used to observe sent span envelopes without blocking the test if no envelopes are sent (good for testing that spans are not sent)getSpanOp: Small helper to easily get the op of a span which we almost always need for thewaitFor*function callbacksAdded 50+ tests, mostly converted from transaction integration tests around spans from
browserTracingIntegration:Also, this PR fixes two bugs discovered through tests:
errorinstead ofok. We want them to have statusokbut an attribute detailing the cancellation reason.Lastly, I discovered a problem with timing data on fetch and XHR spans. Will try to fix as a follow-up. Tracked in #19613
ref #17836