search

getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
926 stars 493 forks source link

Allow SENTRY_TRACE and SENTRY_BAGGAGE env vars to continue traces #2179

Closed natikgadzhi closed 8 months ago

natikgadzhi commented 10 months ago

Summary

This pull request is part 3 of https://github.com/getsentry/sentry-ruby/issues/2056. It adds support for continuing traces by passing SENTRY_TRACE and SENTRY_BAGGAGE environment variables.

Closes #2099.

Changes

Reviewing

I'm new to distributed tracing, and Sentry implementation in particular, so I might need more handholding with the next few PRs.

In #2056:

You may update the traceId and dynamicSamplingContext from the request headers during an incoming request or if the process was exposed to a SENTRY_TRACE and/or SENTRY_BAGGAGE environment variable if performance is disabled.

In my current implementation, if the trace string was not passed in headers otherwise, PropagationContext will try and read the environment variable. It does NOT check whether Sentry Performance is enabled. Is that correct?

/cc @sl0thentr0py

codecov[bot] commented 10 months ago

Codecov Report

Merging #2179 (2a9fa67) into master (c4e4797) will decrease coverage by 0.03%. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2179 +/- ## ========================================== - Coverage 97.34% 97.31% -0.03% ========================================== Files 99 99 Lines 3687 3687 ========================================== - Hits 3589 3588 -1 - Misses 98 99 +1 ``` | [Components](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [sentry-ruby](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `98.03% <100.00%> (ø)` | | | [sentry-rails](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.98% <ø> (ø)` | | | [sentry-sidekiq](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.50% <ø> (ø)` | | | [sentry-resque](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `92.06% <ø> (-1.59%)` | :arrow_down: | | [sentry-delayed_job](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `94.44% <ø> (ø)` | | | [sentry-opentelemetry](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | `100.00% <100.00%> (ø)` | | | [Files](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [...entelemetry/lib/sentry/opentelemetry/propagator.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LW9wZW50ZWxlbWV0cnkvbGliL3NlbnRyeS9vcGVudGVsZW1ldHJ5L3Byb3BhZ2F0b3IucmI=) | `100.00% <100.00%> (ø)` | | | [sentry-ruby/lib/sentry/baggage.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LXJ1YnkvbGliL3NlbnRyeS9iYWdnYWdlLnJi) | `100.00% <100.00%> (ø)` | | | [sentry-ruby/lib/sentry/propagation\_context.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LXJ1YnkvbGliL3NlbnRyeS9wcm9wYWdhdGlvbl9jb250ZXh0LnJi) | `100.00% <100.00%> (ø)` | | | [sentry-ruby/lib/sentry/transaction.rb](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c2VudHJ5LXJ1YnkvbGliL3NlbnRyeS90cmFuc2FjdGlvbi5yYg==) | `100.00% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/getsentry/sentry-ruby/pull/2179/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry)
natikgadzhi commented 9 months ago

@st0012 if you feel like reviewing another one, this should be a quick and clean one.

natikgadzhi commented 9 months ago

Yep, I might be wrong here. Because the request env vars were already respected, and the OS environment vars for this seem a little bit off to me.