ext/Zipkin - Transform resource to tags when exporting #707
Conversation
| @@ -212,6 +212,17 @@ def _extract_tags_from_span(attr): | |||
| return tags | |||
|
|
|||
|
|
|||
| def _extract_tags_from_span(span: Span): | |||
| tags = _extract_tags_from_dict(span.attributes) | |||
There was a problem hiding this comment.
there are cases where a DefaultSpan is used instead of an sdk.trace.Span. In those cases this will return an attributeerror as "attributes" doesn't exist.
There may need to be some sort of getter for attributes in the API itself to ensure this works as expected.
There was a problem hiding this comment.
Can you elaborate on how I should handle this?
Currently I kept the existing behavior.
There was a problem hiding this comment.
Right, this was a bug that existed in the current implementation of the exporter. You'd want to use something like getattr(span, "attributes", None) to prevent an exception from being raised.
There was a problem hiding this comment.
Thanks for clarifying! Changed to getattr.
ext/opentelemetry-ext-zipkin/src/opentelemetry/ext/zipkin/__init__.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-ext-zipkin/src/opentelemetry/ext/zipkin/__init__.py
Outdated
Show resolved
Hide resolved
| @@ -212,6 +212,17 @@ def _extract_tags_from_span(attr): | |||
| return tags | |||
|
|
|||
|
|
|||
| def _extract_tags_from_span(span: Span): | |||
| tags = _extract_tags_from_dict(span.attributes) | |||
There was a problem hiding this comment.
Right, this was a bug that existed in the current implementation of the exporter. You'd want to use something like getattr(span, "attributes", None) to prevent an exception from being raised.
|
Thanks! |
This PR implements the missing part of exporting the TraceProvider resource into Zipkin.
Same as in js.
This PR also aligns Zipkin behavior with my previous PR done to jaeger - #645.
I wanted to preserve the existing behavior of the
tagsbeingNoneif attributes do not exist, so I ended with a nestedif, although I think it's worth discussing maybe the default value should always be{}.As for the test, I added another span to cover all tags permutations: