From ffa6b7bab271b868e9f236a2bedf59bbb454651f Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Thu, 16 Jul 2020 09:55:31 +0200 Subject: [PATCH 1/5] Use repr instead of str --- instana/span.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/instana/span.py b/instana/span.py index f40336c6f..179a2d58b 100644 --- a/instana/span.py +++ b/instana/span.py @@ -55,16 +55,17 @@ def set_tag(self, key, value): final_value = value value_type = type(value) - if value_type not in [bool, float, int, list, str]: + if value_type not in [bool, float, int, str]: try: - final_value = str(value) + final_value = repr(value) except: - final_value = "(non-fatal) span.set_tag: values must be one of these types: bool, float, int, list or str. tag discarded" + final_value = "(non-fatal) span.set_tag: values must be one of these types: bool, float, int, list, " \ + "set, str or alternatively support 'repr'. tag discarded" logger.debug(final_value, exc_info=True) + return self return super(InstanaSpan, self).set_tag(key, final_value) - def mark_as_errored(self, tags = None): """ Mark this span as errored. From 09d8e85e69f1c3aff69f4d1361caf753481b2d49 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Thu, 16 Jul 2020 10:01:05 +0200 Subject: [PATCH 2/5] Add tests for sets and more json tests --- tests/opentracing/test_ot_span.py | 33 +++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/opentracing/test_ot_span.py b/tests/opentracing/test_ot_span.py index 589d8f198..d14ba8188 100644 --- a/tests/opentracing/test_ot_span.py +++ b/tests/opentracing/test_ot_span.py @@ -1,3 +1,5 @@ +import sys +import json import time import unittest import opentracing @@ -5,6 +7,8 @@ from instana.util import to_json from instana.singletons import tracer +PY2 = sys.version_info[0] == 2 +PY3 = sys.version_info[0] == 3 class TestOTSpan(unittest.TestCase): def setUp(self): @@ -146,7 +150,7 @@ def test_span_kind(self): span = spans[4] self.assertEqual(3, span.k) - def test_bad_tag_values(self): + def test_tag_values(self): with tracer.start_active_span('test') as scope: # Set a UUID class as a tag # If unchecked, this causes a json.dumps error: "ValueError: Circular reference detected" @@ -155,23 +159,40 @@ def test_bad_tag_values(self): scope.span.set_tag('tracer', tracer) scope.span.set_tag('none', None) scope.span.set_tag('mylist', [1, 2, 3]) - + scope.span.set_tag('myset', {"one", "two", "three"}) spans = tracer.recorder.queued_spans() assert len(spans) == 1 test_span = spans[0] assert(test_span) - assert(len(test_span.data['sdk']['custom']['tags']) == 4) - assert(test_span.data['sdk']['custom']['tags']['uuid'] == '12345678-1234-5678-1234-567812345678') + assert(len(test_span.data['sdk']['custom']['tags']) == 5) + assert(test_span.data['sdk']['custom']['tags']['uuid'] == "UUID('12345678-1234-5678-1234-567812345678')") assert(test_span.data['sdk']['custom']['tags']['tracer']) assert(test_span.data['sdk']['custom']['tags']['none'] == 'None') - assert(test_span.data['sdk']['custom']['tags']['mylist'] == [1, 2, 3]) + assert(test_span.data['sdk']['custom']['tags']['mylist'] == '[1, 2, 3]') + if PY2: + assert(test_span.data['sdk']['custom']['tags']['myset'] == "set(['three', 'two', 'one'])") + else: + assert(test_span.data['sdk']['custom']['tags']['myset'] == "{'two', 'one', 'three'}") + # Convert to JSON json_data = to_json(test_span) assert(json_data) - def test_bad_tag_names(self): + # And back + span_dict = json.loads(json_data) + assert(len(span_dict['data']['sdk']['custom']['tags']) == 5) + assert(span_dict['data']['sdk']['custom']['tags']['uuid'] == "UUID('12345678-1234-5678-1234-567812345678')") + assert(span_dict['data']['sdk']['custom']['tags']['tracer']) + assert(span_dict['data']['sdk']['custom']['tags']['none'] == 'None') + assert(span_dict['data']['sdk']['custom']['tags']['mylist'] == '[1, 2, 3]') + if PY2: + assert(span_dict['data']['sdk']['custom']['tags']['myset'] == "set(['three', 'two', 'one'])") + else: + assert(span_dict['data']['sdk']['custom']['tags']['myset'] == "{'three', 'two', 'one'}") + + def test_tag_names(self): with tracer.start_active_span('test') as scope: # Tag names (keys) must be strings scope.span.set_tag(1234567890, 'This should not get set') From 5f5295f086fed7a671ccb9f6beb5ce2f99e29ce5 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Thu, 16 Jul 2020 11:09:16 +0200 Subject: [PATCH 3/5] Use six for type checking --- instana/span.py | 22 ++++++++++++---------- setup.py | 1 + tests/opentracing/test_ot_span.py | 9 ++++++--- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/instana/span.py b/instana/span.py index 179a2d58b..3b347fa26 100644 --- a/instana/span.py +++ b/instana/span.py @@ -1,3 +1,4 @@ +import six import sys from .log import logger from .util import DictionaryOfStan @@ -5,14 +6,6 @@ import opentracing.ext.tags as ot_tags -PY3 = sys.version_info[0] == 3 - -if PY3: - string_type = str -else: - string_type = basestring - - class SpanContext(): def __init__( self, @@ -49,13 +42,22 @@ def finish(self, finish_time=None): super(InstanaSpan, self).finish(finish_time) def set_tag(self, key, value): - if not isinstance(key, string_type): + # Key validation + if not isinstance(key, six.text_type) and not isinstance(key, six.string_types) : logger.debug("(non-fatal) span.set_tag: tag names must be strings. tag discarded for %s", type(key)) return self final_value = value value_type = type(value) - if value_type not in [bool, float, int, str]: + + # Value validation + if value_type in [bool, float, int, list, str]: + return super(InstanaSpan, self).set_tag(key, final_value) + + elif isinstance(value, six.text_type): + final_value = str(value) + + else: try: final_value = repr(value) except: diff --git a/setup.py b/setup.py index 3d3c0e4fb..f0df8931d 100644 --- a/setup.py +++ b/setup.py @@ -57,6 +57,7 @@ def check_setuptools(): 'fysom>=2.1.2', 'opentracing>=2.0.0', 'requests>=2.8.0', + 'six>=1.12.0', 'urllib3>=1.18.1'], entry_points={ 'instana': ['string = instana:load'], diff --git a/tests/opentracing/test_ot_span.py b/tests/opentracing/test_ot_span.py index d14ba8188..77aac4230 100644 --- a/tests/opentracing/test_ot_span.py +++ b/tests/opentracing/test_ot_span.py @@ -170,7 +170,7 @@ def test_tag_values(self): assert(test_span.data['sdk']['custom']['tags']['uuid'] == "UUID('12345678-1234-5678-1234-567812345678')") assert(test_span.data['sdk']['custom']['tags']['tracer']) assert(test_span.data['sdk']['custom']['tags']['none'] == 'None') - assert(test_span.data['sdk']['custom']['tags']['mylist'] == '[1, 2, 3]') + assert(test_span.data['sdk']['custom']['tags']['mylist'] == [1, 2, 3]) if PY2: assert(test_span.data['sdk']['custom']['tags']['myset'] == "set(['three', 'two', 'one'])") else: @@ -186,7 +186,7 @@ def test_tag_values(self): assert(span_dict['data']['sdk']['custom']['tags']['uuid'] == "UUID('12345678-1234-5678-1234-567812345678')") assert(span_dict['data']['sdk']['custom']['tags']['tracer']) assert(span_dict['data']['sdk']['custom']['tags']['none'] == 'None') - assert(span_dict['data']['sdk']['custom']['tags']['mylist'] == '[1, 2, 3]') + assert(span_dict['data']['sdk']['custom']['tags']['mylist'] == [1, 2, 3]) if PY2: assert(span_dict['data']['sdk']['custom']['tags']['myset'] == "set(['three', 'two', 'one'])") else: @@ -196,13 +196,16 @@ def test_tag_names(self): with tracer.start_active_span('test') as scope: # Tag names (keys) must be strings scope.span.set_tag(1234567890, 'This should not get set') + # Unicode key name + scope.span.set_tag(u'asdf', 'This should be ok') spans = tracer.recorder.queued_spans() assert len(spans) == 1 test_span = spans[0] assert(test_span) - assert(len(test_span.data['sdk']['custom']['tags']) == 0) + assert(len(test_span.data['sdk']['custom']['tags']) == 1) + assert(test_span.data['sdk']['custom']['tags']['asdf'] == 'This should be ok') json_data = to_json(test_span) assert(json_data) From 0e7186c83d8057eacaf344d1ab58a6d85a5bf7cc Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Thu, 16 Jul 2020 11:35:58 +0200 Subject: [PATCH 4/5] Use regexp to validate set strings --- tests/opentracing/test_ot_span.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/opentracing/test_ot_span.py b/tests/opentracing/test_ot_span.py index 77aac4230..5b469adc8 100644 --- a/tests/opentracing/test_ot_span.py +++ b/tests/opentracing/test_ot_span.py @@ -1,3 +1,4 @@ +import re import sys import json import time @@ -159,7 +160,7 @@ def test_tag_values(self): scope.span.set_tag('tracer', tracer) scope.span.set_tag('none', None) scope.span.set_tag('mylist', [1, 2, 3]) - scope.span.set_tag('myset', {"one", "two", "three"}) + scope.span.set_tag('myset', {"one", 2}) spans = tracer.recorder.queued_spans() assert len(spans) == 1 @@ -172,9 +173,11 @@ def test_tag_values(self): assert(test_span.data['sdk']['custom']['tags']['none'] == 'None') assert(test_span.data['sdk']['custom']['tags']['mylist'] == [1, 2, 3]) if PY2: - assert(test_span.data['sdk']['custom']['tags']['myset'] == "set(['three', 'two', 'one'])") + set_regexp = re.compile(r"set\(\[.*,.*\]\)") + assert(set_regexp.search(test_span.data['sdk']['custom']['tags']['myset'])) else: - assert(test_span.data['sdk']['custom']['tags']['myset'] == "{'two', 'one', 'three'}") + set_regexp = re.compile(r"\{.*,.*\}") + assert(set_regexp.search(test_span.data['sdk']['custom']['tags']['myset'])) # Convert to JSON json_data = to_json(test_span) @@ -188,9 +191,11 @@ def test_tag_values(self): assert(span_dict['data']['sdk']['custom']['tags']['none'] == 'None') assert(span_dict['data']['sdk']['custom']['tags']['mylist'] == [1, 2, 3]) if PY2: - assert(span_dict['data']['sdk']['custom']['tags']['myset'] == "set(['three', 'two', 'one'])") + set_regexp = re.compile(r"set\(\[.*,.*\]\)") + assert(set_regexp.search(test_span.data['sdk']['custom']['tags']['myset'])) else: - assert(span_dict['data']['sdk']['custom']['tags']['myset'] == "{'three', 'two', 'one'}") + set_regexp = re.compile(r"\{.*,.*\}") + assert(set_regexp.search(test_span.data['sdk']['custom']['tags']['myset'])) def test_tag_names(self): with tracer.start_active_span('test') as scope: From 84e82e7e80956111d95356f15f6b5c3ed53e87fa Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Thu, 16 Jul 2020 11:37:23 +0200 Subject: [PATCH 5/5] Remove redundant escape --- tests/opentracing/test_ot_span.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/opentracing/test_ot_span.py b/tests/opentracing/test_ot_span.py index 5b469adc8..955dc4805 100644 --- a/tests/opentracing/test_ot_span.py +++ b/tests/opentracing/test_ot_span.py @@ -194,7 +194,7 @@ def test_tag_values(self): set_regexp = re.compile(r"set\(\[.*,.*\]\)") assert(set_regexp.search(test_span.data['sdk']['custom']['tags']['myset'])) else: - set_regexp = re.compile(r"\{.*,.*\}") + set_regexp = re.compile(r"{.*,.*}") assert(set_regexp.search(test_span.data['sdk']['custom']['tags']['myset'])) def test_tag_names(self):