-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Zipkin Exporter: Adjust span transformation to comply with the spec #1688
Zipkin Exporter: Adjust span transformation to comply with the spec #1688
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1688 +/- ##
=======================================
+ Coverage 78.7% 78.8% +0.1%
=======================================
Files 134 134
Lines 7116 7157 +41
=======================================
+ Hits 5602 5643 +41
Misses 1268 1268
Partials 246 246
|
a11ea39
to
6adf24e
Compare
- remove `otel.status_description`; use `error` instead for description - do not report status code if unset - do not report description if OK or unset - omit tags if no tag has been mapped - adjust tests
6adf24e
to
d4181bf
Compare
Great work! |
Overall it looks great, my only strong concern is the usage of |
- Simplify deletion of redundant error code - Simplify endpoint rank determination
The specification for Zipkin exporter specifies a number of requirements on how the span data should be mapped between OpenTelemetry and Zipkin.
This PR intends to correct some discrepancies which have been found previously, in particular:
otel.library.*
instead ofotel.instrumentation_library.*
(relevant section)otel.status_description
and useerror
instead; Do not report Unset status and do not include status message for Ok / Unset span status (relevant section).remoteEndpoint
according to the specification (relevant section)Includes test changes as well.
Related issue
An issue for setting the service name is still pending in #1549
Resolves (partially) #1376.