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

fix parent_id formatting inconsistency #1833

Merged
merged 4 commits into from
May 10, 2021
Merged

fix parent_id formatting inconsistency #1833

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aacca99
fix parent_id formatting inconsistency
May 10, 2021
489b450
update changelog
May 10, 2021
8567bea
Merge branch 'main' into codeboten/fix-1832
May 10, 2021
dac5eb2
Merge branch 'main' into codeboten/fix-1832
May 10, 2021
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Propagators use the root context as default for `extract` and do not modify
the context if extracting from carrier does not work.
([#1811](https://github.com/open-telemetry/opentelemetry-python/pull/1811))
- Fixed inconsistency in parent_id formatting from the ConsoleSpanExporter
([#1833](https://github.com/open-telemetry/opentelemetry-python/pull/1833))

### Removed
- Moved `opentelemetry-instrumentation` to contrib repository.
Expand Down
8 changes: 6 additions & 2 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,9 +483,13 @@ def to_json(self, indent=4):
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)
parent_id = "0x{}".format(
trace_api.format_span_id(ctx.span_id)
)
elif isinstance(self.parent, SpanContext):
parent_id = trace_api.format_span_id(self.parent.span_id)
parent_id = "0x{}".format(
trace_api.format_span_id(self.parent.span_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused by this. Doesn't format_span_id already format as hex and wouldn't this add 0x twice to the string?

Copy link
Contributor Author

@codeboten codeboten May 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

it changes the values to hex, but doesnt prepend the string w/ 0x, see the _format_context method at line 526

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably a gap in my understanding but wouldn't that be inconsistent with trace id and span id values then? Seems weird that we'd render parent_id differently from span/trace ID. Are we forced to do this because we documented it this way? Sounds like a bug in the documentation then :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

format context prepends 0x to the trace_id and span_id

    @staticmethod
    @staticmethod
    def _format_context(context):
    def _format_context(context):
        x_ctx = OrderedDict()
        x_ctx = OrderedDict()
        x_ctx["trace_id"] = "0x{}".format(
        x_ctx["trace_id"] = "0x{}".format(
            trace_api.format_trace_id(context.trace_id)
            trace_api.format_trace_id(context.trace_id)
        )
        )
        x_ctx["span_id"] = "0x{}".format(
        x_ctx["span_id"] = "0x{}".format(
            trace_api.format_span_id(context.span_id)
            trace_api.format_span_id(context.span_id)
        )
        )
        x_ctx["trace_state"] = repr(context.trace_state)
        x_ctx["trace_state"] = repr(context.trace_state)
        return x_ctx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change makes the formatting consistent in parent_id as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Makes sense.

)

start_time = None
if self._start_time:
Expand Down
9 changes: 6 additions & 3 deletions opentelemetry-sdk/tests/trace/test_trace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,10 @@ def test_to_json(self):
is_remote=False,
trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED),
)
span = trace._Span("span-name", context, resource=Resource({}))
parent = trace._Span("parent-name", context, resource=Resource({}))
span = trace._Span(
"span-name", context, resource=Resource({}), parent=parent
)

self.assertEqual(
span.to_json(),
Expand All @@ -1233,7 +1236,7 @@ def test_to_json(self):
"trace_state": "[]"
},
"kind": "SpanKind.INTERNAL",
"parent_id": null,
"parent_id": "0x00000000deadbef0",
Copy link
Contributor

Choose a reason for hiding this comment

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

😂

"start_time": null,
"end_time": null,
"status": {
Expand All @@ -1247,7 +1250,7 @@ def test_to_json(self):
)
self.assertEqual(
span.to_json(indent=None),
'{"name": "span-name", "context": {"trace_id": "0x000000000000000000000000deadbeef", "span_id": "0x00000000deadbef0", "trace_state": "[]"}, "kind": "SpanKind.INTERNAL", "parent_id": null, "start_time": null, "end_time": null, "status": {"status_code": "UNSET"}, "attributes": {}, "events": [], "links": [], "resource": {}}',
'{"name": "span-name", "context": {"trace_id": "0x000000000000000000000000deadbeef", "span_id": "0x00000000deadbef0", "trace_state": "[]"}, "kind": "SpanKind.INTERNAL", "parent_id": "0x00000000deadbef0", "start_time": null, "end_time": null, "status": {"status_code": "UNSET"}, "attributes": {}, "events": [], "links": [], "resource": {}}',
)

def test_attributes_to_json(self):
Expand Down