Uh oh!
There was an error while loading. Please reload this page.
Add new sentry-opentelemetry gem - #1948
Conversation
be40827 to
2756a9bCompare7fedc8e to
9ed5e42Compare4034743 to
cd8fe62Comparesl0thentr0py
commented
Nov 29, 2022
@st0012 do you still want to review this separately or are you ok with us shipping this? |
st0012
commented
Nov 29, 2022
@sl0thentr0py I want to review this separately because I was focusing on other changes in my first pass. |
sl0thentr0py
commented
Nov 29, 2022
ok, no rush then! just wanted to confirm |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| attr_reader :span_map | ||
| def initialize | ||
| @span_map = {} |
There was a problem hiding this comment.
I think we may store configuration.logger and configuration.instrumenter here so we don't need to perform configuration lookup repeatedly.
There was a problem hiding this comment.
- if we do this, the precedence order of initialization comes back because you need access to the instantiated
Sentry.configurationobject - eventually we might do all the otel config automatically ourselves and then we can fix all precedence problems in our setup, but we don't want to make assumptions on user setups right now
for now, I will just kill the logger statements since they were mostly for development and keep the other stuff as is.
sl0thentr0py
commented
Dec 1, 2022
@st0012 bump, is there something more that you want to discuss here? Once again, this is a standalone gem and it does not have to be the perfect implementation, the main idea is
We have thought through some of the trade-offs and are aware of some possible problems, your feedback is of course very welcome. |
st0012
left a comment
There was a problem hiding this comment.
Yeah I think we can test it first. Just one more thing: do you want to add it to the top-level` readme too?
sl0thentr0py
commented
Dec 1, 2022
docs PR merged, will be live soon |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## master #1948 +/- ##
==========================================
+ Coverage 98.35% 98.41% +0.05%
==========================================
Files 151 156 +5 Lines 9450 9793 +343 ==========================================
+ Hits 9295 9638 +343
Misses 155 155 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
sentry-opentelemetryto be used as described hereconfig.instrumeneteroption so that sentry and otel do not step on each other's toesSpanProcessorthat hooks intoon_start/on_finishwhenever OpenTelemetry starts/finishes a span. This in turn creates sentry transactions/spans as necessary and keeps track of them in an internalspan_maphash.Propagatorto make sure distributed tracing and dynamic sampling work by hooking intoinject/extractfor incoming/outgoing sentry-trace/baggage header management.Spec
https://develop.sentry.dev/sdk/performance/opentelemetry/
#skip-changelog