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.

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

Improve console span exporter #505

6 changes: 3 additions & 3 deletions docs/examples/basic_tracer/tests/test_tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ def test_basic_tracer(self):
(sys.executable, test_script)
).decode()

self.assertIn('name="foo"', output)
self.assertIn('name="bar"', output)
self.assertIn('name="baz"', output)
self.assertIn('"name": "foo"', output)
self.assertIn('"name": "bar"', output)
self.assertIn('"name": "baz"', output)
2 changes: 1 addition & 1 deletion docs/examples/http/tests/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_http(self):
output = subprocess.check_output(
(sys.executable, test_script)
).decode()
self.assertIn('name="/"', output)
self.assertIn('"name":', output)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason this is changing the test to not check for /?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching that, updated.


@classmethod
def teardown_class(cls):
Expand Down
75 changes: 63 additions & 12 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@


import atexit
import json
import logging
import os
import random
import threading
from collections import OrderedDict
from contextlib import contextmanager
from types import TracebackType
from typing import Iterator, Optional, Sequence, Tuple, Type
Expand Down Expand Up @@ -205,18 +208,66 @@ def __repr__(self):
)

def __str__(self):
return (
'{}(name="{}", context={}, kind={}, '
"parent={}, start_time={}, end_time={})"
).format(
type(self).__name__,
self.name,
self.context,
self.kind,
repr(self.parent),
util.ns_to_iso_str(self.start_time) if self.start_time else "None",
util.ns_to_iso_str(self.end_time) if self.end_time else "None",
)
def format_context(context):
Copy link
Member

Choose a reason for hiding this comment

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

do these need to be inline functions? why not utility functions outside of the string?

I haven't done the investigation, but I wonder if that's adding overhead by requiring lambdas to be generated dynamically every time str is called.

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved them as static functions on the class.

x_ctx = OrderedDict()
Copy link
Member

Choose a reason for hiding this comment

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

is this necessary? also in python3.7 and above, dicts are always ordered.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so. The main purpose of this exporter is for debugging, so I think guaranteeing that the output order is deterministic will help a lot.

x_ctx["trace_id"] = trace_api.format_trace_id(context.trace_id)
x_ctx["span_id"] = trace_api.format_span_id(context.span_id)
x_ctx["trace_state"] = repr(context.trace_state)
return x_ctx

def format_attributes(attributes):
if isinstance(attributes, BoundedDict):
return attributes._dict # pylint: disable=protected-access
return attributes

def format_events(events):
f_events = []
for event in events:
f_event = OrderedDict()
f_event["name"] = event.name
f_event["timestamp"] = util.ns_to_iso_str(event.timestamp)
f_event["attributes"] = format_attributes(event.attributes)
f_events.append(f_event)
return f_events

def format_links(links):
f_links = []
for link in links:
f_link = OrderedDict()
f_link["context"] = format_context(link.context)
f_link["attributes"] = format_attributes(link.attributes)
f_links.append(f_link)
return f_links

parent_id = None
if self.parent is not None:
if isinstance(self.parent, Span):
ctx = self.parent.context
parent_id = trace_api.format_span_id(ctx.span_id)
elif isinstance(self.parent, SpanContext):
parent_id = trace_api.format_span_id(self.parent.span_id)

start_time = None
if self.start_time:
start_time = util.ns_to_iso_str(self.start_time)

end_time = None
if self.end_time:
end_time = util.ns_to_iso_str(self.end_time)

f_span = OrderedDict()

f_span["name"] = self.name
f_span["context"] = format_context(self.context)
f_span["kind"] = str(self.kind)
f_span["parent_id"] = parent_id
f_span["start_time"] = start_time
f_span["end_time"] = end_time
f_span["attributes"] = format_attributes(self.attributes)
f_span["events"] = format_events(self.events)
f_span["links"] = format_links(self.links)

return json.dumps(f_span, indent=4)

def get_context(self):
return self.context
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/tests/trace/export/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ def test_export(self): # pylint: disable=no-self-use

# Mocking stdout interferes with debugging and test reporting, mock on
# the exporter instance instead.
span = trace.Span("span name", mock.Mock())
span = trace.Span("span name", trace_api.INVALID_SPAN_CONTEXT)
with mock.patch.object(exporter, "out") as mock_stdout:
exporter.export([span])
mock_stdout.write.assert_called_once_with(str(span) + os.linesep)
Expand Down