Skip to content

ref: Read propagation context off of scope and isolation scope when propagating and applying trace context#10297

Merged
lforst merged 6 commits intodevelopfrom
lforst-isolation-scope-propagation
Jan 29, 2024
Merged

ref: Read propagation context off of scope and isolation scope when propagating and applying trace context#10297
lforst merged 6 commits intodevelopfrom
lforst-isolation-scope-propagation

Conversation

@lforst
Copy link
Copy Markdown
Contributor

@lforst lforst commented Jan 23, 2024 •
edited
Loading

  • Read propagation context off of current scope and isolation scope when propagating
  • Align how we propagate in http, fetch, xhr, node-fetch instrumentation
  • Don't put propagation context on processing metadata
  • Read propagation context off of current scope and isolation scope when creating trace context
  • Appropriately freeze DSC in tracingContextFromHeaders.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 23, 2024 •
edited
Loading

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 78.1 KB (-0.05% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 69.32 KB (-0.04% 🔽)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 73.2 KB (-0.06% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.94 KB (-0.05% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.34 KB (-0.13% 🔽)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.23 KB (-0.14% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.33 KB (+0.01% 🔺)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.34 KB (+0.01% 🔺)
@sentry/browser - Webpack (gzipped) 22.6 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 75.88 KB (-0.1% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.44 KB (-0.09% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.32 KB (-0.15% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.51 KB (-0.03% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 212.52 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 100.53 KB (-0.04% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 73.44 KB (+0.13% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.4 KB (-0.18% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.71 KB (-0.08% 🔽)
@sentry/react - Webpack (gzipped) 22.63 KB (-0.02% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 86.38 KB (-0.05% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 50.69 KB (-0.08% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 17.21 KB (-0.04% 🔽)

mydea
mydea reviewed Jan 23, 2024
View reviewed changes
@lforst lforst force-pushed the lforst-isolation-scope-propagation branch from 4e4f46e to 36d4cfe Compare January 25, 2024 13:40
@lforst lforst changed the title ref: Read propagation context off of scope and isolation scope when propagating ref: Read propagation context off of scope and isolation scope when propagating and applying trace context Jan 25, 2024
@lforst lforst marked this pull request as ready for review January 25, 2024 15:06
@lforst lforst requested a review from mydea January 25, 2024 15:06
@lforst lforst assigned Lms24 and AbhiPrasad and unassigned Lms24 and AbhiPrasad Jan 25, 2024
@lforst lforst requested review from AbhiPrasad and Lms24 January 25, 2024 16:08
mydea
mydea reviewed Jan 29, 2024
View reviewed changes
packages/node/src/integrations/undici/index.ts
const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
setHeadersOnRequest(request, sentryTrace, sentryBaggageHeader);
}
const { traceId, spanId, sampled, dsc } = {
Copy link
Copy Markdown
Member

@mydea mydea Jan 29, 2024

Choose a reason for hiding this comment

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

l: It feels like maybe we could extract this into a util 🤔 as this appears to be the same as for http? (and maybe others)

mydea
mydea reviewed Jan 29, 2024
View reviewed changes
packages/types/src/tracing.ts

export type TracePropagationTargets = (string | RegExp)[];

/**
Copy link
Copy Markdown
Member

@mydea mydea Jan 29, 2024

Choose a reason for hiding this comment

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

Great comments ❤️

mydea
mydea approved these changes Jan 29, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

great work on that - it is always a bit hard to follow along everywhere and ensure the behavior is "expected" but this seems to me like a massive simplification & streamlining of all of that.

@lforst lforst force-pushed the lforst-isolation-scope-propagation branch from 0f28ba7 to 3314892 Compare January 29, 2024 13:20
@lforst lforst merged commit b3af0dc into develop Jan 29, 2024
@lforst lforst deleted the lforst-isolation-scope-propagation branch January 29, 2024 14:35
@lforst lforst mentioned this pull request Jan 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mydea mydea mydea approved these changes

@Lms24 Lms24 Awaiting requested review from Lms24

+1 more reviewer

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lforst @mydea @AbhiPrasad @Lms24