-
Notifications
You must be signed in to change notification settings - Fork 192
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
Fetch InstrumentationScope from ReadableSpan #705
Comments
@bobstrecansky it looks like @brettmc updated this for the Jaeger and Zipkin cases at some point per this search. Is the concern that this needs to be added to other non-OTLP exporter cases in the repo? Or maybe that the instrumentation library piece needs to be added back for backwards compatibility? (for what exactly I'm not really clear. Maybe like if Jaeger and Zipkin have been configured to look for the instrumentation library piece perhaps already, it's not gonna work with just sending the instrumentation scope piece? Also I find it somewhat hard to follow the conversation on the original PR that updated the spec) Or maybe other things? |
This issue is mostly a placeholder to ensure that all of that spec compliant matrix is completed. Looks like you've done a bunch of sleuthing; I believe this is done for us. That deprecated bit shouldn't matter for us, maybe? We should probably instrument those as well for consistency's sake. |
It looks like we can fetch the instrumentation scope from a span, so as Bob said we just needed to verify that we have implemented per spec, and you could submit a PR against the compliance matrix to complete this issue. |
Spun up open-telemetry/opentelemetry-specification#2622 on the assumption that the breaking changes the other libraries have to worry about are about direct consumers relying on the old instrumentation library piece, not backends (from looking through a few more PRs of other language libraries linked to that main spec PR, erlang being the only one I've seen so far that did the straight swap without deprecating). |
OTEL-PHP at some point switched to using InstrumentationScope instead of InstrumentationLibrary everywhere, which incidentally satisfied the ["Fetch InstrumentationScope from ReadableSpan"](/~https://github.com/open-telemetry/opentelemetry-specification/blob/9abbdd39d0b35f635f833f072013431da419894e/specification/common/mapping-to-non-otlp.md#instrumentationscope) part of the spec (otel-php is still alpha so it seemed ok to not deprecate the instrumentation library and just remove it's use entirely) Related issues # open-telemetry/opentelemetry-php#705 Related [OTEP(s)](/~https://github.com/open-telemetry/oteps) # Don't think it was an OTEP but #2276 is where the push for the change originally came from I believe
The comparison matrix has been updated so probably good to close this now |
…metry#2622) OTEL-PHP at some point switched to using InstrumentationScope instead of InstrumentationLibrary everywhere, which incidentally satisfied the ["Fetch InstrumentationScope from ReadableSpan"](/~https://github.com/open-telemetry/opentelemetry-specification/blob/9abbdd39d0b35f635f833f072013431da419894e/specification/common/mapping-to-non-otlp.md#instrumentationscope) part of the spec (otel-php is still alpha so it seemed ok to not deprecate the instrumentation library and just remove it's use entirely) Related issues # open-telemetry/opentelemetry-php#705 Related [OTEP(s)](/~https://github.com/open-telemetry/oteps) # Don't think it was an OTEP but open-telemetry#2276 is where the push for the change originally came from I believe
OTEL-PHP at some point switched to using InstrumentationScope instead of InstrumentationLibrary everywhere, which incidentally satisfied the ["Fetch InstrumentationScope from ReadableSpan"](/~https://github.com/open-telemetry/opentelemetry-specification/blob/65624332cadac9990923f442e96a424c308e502c/specification/common/mapping-to-non-otlp.md#instrumentationscope) part of the spec (otel-php is still alpha so it seemed ok to not deprecate the instrumentation library and just remove it's use entirely) Related issues # open-telemetry/opentelemetry-php#705 Related [OTEP(s)](/~https://github.com/open-telemetry/oteps) # Don't think it was an OTEP but open-telemetry/opentelemetry-specification#2276 is where the push for the change originally came from I believe
OTEL-PHP at some point switched to using InstrumentationScope instead of InstrumentationLibrary everywhere, which incidentally satisfied the ["Fetch InstrumentationScope from ReadableSpan"](/~https://github.com/open-telemetry/opentelemetry-specification/blob/9abbdd39d0b35f635f833f072013431da419894e/specification/common/mapping-to-non-otlp.md#instrumentationscope) part of the spec (otel-php is still alpha so it seemed ok to not deprecate the instrumentation library and just remove it's use entirely) Related issues # open-telemetry/opentelemetry-php#705 Related [OTEP(s)](/~https://github.com/open-telemetry/oteps) # Don't think it was an OTEP but open-telemetry/opentelemetry-specification#2276 is where the push for the change originally came from I believe
The spec compliance matrix defines Fetch InstrumentationScope from ReadableSpan
We need to implement this.
The text was updated successfully, but these errors were encountered: