From 6c8afe3177b748767b8214d0af944db6bd70dbfa Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Tue, 16 Jul 2019 18:03:07 +0200 Subject: [PATCH 01/11] span id as integer instead of hex --- .../trace/propagation/google_cloud_format.py | 2 +- opencensus/trace/span_context.py | 40 +++++++++++-------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/opencensus/trace/propagation/google_cloud_format.py b/opencensus/trace/propagation/google_cloud_format.py index 83cd8141b..68895f690 100644 --- a/opencensus/trace/propagation/google_cloud_format.py +++ b/opencensus/trace/propagation/google_cloud_format.py @@ -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 = ';' diff --git a/opencensus/trace/span_context.py b/opencensus/trace/span_context.py index 6cd659e17..15b712341 100644 --- a/opencensus/trace/span_context.py +++ b/opencensus/trace/span_context.py @@ -25,7 +25,6 @@ INVALID_SPAN_ID = '0' * 16 TRACE_ID_PATTERN = re.compile('[0-9a-f]{32}?') -SPAN_ID_PATTERN = re.compile('[0-9a-f]{16}?') # Default options, don't force sampling DEFAULT_OPTIONS = '0' @@ -86,8 +85,8 @@ def __repr__(self): ) def _check_span_id(self, span_id): - """Check the format of the span_id to ensure it is 16-character hex - value representing a 64-bit number. If span_id is invalid, logs a + """Check the format of the span_id to ensure it is a + value representing a 64-bit integer. If span_id is invalid, logs a warning message and returns None :type span_id: str @@ -106,16 +105,14 @@ def _check_span_id(self, span_id): self.from_header = False return None - match = SPAN_ID_PATTERN.match(span_id) - - if match: + if is_64bit_int(span_id): return span_id - else: - logging.warning( - 'Span_id %s does not the match the ' - 'required format', span_id) - self.from_header = False - return None + + logging.warning( + 'Span_id %s does not the match the ' + 'required format', span_id) + self.from_header = False + return None def _check_trace_id(self, trace_id): """Check the format of the trace_id to ensure it is 32-character hex @@ -150,13 +147,13 @@ def _check_trace_id(self, trace_id): def generate_span_id(): - """Return the random generated span ID for a span. Must be a 16 character - hexadecimal encoded string + """Return the random generated span ID for a span. Must be a 64 bit + integer as string :rtype: str - :returns: 16 digit randomly generated hex trace id. + :returns: digit randomly generated trace id. """ - return '{:016x}'.format(random.getrandbits(64)) + return str(random.getrandbits(64)) def generate_trace_id(): @@ -166,3 +163,14 @@ def generate_trace_id(): :returns: 32 digit randomly generated hex trace id. """ return '{:032x}'.format(random.getrandbits(128)) + + +def is_64bit_int(span_id): + """Return if the given string represents a 64-bit integer + + :rtype: bool + """ + try: + return int(span_id, 10)>>64 == 0 + except (TypeError, ValueError): + return False From 551264f5234e06800c184a38096ecf64a0b06a2a Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Tue, 16 Jul 2019 19:45:16 +0200 Subject: [PATCH 02/11] do conversion in propagator methods --- .../trace/propagation/google_cloud_format.py | 5 ++- opencensus/trace/span_context.py | 40 ++++++++----------- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/opencensus/trace/propagation/google_cloud_format.py b/opencensus/trace/propagation/google_cloud_format.py index 68895f690..e26e7c1b8 100644 --- a/opencensus/trace/propagation/google_cloud_format.py +++ b/opencensus/trace/propagation/google_cloud_format.py @@ -61,6 +61,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, @@ -107,7 +110,7 @@ def to_header(self, span_context): header = '{}/{};o={}'.format( trace_id, - span_id, + int(span_id, 16), int(trace_options)) return header diff --git a/opencensus/trace/span_context.py b/opencensus/trace/span_context.py index bfe1b9273..3bd501fd9 100644 --- a/opencensus/trace/span_context.py +++ b/opencensus/trace/span_context.py @@ -25,6 +25,7 @@ INVALID_SPAN_ID = '0' * 16 TRACE_ID_PATTERN = re.compile('[0-9a-f]{32}?') +SPAN_ID_PATTERN = re.compile('[0-9a-f]{16}?') # Default options, don't force sampling DEFAULT_OPTIONS = '0' @@ -84,8 +85,8 @@ def __repr__(self): ) def _check_span_id(self, span_id): - """Check the format of the span_id to ensure it is a - value representing a 64-bit integer. If span_id is invalid, logs a + """Check the format of the span_id to ensure it is 16-character hex + value representing a 64-bit number. If span_id is invalid, logs a warning message and returns None :type span_id: str @@ -104,14 +105,16 @@ def _check_span_id(self, span_id): self.from_header = False return None - if is_64bit_int(span_id): - return span_id + match = SPAN_ID_PATTERN.match(span_id) - logging.warning( - 'Span_id %s does not the match the ' - 'required format', span_id) - self.from_header = False - return None + if match: + return span_id + else: + logging.warning( + 'Span_id %s does not the match the ' + 'required format', span_id) + self.from_header = False + return None def _check_trace_id(self, trace_id): """Check the format of the trace_id to ensure it is 32-character hex @@ -146,13 +149,13 @@ def _check_trace_id(self, trace_id): def generate_span_id(): - """Return the random generated span ID for a span. Must be a 64 bit - integer as string + """Return the random generated span ID for a span. Must be a 16 character + hexadecimal encoded string :rtype: str - :returns: digit randomly generated trace id. + :returns: 16 digit randomly generated hex trace id. """ - return str(random.getrandbits(64)) + return '{:016x}'.format(random.getrandbits(64)) def generate_trace_id(): @@ -162,14 +165,3 @@ def generate_trace_id(): :returns: 32 digit randomly generated hex trace id. """ return '{:032x}'.format(random.getrandbits(128)) - - -def is_64bit_int(span_id): - """Return if the given string represents a 64-bit integer - - :rtype: bool - """ - try: - return int(span_id, 10)>>64 == 0 - except (TypeError, ValueError): - return False From 14d1870e9928666e33215cab3aac3181b47cc7aa Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Tue, 16 Jul 2019 19:50:51 +0200 Subject: [PATCH 03/11] tests updated with correct format of header --- tests/unit/trace/propagation/test_google_cloud_format.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/trace/propagation/test_google_cloud_format.py b/tests/unit/trace/propagation/test_google_cloud_format.py index f7d753e77..cb7635a18 100644 --- a/tests/unit/trace/propagation/test_google_cloud_format.py +++ b/tests/unit/trace/propagation/test_google_cloud_format.py @@ -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' @@ -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' @@ -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' @@ -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' From b0e11c6755938fc18a234ce956c86947c31348da Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Tue, 16 Jul 2019 19:54:36 +0200 Subject: [PATCH 04/11] to_header tests --- tests/unit/trace/propagation/test_google_cloud_format.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/trace/propagation/test_google_cloud_format.py b/tests/unit/trace/propagation/test_google_cloud_format.py index cb7635a18..3f28c4287 100644 --- a/tests/unit/trace/propagation/test_google_cloud_format.py +++ b/tests/unit/trace/propagation/test_google_cloud_format.py @@ -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) @@ -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) From 4778099dd7f55e76478926868909f24dfdfb1d34 Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Tue, 16 Jul 2019 20:13:33 +0200 Subject: [PATCH 05/11] remove encode step --- opencensus/trace/propagation/google_cloud_format.py | 1 - 1 file changed, 1 deletion(-) diff --git a/opencensus/trace/propagation/google_cloud_format.py b/opencensus/trace/propagation/google_cloud_format.py index e26e7c1b8..6b86873bb 100644 --- a/opencensus/trace/propagation/google_cloud_format.py +++ b/opencensus/trace/propagation/google_cloud_format.py @@ -91,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): From 59516ee8de17abbd5d5eeb3b90de1c178261c622 Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Tue, 16 Jul 2019 20:24:20 +0200 Subject: [PATCH 06/11] add to changelog --- CHANGELOG.md | 4 ++++ opencensus/trace/propagation/google_cloud_format.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2599f325..0c45a9f37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/opencensus/trace/propagation/google_cloud_format.py b/opencensus/trace/propagation/google_cloud_format.py index 6b86873bb..2170d2236 100644 --- a/opencensus/trace/propagation/google_cloud_format.py +++ b/opencensus/trace/propagation/google_cloud_format.py @@ -61,7 +61,7 @@ def from_header(self, header): if trace_options is None: trace_options = 1 - + if span_id: span_id = '{:016x}'.format(int(span_id)) From f5cc5f2fef9b7d7cfa5c8a7518a3abe26aae7dfe Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Wed, 17 Jul 2019 17:23:51 +0200 Subject: [PATCH 07/11] update system test --- tests/system/trace/basic_trace/basic_trace_system_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/trace/basic_trace/basic_trace_system_test.py b/tests/system/trace/basic_trace/basic_trace_system_test.py index c6288d3db..1f08bc026 100644 --- a/tests/system/trace/basic_trace/basic_trace_system_test.py +++ b/tests/system/trace/basic_trace/basic_trace_system_test.py @@ -33,7 +33,7 @@ def test_tracer(self): span_id = '6e0c63257de34c92' trace_option = 1 - trace_header = '{}/{};o={}'.format(trace_id, span_id, trace_option) + trace_header = '{}/{};o={}'.format(trace_id, int(span_id, 16), trace_option) sampler = samplers.AlwaysOnSampler() exporter = file_exporter.FileExporter() From b6f886e74dbd08d1387e885e68b208e6979611b0 Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Wed, 17 Jul 2019 17:29:41 +0200 Subject: [PATCH 08/11] line length --- tests/system/trace/basic_trace/basic_trace_system_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/system/trace/basic_trace/basic_trace_system_test.py b/tests/system/trace/basic_trace/basic_trace_system_test.py index 1f08bc026..86a895ed0 100644 --- a/tests/system/trace/basic_trace/basic_trace_system_test.py +++ b/tests/system/trace/basic_trace/basic_trace_system_test.py @@ -33,7 +33,8 @@ def test_tracer(self): span_id = '6e0c63257de34c92' trace_option = 1 - trace_header = '{}/{};o={}'.format(trace_id, int(span_id, 16), trace_option) + trace_header = '{}/{};o={}'.format( + trace_id, int(span_id, 16), trace_option) sampler = samplers.AlwaysOnSampler() exporter = file_exporter.FileExporter() From 551f50babcef4f99330ca1ce2923d94c4e1641b7 Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Wed, 17 Jul 2019 18:50:55 +0200 Subject: [PATCH 09/11] system test updated --- tests/system/trace/flask/flask_system_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/trace/flask/flask_system_test.py b/tests/system/trace/flask/flask_system_test.py index 440606f1f..a733cd245 100644 --- a/tests/system/trace/flask/flask_system_test.py +++ b/tests/system/trace/flask/flask_system_test.py @@ -43,7 +43,7 @@ def generate_header(): span_id = uuid.uuid4().hex[:16] trace_option = 1 - header = '{}/{};o={}'.format(trace_id, span_id, trace_option) + header = '{}/{};o={}'.format(trace_id, int(span_id, 16), trace_option) return trace_id, span_id, header @@ -73,7 +73,7 @@ def setUp(self): self.headers_trace = { 'X-Cloud-Trace-Context': - '{}/{};o={}'.format(self.trace_id, self.span_id, 1) + '{}/{};o={}'.format(self.trace_id, int(self.span_id, 16), 1) } # Wait the application to start From cc83f671c06110818cba94746895db684b779a1d Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Wed, 17 Jul 2019 19:16:07 +0200 Subject: [PATCH 10/11] PR comment --- opencensus/trace/propagation/google_cloud_format.py | 2 +- tests/system/trace/django/django_system_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opencensus/trace/propagation/google_cloud_format.py b/opencensus/trace/propagation/google_cloud_format.py index 2170d2236..90078df1a 100644 --- a/opencensus/trace/propagation/google_cloud_format.py +++ b/opencensus/trace/propagation/google_cloud_format.py @@ -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]{0,32})(\/([\d]{0,20}))?(;o=(\d+))?' +_TRACE_CONTEXT_HEADER_FORMAT = r'([0-9a-f]{32})(\/([\d]{0,20}))?(;o=(\d+))?' _TRACE_CONTEXT_HEADER_RE = re.compile(_TRACE_CONTEXT_HEADER_FORMAT) _TRACE_ID_DELIMETER = '/' _SPAN_ID_DELIMETER = ';' diff --git a/tests/system/trace/django/django_system_test.py b/tests/system/trace/django/django_system_test.py index e9e01c8cc..0bb227e2c 100644 --- a/tests/system/trace/django/django_system_test.py +++ b/tests/system/trace/django/django_system_test.py @@ -43,7 +43,7 @@ def generate_header(): span_id = uuid.uuid4().hex[:16] trace_option = 1 - header = '{}/{};o={}'.format(trace_id, span_id, trace_option) + header = '{}/{};o={}'.format(trace_id, int(span_id, 16), trace_option) return trace_id, span_id, header @@ -74,7 +74,7 @@ def setUp(self): self.headers_trace = { 'x-cloud-trace-context': - '{}/{};o={}'.format(self.trace_id, self.span_id, 1) + '{}/{};o={}'.format(self.trace_id, int(self.span_id, 16), 1) } # Wait the application to start From d8cc6faa69a32bb1635f518d5be9925add47c46a Mon Sep 17 00:00:00 2001 From: Mathieu Hinderyckx Date: Wed, 17 Jul 2019 20:23:24 +0200 Subject: [PATCH 11/11] add test --- .../propagation/test_google_cloud_format.py | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/unit/trace/propagation/test_google_cloud_format.py b/tests/unit/trace/propagation/test_google_cloud_format.py index 3f28c4287..4fe455504 100644 --- a/tests/unit/trace/propagation/test_google_cloud_format.py +++ b/tests/unit/trace/propagation/test_google_cloud_format.py @@ -76,6 +76,56 @@ def test_header_match(self): self.assertEqual(span_context.span_id, expected_span_id) self.assertTrue(span_context.trace_options.enabled) + def test_header_match_no_span_id(self): + # Trace option is not enabled. + header = '6e0c63257de34c92bf9efcd03927272e;o=0' + expected_trace_id = '6e0c63257de34c92bf9efcd03927272e' + expected_span_id = None + + propagator = google_cloud_format.GoogleCloudFormatPropagator() + span_context = propagator.from_header(header) + + self.assertEqual(span_context.trace_id, expected_trace_id) + self.assertEqual(span_context.span_id, expected_span_id) + self.assertFalse(span_context.trace_options.enabled) + + # Trace option is enabled. + header = '6e0c63257de34c92bf9efcd03927272e;o=1' + expected_trace_id = '6e0c63257de34c92bf9efcd03927272e' + expected_span_id = None + + propagator = google_cloud_format.GoogleCloudFormatPropagator() + span_context = propagator.from_header(header) + + self.assertEqual(span_context.trace_id, expected_trace_id) + self.assertEqual(span_context.span_id, expected_span_id) + self.assertTrue(span_context.trace_options.enabled) + + def test_header_match_empty_span_id(self): + # Trace option is not enabled. + header = '6e0c63257de34c92bf9efcd03927272e/;o=0' + expected_trace_id = '6e0c63257de34c92bf9efcd03927272e' + expected_span_id = None + + propagator = google_cloud_format.GoogleCloudFormatPropagator() + span_context = propagator.from_header(header) + + self.assertEqual(span_context.trace_id, expected_trace_id) + self.assertEqual(span_context.span_id, expected_span_id) + self.assertFalse(span_context.trace_options.enabled) + + # Trace option is enabled. + header = '6e0c63257de34c92bf9efcd03927272e/;o=1' + expected_trace_id = '6e0c63257de34c92bf9efcd03927272e' + expected_span_id = None + + propagator = google_cloud_format.GoogleCloudFormatPropagator() + span_context = propagator.from_header(header) + + self.assertEqual(span_context.trace_id, expected_trace_id) + self.assertEqual(span_context.span_id, expected_span_id) + self.assertTrue(span_context.trace_options.enabled) + def test_header_match_no_option(self): header = '6e0c63257de34c92bf9efcd03927272e/67667974448284343' expected_trace_id = '6e0c63257de34c92bf9efcd03927272e'