Skip to content

Span id encoding #719

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

Merged
merged 14 commits into from
Jul 17, 2019
Merged

Span id encoding #719

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a5ed203
Merge pull request #1 from census-instrumentation/master
mhindery Jul 2, 2019
6c8afe3
span id as integer instead of hex
mhindery Jul 16, 2019
a1adbf0
Merge remote-tracking branch 'upstream/master'
mhindery Jul 16, 2019
5655d22
Merge branch 'master' into span_id_encoding
mhindery Jul 16, 2019
551264f
do conversion in propagator methods
mhindery Jul 16, 2019
14d1870
tests updated with correct format of header
mhindery Jul 16, 2019
b0e11c6
to_header tests
mhindery Jul 16, 2019
4778099
remove encode step
mhindery Jul 16, 2019
59516ee
add to changelog
mhindery Jul 16, 2019
f5cc5f2
update system test
mhindery Jul 17, 2019
b6f886e
line length
mhindery Jul 17, 2019
551f50b
system test updated
mhindery Jul 17, 2019
cc83f67
PR comment
mhindery Jul 17, 2019
d8cc6fa
add test
mhindery Jul 17, 2019
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

- Fix cloud format propagator to use decimal span_id encoding instead of hex
([#719](https://github.com/census-instrumentation/opencensus-python/pull/719))


## 0.6.0
Released 2019-05-31

Expand Down
8 changes: 5 additions & 3 deletions opencensus/trace/propagation/google_cloud_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from opencensus.trace.trace_options import TraceOptions

_TRACE_CONTEXT_HEADER_NAME = 'X-Cloud-Trace-Context'
_TRACE_CONTEXT_HEADER_FORMAT = r'([0-9a-f]{32})(\/([0-9a-f]{16}))?(;o=(\d+))?'
_TRACE_CONTEXT_HEADER_FORMAT = r'([0-9a-f]{0,32})(\/([\d]{0,20}))?(;o=(\d+))?'
_TRACE_CONTEXT_HEADER_RE = re.compile(_TRACE_CONTEXT_HEADER_FORMAT)
_TRACE_ID_DELIMETER = '/'
_SPAN_ID_DELIMETER = ';'
Expand Down Expand Up @@ -62,6 +62,9 @@ def from_header(self, header):
if trace_options is None:
trace_options = 1

if span_id:
span_id = '{:016x}'.format(int(span_id))

span_context = SpanContext(
trace_id=trace_id,
span_id=span_id,
Expand All @@ -88,7 +91,6 @@ def from_headers(self, headers):
header = headers.get(_TRACE_CONTEXT_HEADER_NAME)
if header is None:
return SpanContext()
header = str(header.encode('utf-8'))
return self.from_header(header)

def to_header(self, span_context):
Expand All @@ -107,7 +109,7 @@ def to_header(self, span_context):

header = '{}/{};o={}'.format(
trace_id,
span_id,
int(span_id, 16),
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need to update from_header as well.

@c24t FYI - this is a potential breaking change (e.g. if customers mix use old version and the latest version of OpenCensus Python SDK).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the from_header has been updated, it contains

            if span_id:
                span_id = '{:016x}'.format(int(span_id))

to do the conversion, or do you mean something additional?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mhindery please update CHANGELOG.md if this is a breaking change. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding @c24t to comment from Google side.

I cannot find the official spec for X-Cloud-Trace-Context, and I'm a bit worried that this change might break existing stuff for Google (considering there are system tests running as part of the CI, it is a bit surprising to me that we've been sending the wrong format to Stackdriver).

int(trace_options))
return header

Expand Down
13 changes: 7 additions & 6 deletions tests/unit/trace/propagation/test_google_cloud_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_header_type_error(self):

def test_header_match(self):
# Trace option is not enabled.
header = '6e0c63257de34c92bf9efcd03927272e/00f067aa0ba902b7;o=0'
header = '6e0c63257de34c92bf9efcd03927272e/67667974448284343;o=0'
expected_trace_id = '6e0c63257de34c92bf9efcd03927272e'
expected_span_id = '00f067aa0ba902b7'

Expand All @@ -65,7 +65,7 @@ def test_header_match(self):
self.assertFalse(span_context.trace_options.enabled)

# Trace option is enabled.
header = '6e0c63257de34c92bf9efcd03927272e/00f067aa0ba902b7;o=1'
header = '6e0c63257de34c92bf9efcd03927272e/67667974448284343;o=1'
expected_trace_id = '6e0c63257de34c92bf9efcd03927272e'
expected_span_id = '00f067aa0ba902b7'

Expand All @@ -77,7 +77,7 @@ def test_header_match(self):
self.assertTrue(span_context.trace_options.enabled)

def test_header_match_no_option(self):
header = '6e0c63257de34c92bf9efcd03927272e/00f067aa0ba902b7'
header = '6e0c63257de34c92bf9efcd03927272e/67667974448284343'
expected_trace_id = '6e0c63257de34c92bf9efcd03927272e'
expected_span_id = '00f067aa0ba902b7'

Expand All @@ -101,7 +101,7 @@ def test_headers_match(self):
# Trace option is enabled.
headers = {
'X-Cloud-Trace-Context':
'6e0c63257de34c92bf9efcd03927272e/00f067aa0ba902b7;o=1',
'6e0c63257de34c92bf9efcd03927272e/67667974448284343;o=1',
}
expected_trace_id = '6e0c63257de34c92bf9efcd03927272e'
expected_span_id = '00f067aa0ba902b7'
Expand All @@ -128,7 +128,7 @@ def test_to_header(self):

header = propagator.to_header(span_context)
expected_header = '{}/{};o={}'.format(
trace_id, span_id, 1)
trace_id, int(span_id, 16), 1)

self.assertEqual(header, expected_header)

Expand All @@ -147,7 +147,8 @@ def test_to_headers(self):

headers = propagator.to_headers(span_context)
expected_headers = {
'X-Cloud-Trace-Context': '{}/{};o={}'.format(trace_id, span_id, 1),
'X-Cloud-Trace-Context': '{}/{};o={}'.format(
trace_id, int(span_id, 16), 1),
}

self.assertEqual(headers, expected_headers)