Use unsigned base 16 IDs internally & update tests (#119)

* Use Unsigned based 16 IDs internally & update tests

* Add test for 128 bit incoming headers

* Remove unused imports; Add function documentation

* Fix assertion values

Author: pglombardo

instana/http_propagator.py

@@ -4,7 +4,7 @@
 from basictracer.context import SpanContext
 
 from .log import logger
-from .util import id_to_header, header_to_id
+from .util import header_to_id
 
 # The carrier can be a dict or a list.
 # Using the trace header as an example, it can be in the following forms
@@ -37,8 +37,8 @@ class HTTPPropagator():
 
     def inject(self, span_context, carrier):
         try:
-            trace_id = id_to_header(span_context.trace_id)
-            span_id = id_to_header(span_context.span_id)
+            trace_id = span_context.trace_id
+            span_id = span_context.span_id
 
             if type(carrier) is dict or hasattr(carrier, "__dict__"):
                 carrier[self.HEADER_KEY_T] = trace_id

instana/text_propagator.py

@@ -4,7 +4,7 @@
 from basictracer.context import SpanContext
 
 from .log import logger
-from .util import id_to_header, header_to_id
+from .util import header_to_id
 
 prefix_tracer_state = 'X-INSTANA-'
 prefix_baggage = 'X-INSTANA-BAGGAGE-'
@@ -19,8 +19,8 @@ class TextPropagator():
 
     def inject(self, span_context, carrier):
         try:
-            carrier[field_name_trace_id] = '{0:x}'.format(span_context.trace_id)
-            carrier[field_name_span_id] = '{0:x}'.format(span_context.span_id)
+            carrier[field_name_trace_id] = span_context.trace_id
+            carrier[field_name_span_id] = span_context.span_id
             if span_context.baggage is not None:
                 for k in span_context.baggage:
                     carrier[prefix_baggage+k] = span_context.baggage[k]

instana/util.py

@@ -1,9 +1,7 @@
-import binascii
 import json
 import os
 import random
 import re
-import struct
 import sys
 import time
 
@@ -26,54 +24,62 @@
 _rnd = random.Random()
 _current_pid = 0
 
-BAD_ID_LONG = 3135097598  # Bad Cafe in base 10
-BAD_ID_HEADER = "BADDCAFE"  # Bad Cafe
+BAD_ID = "BADCAFFE"  # Bad Caffe
 
 
 def generate_id():
-    """ Generate a 64bit signed integer for use as a Span or Trace ID """
+    """ Generate a 64bit base 16 ID for use as a Span or Trace ID """
     global _current_pid
 
     pid = os.getpid()
     if _current_pid != pid:
         _current_pid = pid
         _rnd.seed(int(1000000 * time.time()) ^ pid)
-    return _rnd.randint(-9223372036854775808, 9223372036854775807)
+    id = format(_rnd.randint(0, 18446744073709551615), '02x')
 
+    if len(id) < 16:
+        id = id.zfill(16)
 
-def id_to_header(id):
-    """ Convert a 64bit signed integer to an unsigned base 16 hex string """
-
-    try:
-        if not isinstance(id, int):
-            return BAD_ID_HEADER
-
-        byte_string = struct.pack('>q', id)
-        return str(binascii.hexlify(byte_string).decode('UTF-8').lstrip('0'))
-    except Exception as e:
-        logger.debug(e)
-        return BAD_ID_HEADER
+    return id
 
 
 def header_to_id(header):
-    """ Convert an unsigned base 16 hex string into a 64bit signed integer """
+    """
+    We can receive headers in the following formats:
+      1. unsigned base 16 hex string of variable length
+      2. [eventual]
 
+    :param header: the header to analyze, validate and convert (if needed)
+    :return: a valid ID to be used internal to the tracer
+    """
     if not isinstance(header, string_types):
-        return BAD_ID_LONG
+        return BAD_ID
 
     try:
         # Test that header is truly a hexadecimal value before we try to convert
         int(header, 16)
 
-        # Pad the header to 16 chars
-        header = header.zfill(16)
-        r = binascii.unhexlify(header)
-        return struct.unpack('>q', r)[0]
+        length = len(header)
+        if length < 16:
+            # Left pad ID with zeros
+            header = header.zfill(16)
+        elif length > 16:
+            # Phase 0: Discard everything but the last 16byte
+            header = header[-16:]
+
+        return header
     except ValueError:
-        return BAD_ID_LONG
+        return BAD_ID
 
 
 def to_json(obj):
+    """
+    Convert obj to json.  Used mostly to convert the classes in json_span.py until we switch to nested
+    dicts (or something better)
+
+    :param obj: the object to serialize to json
+    :return:  json string
+    """
     try:
         return json.dumps(obj, default=lambda obj: {k.lower(): v for k, v in obj.__dict__.items()},
                           sort_keys=False, separators=(',', ':')).encode()
@@ -82,6 +88,11 @@ def to_json(obj):
 
 
 def package_version():
+    """
+    Determine the version of this package.
+
+    :return: String representing known version
+    """
     version = ""
     try:
         version = pkg_resources.get_distribution('instana').version

tests/test_django.py

@@ -212,7 +212,7 @@ def test_with_incoming_context(self):
         assert_equals(200, response.status)
         assert('X-Instana-T' in response.headers)
         assert(int(response.headers['X-Instana-T'], 16))
-        self.assertEqual('1', response.headers['X-Instana-T'])
+        self.assertEqual('0000000000000001', response.headers['X-Instana-T'])
         assert('X-Instana-S' in response.headers)
         assert(int(response.headers['X-Instana-S'], 16))
         assert('X-Instana-L' in response.headers)
@@ -223,8 +223,8 @@ def test_with_incoming_context(self):
 
         django_span = spans[0]
 
-        assert_equals(django_span.t, 1)
-        assert_equals(django_span.p, 1)
+        assert_equals(django_span.t, '0000000000000001')
+        assert_equals(django_span.p, '0000000000000001')
 
     def test_with_incoming_mixed_case_context(self):
         request_headers = dict()
@@ -237,7 +237,7 @@ def test_with_incoming_mixed_case_context(self):
         assert_equals(200, response.status)
         assert('X-Instana-T' in response.headers)
         assert(int(response.headers['X-Instana-T'], 16))
-        self.assertEqual('1', response.headers['X-Instana-T'])
+        self.assertEqual('0000000000000001', response.headers['X-Instana-T'])
         assert('X-Instana-S' in response.headers)
         assert(int(response.headers['X-Instana-S'], 16))
         assert('X-Instana-L' in response.headers)
@@ -248,5 +248,5 @@ def test_with_incoming_mixed_case_context(self):
 
         django_span = spans[0]
 
-        assert_equals(django_span.t, 1)
-        assert_equals(django_span.p, 1)
+        assert_equals(django_span.t, '0000000000000001')
+        assert_equals(django_span.p, '0000000000000001')

tests/test_id_management.py

@@ -13,126 +13,49 @@
 
 def test_id_generation():
     count = 0
-    while count <= 1000:
+    while count <= 10000:
         id = instana.util.generate_id()
-        assert id >= -9223372036854775808
-        assert id <= 9223372036854775807
+        base10_id = int(id, 16)
+        assert base10_id >= 0
+        assert base10_id <= 18446744073709551615
         count += 1
 
 
-def test_id_max_value_and_conversion():
-    max_id = 9223372036854775807
-    min_id = -9223372036854775808
-    max_hex = "7fffffffffffffff"
-    min_hex = "8000000000000000"
-
-    assert_equals(max_hex, instana.util.id_to_header(max_id))
-    assert_equals(min_hex, instana.util.id_to_header(min_id))
-
-    assert_equals(max_id, instana.util.header_to_id(max_hex))
-    assert_equals(min_id, instana.util.header_to_id(min_hex))
-
-
-def test_id_conversion_back_and_forth():
-    # id --> header --> id
-    original_id = instana.util.generate_id()
-    header_id = instana.util.id_to_header(original_id)
-    converted_back_id = instana.util.header_to_id(header_id)
-    assert original_id == converted_back_id
-
-    # header --> id --> header
-    original_header_id = "c025ee93b1aeda7b"
-    id = instana.util.header_to_id(original_header_id)
-    converted_back_header_id = instana.util.id_to_header(id)
-    assert_equals(original_header_id, converted_back_header_id)
-
-    # Test a random value
-    id = -7815363404733516491
-    header = "938a406416457535"
-
-    result = instana.util.header_to_id(header)
-    assert_equals(id, result)
-
-    result = instana.util.id_to_header(id)
-    assert_equals(header, result)
-
-
-def test_that_leading_zeros_handled_correctly():
-    header = instana.util.id_to_header(16)
-    assert_equals("10", header)
-
-    id = instana.util.header_to_id("10")
-    assert_equals(16, id)
-
-    id = instana.util.header_to_id("0000000000000010")
-    assert_equals(16, id)
-
-    id = instana.util.header_to_id("88b6c735206ca42")
-    assert_equals(615705016619420226, id)
-
-    id = instana.util.header_to_id("088b6c735206ca42")
-    assert_equals(615705016619420226, id)
-
-
-def test_id_to_header_conversion():
-    # Test passing a standard Integer ID
-    original_id = instana.util.generate_id()
-    converted_id = instana.util.id_to_header(original_id)
-
-    # Assert that it is a string and there are no non-hex characters
-    assert isinstance(converted_id, string_types)
-    assert all(c in string.hexdigits for c in converted_id)
-
-    # Test passing a standard Integer ID as a String
-    original_id = instana.util.generate_id()
-    converted_id = instana.util.id_to_header(original_id)
-
-    # Assert that it is a string and there are no non-hex characters
-    assert isinstance(converted_id, string_types)
-    assert all(c in string.hexdigits for c in converted_id)
-
-
-def test_id_to_header_conversion_with_bogus_id():
-    # Test passing an empty String
-    converted_id = instana.util.id_to_header('')
-
-    # Assert that it is a string and there are no non-hex characters
-    assert isinstance(converted_id, string_types)
-    assert converted_id == instana.util.BAD_ID_HEADER
-
-    # Test passing a nil
-    converted_id = instana.util.id_to_header(None)
-
-    # Assert that it is a string and there are no non-hex characters
-    assert isinstance(converted_id, string_types)
-    assert converted_id == instana.util.BAD_ID_HEADER
-
-    # Test passing an Array
-    converted_id = instana.util.id_to_header([])
-
-    # Assert that it is a string and there are no non-hex characters
-    assert isinstance(converted_id, string_types)
-    assert converted_id == instana.util.BAD_ID_HEADER
-
-
-def test_header_to_id_conversion():
+def test_various_header_to_id_conversion():
     # Get a hex string to test against & convert
-    header_id = instana.util.id_to_header(instana.util.generate_id)
+    header_id = instana.util.generate_id()
     converted_id = instana.util.header_to_id(header_id)
+    assert_equals(header_id, converted_id)
 
-    # Assert that it is an Integer
-    assert isinstance(converted_id, int)
+    # Hex value - result should be left padded
+    result = instana.util.header_to_id('abcdef')
+    assert_equals('0000000000abcdef', result)
+
+    # Hex value
+    result = instana.util.header_to_id('0123456789abcdef')
+    assert_equals('0123456789abcdef', result)
+
+    # Very long incoming header should just return the rightmost 16 bytes
+    result = instana.util.header_to_id('0x0123456789abcdef0123456789abcdef')
+    assert_equals('0123456789abcdef', result)
 
 
 def test_header_to_id_conversion_with_bogus_header():
     # Bogus nil arg
     bogus_result = instana.util.header_to_id(None)
-    assert_equals(instana.util.BAD_ID_LONG, bogus_result)
+    assert_equals(instana.util.BAD_ID, bogus_result)
 
     # Bogus Integer arg
     bogus_result = instana.util.header_to_id(1234)
-    assert_equals(instana.util.BAD_ID_LONG, bogus_result)
+    assert_equals(instana.util.BAD_ID, bogus_result)
 
     # Bogus Array arg
     bogus_result = instana.util.header_to_id([1234])
-    assert_equals(instana.util.BAD_ID_LONG, bogus_result)
+    assert_equals(instana.util.BAD_ID, bogus_result)
+
+    # Bogus Hex Values in String
+    bogus_result = instana.util.header_to_id('0xZZZZZZ')
+    assert_equals(instana.util.BAD_ID, bogus_result)
+
+    bogus_result = instana.util.header_to_id('ZZZZZZ')
+    assert_equals(instana.util.BAD_ID, bogus_result)

tests/test_ot_propagators.py

@@ -30,9 +30,9 @@ def test_inject():
     ot.tracer.inject(span.context, ot.Format.HTTP_HEADERS, carrier)
 
     assert 'X-Instana-T' in carrier
-    assert_equals(carrier['X-Instana-T'], util.id_to_header(span.context.trace_id))
+    assert_equals(carrier['X-Instana-T'], span.context.trace_id)
     assert 'X-Instana-S' in carrier
-    assert_equals(carrier['X-Instana-S'], util.id_to_header(span.context.span_id))
+    assert_equals(carrier['X-Instana-S'], span.context.span_id)
     assert 'X-Instana-L' in carrier
     assert_equals(carrier['X-Instana-L'], "1")
 
@@ -45,8 +45,8 @@ def test_basic_extract():
     ctx = ot.tracer.extract(ot.Format.HTTP_HEADERS, carrier)
 
     assert type(ctx) is basictracer.context.SpanContext
-    assert_equals(1, ctx.trace_id)
-    assert_equals(1, ctx.span_id)
+    assert_equals('0000000000000001', ctx.trace_id)
+    assert_equals('0000000000000001', ctx.span_id)
 
 
 def test_mixed_case_extract():
@@ -57,8 +57,8 @@ def test_mixed_case_extract():
     ctx = ot.tracer.extract(ot.Format.HTTP_HEADERS, carrier)
 
     assert type(ctx) is basictracer.context.SpanContext
-    assert_equals(1, ctx.trace_id)
-    assert_equals(1, ctx.span_id)
+    assert_equals('0000000000000001', ctx.trace_id)
+    assert_equals('0000000000000001', ctx.span_id)
 
 
 def test_no_context_extract():
@@ -69,3 +69,17 @@ def test_no_context_extract():
     ctx = ot.tracer.extract(ot.Format.HTTP_HEADERS, carrier)
 
     assert ctx is None
+
+
+def test_128bit_headers():
+    opts = options.Options()
+    ot.tracer = InstanaTracer(opts)
+
+    carrier = {'X-Instana-T': '0000000000000000b0789916ff8f319f',
+               'X-Instana-S': '0000000000000000b0789916ff8f319f', 'X-Instana-L': '1'}
+    ctx = ot.tracer.extract(ot.Format.HTTP_HEADERS, carrier)
+
+    assert type(ctx) is basictracer.context.SpanContext
+    assert_equals('b0789916ff8f319f', ctx.trace_id)
+    assert_equals('b0789916ff8f319f', ctx.span_id)
+