Add translation of InstrumentationLibraryInfo to Jaeger #801
Conversation
|
See my comment #800 (comment) I think it is best to define conventions for all OTLP fields that will be applicable to all other protocols, instead of defining it individually for every protocol (which results in a duplicate like we can see here for Jaeger and in the other PR for Zipkin). |
|
@tigrannajaryan some fields are duplicate indeed, but majority are not. And some protocols needs a specific translation for a field "foo" as attribute in a Span but others may have "Process" like Jaeger. |
How about we have a "common" section that is applicable to all protocols, and then individual sections for each protocol that has some specific handling (like Jaeger's Process)? |
|
@tigrannajaryan @bogdandrutu Is it Ok if I create a follow up issue about extracting common translation rules (I may assign it myself) and in the meantime we can merge this and #800 ? I think these two PRs are already useful and make a small step forward. Preparing common document will take more time from me to familiarise myself with protocols. |
|
I will close and re-open PR for auto-assignment |
Sounds good. I can also help with that issue. |
|
Created #806 as a follow-up |
|
@SergeyKanzhelev do you still want to block this PR? |
|
swapped issue with @yurishkuro as Yuri might have more context here to merge |
|
@yurishkuro can you confirm is ok? Not too much text but we can have this as a starting point :) |
| | OpenTelemetry | Jaeger | | ||
| | ------------- | ------ | | ||
| | `InstrumentationLibrary.name`|`otel.instrumentation_library.name`| | ||
| | `InstrumentationLibrary.version`|`otel.instrumentation_library.version`| |
There was a problem hiding this comment.
I do not follow why we need to change the naming convention when translating to Jaeger. Jaeger backend has very few semantic conventions that it recognizes, this one is certainly not one of them, so it would make no difference if the attributes were to be translated as is. Not to mention that we plan to align Jaeger with OTel data model and conventions directly in the future.
Separately, where did the strings InstrumentationLibrary.name come from? I spot-checked https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/trace/semantic_conventions and they all seem to use snake_case.snake_case naming, not PascalCase.
There was a problem hiding this comment.
InstrumentationLibrary.name is not an attributes. InstrumentationLibrary is a class that Tracer hold on to.
There was a problem hiding this comment.
I don't understand what do you mean by "change the naming convention when translating to Jaeger"?
First, as Sergey answered above, InstrumentationLibrary is not an attribute, this is a separate field in InstrumentationLibrarySpans protocol message.
Second, the same translation, to the same attribute names, was proposed, accepted and merged for OTLP->Zipkin transaltion.
There was a problem hiding this comment.
@yurishkuro is this clear now?
This addresses spec issues open-telemetry/opentelemetry-specification#800 open-telemetry/opentelemetry-specification#801 and opentelemetry-go issues open-telemetry#1086 open-telemetry#1087
This addresses spec issues open-telemetry/opentelemetry-specification#800 open-telemetry/opentelemetry-specification#801 and opentelemetry-go issues open-telemetry#1086 open-telemetry#1087
* Add InstrumentationLibrary info to Zipkin/Jaeger exporters This addresses spec issues open-telemetry/opentelemetry-specification#800 open-telemetry/opentelemetry-specification#801 and opentelemetry-go issues #1086 #1087 * Reflect change in CHANGELOG
…metry#1119) * Add InstrumentationLibrary info to Zipkin/Jaeger exporters This addresses spec issues open-telemetry/opentelemetry-specification#800 open-telemetry/opentelemetry-specification#801 and opentelemetry-go issues open-telemetry#1086 open-telemetry#1087 * Reflect change in CHANGELOG
According to spec open-telemetry/opentelemetry-specification#800 and open-telemetry/opentelemetry-specification#801, We MUST export instrumentation library information for Jaeger and Zipkin exporter
…ent version for Jaeger. According to spec open-telemetry/opentelemetry-specification#800 and open-telemetry/opentelemetry-specification#801, We MUST export instrumentation library information for Jaeger and Zipkin exporter. Use crate version as default Jaeger instrumentation library version in consistent with Zipkin.
…ent version for Jaeger. (#243) According to spec open-telemetry/opentelemetry-specification#800 and open-telemetry/opentelemetry-specification#801, We MUST export instrumentation library information for Jaeger and Zipkin exporter.
* Add InstrumentationLibrary info to Zipkin/Jaeger exporters This addresses spec issues open-telemetry/opentelemetry-specification#800 open-telemetry/opentelemetry-specification#801 and opentelemetry-go issues open-telemetry/opentelemetry-go#1086 open-telemetry/opentelemetry-go#1087 * Reflect change in CHANGELOG
Changes
Add rules for translating InstrumentationLibraryInfo from OTLP to Jaeger.