-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
Dynamic Sampling Context Continuation / Baggage #1869
Conversation
5e088e8
to
2619677
Compare
Codecov ReportBase: 98.38% // Head: 98.46% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1869 +/- ##
==========================================
+ Coverage 98.38% 98.46% +0.08%
==========================================
Files 148 150 +2
Lines 8893 9240 +347
==========================================
+ Hits 8749 9098 +349
+ Misses 144 142 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
775f94c
to
f2c1089
Compare
ed734cd
to
e44980f
Compare
b8e3f9e
to
0d7350a
Compare
0fbe77a
to
c08e737
Compare
c4cfc52
to
38c1603
Compare
38c1603
to
5cea861
Compare
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.
Do we want to release this in the next minor release?
@st0012 for releasing this, we need 2 more PRs, so maybe let's merge them all together. They're independent as features, but should go out together ideally.
|
75bcad6
to
522f0f2
Compare
I'm making this a draft again so we don't merge it accidentally |
f1bd637
to
7490335
Compare
7490335
to
8b8af16
Compare
@st0012 after some discussion, we decided to remove all |
Creates new `Baggage` entries when the ruby SDK is the `head SDK`, i.e the first SDK that originates a distributed trace. The new `Baggage` entry is created when `get_baggage` is first called, either * when the first outgoing HTTP request is made * the first transaction finishes and the envelope is sent to sentry
E2E testing documentationJS as head SDK / continuationrelevant frontend request headers (baggage)
Outgoing [5] pry(#<Sentry::HTTPTransport>)> envelope.headers[:trace]
=> {"environment"=>"development", "release"=>"foobar", "public_key"=>"d16ae2d36f9249849c7964e9a3a8a608", "trace_id"=>"9821e65c92894d60ab9b05e69d84e218", "sample_rate"=>"1"} correct source annotation in [8] pry(#<Sentry::HTTPTransport>)> envelope.items.first.payload[:transaction_info]
=> {:source=>:view} Backend-backend propagation of baggageRails as head SDKmeta tags on rails template <meta name="sentry-trace" content="43906b36c4e84d97b310908dfa323ca0-534d009e7d378188-1">
<meta name="baggage" content="sentry-trace_id=43906b36c4e84d97b310908dfa323ca0,sentry-sample_rate=1.0,sentry-environment=development,sentry-release=test-sessions-2022-09-28+14%3A34%3A02+UTC,sentry-public_key=2fb45f003d054a7ea47feb45898f7649"> outgoing request from browser continues that trace
envelope from browser transaction has correct "trace":{"trace_id":"43906b36c4e84d97b310908dfa323ca0","sample_rate":"1.0","environment":"development","release":"test-sessions-2022-09-28+14:34:02+UTC","public_key":"2fb45f003d054a7ea47feb45898f7649"}} final flask request also has correct baggage header
|
ok @st0012 I've tested this E2E properly in all scenarios, everything seems to work 🎉 |
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.
Just a couple of question for me to understand. The tests I just skimmed over because my brain is already porridge, but everything looks ok (because it also looks very similar to the python implementation.)
@sl0thentr0py Should we make this PR ready for review? |
This PR adds support for the first part of our Dynamic Sampling product, i.e.
W3C Baggage
headers and adding them as aBaggage
object on the transactionbaggage
header to other outgoing HTTP requests in ournet/http
patchsentry-*
items in the baggage to a DSC (Dynamic Sampling Context) and attaching them to outgoing envelope headers in thetrace
fieldnow contains #1898 and #1902
closes #1835