Skip to content
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.

Sign up for GitHub

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

ext/Zipkin - Transform resource to tags when exporting #707

Merged
merged 10 commits into from
May 22, 2020
Merged

ext/Zipkin - Transform resource to tags when exporting #707

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cb44c0c
ext/zipkin - Transform resource to tags when exporting
nirsky May 19, 2020
285121e
lint fix
nirsky May 19, 2020
b505e0d
CR
nirsky May 20, 2020
e3067ff
Merge branch 'master' into feat/zipkin/resource
May 20, 2020
6c861cf
Access span.attributes using getattr
nirsky May 21, 2020
78c5003
Merge branch 'feat/zipkin/resource' of https://github.com/nirsky/open…
nirsky May 21, 2020
5099cac
Merge branch 'master' into feat/zipkin/resource
nirsky May 21, 2020
2c72067
Merge branch 'master' into feat/zipkin/resource
toumorokoshi May 21, 2020
16e5805
Merge branch 'master' into feat/zipkin/resource
toumorokoshi May 21, 2020
0a2e617
Merge branch 'master' into feat/zipkin/resource
toumorokoshi May 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions ext/opentelemetry-ext-zipkin/src/opentelemetry/ext/zipkin/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def _translate_to_zipkin(self, spans: Sequence[Span]):
"duration": duration_mus,
"localEndpoint": local_endpoint,
"kind": SPAN_KIND_MAP[span.kind],
"tags": _extract_tags_from_span(span.attributes),
"tags": _extract_tags_from_span(span),
"annotations": _extract_annotations_from_events(span.events),
}

Expand All @@ -196,11 +196,11 @@ def shutdown(self) -> None:
pass


def _extract_tags_from_span(attr):
if not attr:
return None
def _extract_tags_from_dict(tags_dict):
tags = {}
for attribute_key, attribute_value in attr.items():
if not tags_dict:
return tags
for attribute_key, attribute_value in tags_dict.items():
if isinstance(attribute_value, (int, bool, float)):
value = str(attribute_value)
elif isinstance(attribute_value, str):
Expand All @@ -212,6 +212,13 @@ def _extract_tags_from_span(attr):
return tags


def _extract_tags_from_span(span: Span):
tags = _extract_tags_from_dict(span.attributes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on how I should handle this?
Currently I kept the existing behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying! Changed to getattr.

if span.resource:
tags.update(_extract_tags_from_dict(span.resource.labels))
return tags


def _extract_annotations_from_events(events):
return (
[
Expand Down
36 changes: 32 additions & 4 deletions ext/opentelemetry-ext-zipkin/tests/test_zipkin_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from opentelemetry import trace as trace_api
from opentelemetry.ext.zipkin import ZipkinSpanExporter
from opentelemetry.sdk import trace
from opentelemetry.sdk.trace import Resource
from opentelemetry.sdk.trace.export import SpanExportResult
from opentelemetry.trace import TraceFlags

Expand Down Expand Up @@ -95,7 +96,7 @@ def test_constructor_explicit(self):
# pylint: disable=too-many-locals
def test_export(self):

span_names = ("test1", "test2", "test3")
span_names = ("test1", "test2", "test3", "test4")
trace_id = 0x6E0C63257DE34C926F9EFCD03927272E
span_id = 0x34BF92DEEFC58C92
parent_id = 0x1111111111111111
Expand All @@ -106,12 +107,14 @@ def test_export(self):
base_time,
base_time + 150 * 10 ** 6,
base_time + 300 * 10 ** 6,
base_time + 400 * 10 ** 6,
)
durations = (50 * 10 ** 6, 100 * 10 ** 6, 200 * 10 ** 6)
durations = (50 * 10 ** 6, 100 * 10 ** 6, 200 * 10 ** 6, 300 * 10 ** 6)
end_times = (
start_times[0] + durations[0],
start_times[1] + durations[1],
start_times[2] + durations[2],
start_times[3] + durations[3],
)

span_context = trace_api.SpanContext(
Expand Down Expand Up @@ -158,6 +161,7 @@ def test_export(self):
name=span_names[1], context=parent_context, parent=None
),
trace.Span(name=span_names[2], context=other_context, parent=None),
trace.Span(name=span_names[3], context=other_context, parent=None),
]

otel_spans[0].start(start_time=start_times[0])
Expand All @@ -168,11 +172,21 @@ def test_export(self):
otel_spans[0].end(end_time=end_times[0])

otel_spans[1].start(start_time=start_times[1])
otel_spans[1].resource = Resource(
labels={"key_resource": "some_resource"}
)
otel_spans[1].end(end_time=end_times[1])

otel_spans[2].start(start_time=start_times[2])
otel_spans[2].set_attribute("key_string", "hello_world")
otel_spans[2].resource = Resource(
labels={"key_resource": "some_resource"}
)
otel_spans[2].end(end_time=end_times[2])

otel_spans[3].start(start_time=start_times[3])
otel_spans[3].end(end_time=end_times[3])

service_name = "test-service"
local_endpoint = {"serviceName": service_name, "port": 9411}

Expand Down Expand Up @@ -208,7 +222,7 @@ def test_export(self):
"duration": durations[1] // 10 ** 3,
"localEndpoint": local_endpoint,
"kind": None,
"tags": None,
"tags": {"key_resource": "some_resource"},
"annotations": None,
},
{
Expand All @@ -219,7 +233,21 @@ def test_export(self):
"duration": durations[2] // 10 ** 3,
"localEndpoint": local_endpoint,
"kind": None,
"tags": None,
"tags": {
"key_string": "hello_world",
"key_resource": "some_resource",
},
"annotations": None,
},
{
"traceId": format(trace_id, "x"),
"id": format(other_id, "x"),
"name": span_names[3],
"timestamp": start_times[3] // 10 ** 3,
"duration": durations[3] // 10 ** 3,
"localEndpoint": local_endpoint,
"kind": None,
"tags": {},
"annotations": None,
},
]
Expand Down