From 530cd7095ff76806b17402940b8a446714e69103 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Wed, 15 Jul 2020 15:23:40 +0200 Subject: [PATCH 1/4] Span.set_tag: validate tag name and value types --- instana/span.py | 26 ++++++++++++++++++++++++++ tests/{ => clients}/test_logging.py | 0 2 files changed, 26 insertions(+) rename tests/{ => clients}/test_logging.py (100%) diff --git a/instana/span.py b/instana/span.py index 78330817..98983330 100644 --- a/instana/span.py +++ b/instana/span.py @@ -1,9 +1,18 @@ +import sys from .log import logger from .util import DictionaryOfStan from basictracer.span import BasicSpan 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, @@ -39,6 +48,23 @@ class InstanaSpan(BasicSpan): def finish(self, finish_time=None): super(InstanaSpan, self).finish(finish_time) + def set_tag(self, key, value): + if not isinstance(key, string_type): + 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]: + try: + final_value = str(value) + except: + final_value = "(non-fatal) span.set_tag: values must be one of these types: bool, float, int or str. tag discarded" + logger.debug(final_value, exc_info=True) + + return super(InstanaSpan, self).set_tag(key, final_value) + + def mark_as_errored(self, tags = None): """ Mark this span as errored. diff --git a/tests/test_logging.py b/tests/clients/test_logging.py similarity index 100% rename from tests/test_logging.py rename to tests/clients/test_logging.py From f3888ad3546744fd8bd8c58caaf6e8665c1a0047 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Wed, 15 Jul 2020 15:24:25 +0200 Subject: [PATCH 2/4] Test set_tag bad data types --- tests/test_utils.py | 51 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/test_utils.py diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 00000000..8d092a1e --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,51 @@ +from __future__ import absolute_import + +from uuid import UUID +from instana.util import to_json +from instana.singletons import tracer + + +def setup_method(): + """ Clear all spans before a test run """ + tracer.recorder.clear_spans() + + +def test_to_json_bad_tag_values(): + 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" + scope.span.set_tag('uuid', UUID(bytes=b'\x12\x34\x56\x78'*4)) + # Arbitrarily setting an instance of some class + scope.span.set_tag('tracer', tracer) + scope.span.set_tag('none', None) + + + spans = tracer.recorder.queued_spans() + assert len(spans) == 1 + + test_span = spans[0] + assert(test_span) + assert(len(test_span.data['sdk']['custom']['tags']) == 3) + assert(test_span.data['sdk']['custom']['tags']['uuid'] == '12345678-1234-5678-1234-567812345678') + assert(test_span.data['sdk']['custom']['tags']['tracer']) + assert(test_span.data['sdk']['custom']['tags']['none'] == 'None') + + json_data = to_json(test_span) + assert(json_data) + + +def test_to_json_bad_key_values(): + with tracer.start_active_span('test') as scope: + # Tag names (keys) must be strings + scope.span.set_tag(1234567890, 'This should not get set') + + 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) + + json_data = to_json(test_span) + assert(json_data) + From fed6e75d96e1bbdbec2d582bd252192fef539fa9 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Wed, 15 Jul 2020 15:33:38 +0200 Subject: [PATCH 3/4] Add list to the valid types --- instana/span.py | 4 ++-- tests/test_utils.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/instana/span.py b/instana/span.py index 98983330..f40336c6 100644 --- a/instana/span.py +++ b/instana/span.py @@ -55,11 +55,11 @@ def set_tag(self, key, value): final_value = value value_type = type(value) - if value_type not in [bool, float, int, str]: + if value_type not in [bool, float, int, list, str]: try: final_value = str(value) except: - final_value = "(non-fatal) span.set_tag: values must be one of these types: bool, float, int or str. tag discarded" + final_value = "(non-fatal) span.set_tag: values must be one of these types: bool, float, int, list or str. tag discarded" logger.debug(final_value, exc_info=True) return super(InstanaSpan, self).set_tag(key, final_value) diff --git a/tests/test_utils.py b/tests/test_utils.py index 8d092a1e..ee39cdcf 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -18,6 +18,7 @@ def test_to_json_bad_tag_values(): # Arbitrarily setting an instance of some class scope.span.set_tag('tracer', tracer) scope.span.set_tag('none', None) + scope.span.set_tag('mylist', [1, 2, 3]) spans = tracer.recorder.queued_spans() @@ -25,16 +26,17 @@ def test_to_json_bad_tag_values(): test_span = spans[0] assert(test_span) - assert(len(test_span.data['sdk']['custom']['tags']) == 3) + assert(len(test_span.data['sdk']['custom']['tags']) == 4) assert(test_span.data['sdk']['custom']['tags']['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]) json_data = to_json(test_span) assert(json_data) -def test_to_json_bad_key_values(): +def test_to_json_bad_tag_names(): 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 d9c6d503756da8c5f83297fb897e44029b2d40d7 Mon Sep 17 00:00:00 2001 From: Peter Giacomo Lombardo Date: Wed, 15 Jul 2020 16:10:27 +0200 Subject: [PATCH 4/4] Move tests to the opentracing area --- tests/opentracing/test_ot_span.py | 44 ++++++++++++++++++++++++- tests/test_utils.py | 53 ------------------------------- 2 files changed, 43 insertions(+), 54 deletions(-) delete mode 100644 tests/test_utils.py diff --git a/tests/opentracing/test_ot_span.py b/tests/opentracing/test_ot_span.py index a01e7998..589d8f19 100644 --- a/tests/opentracing/test_ot_span.py +++ b/tests/opentracing/test_ot_span.py @@ -1,7 +1,8 @@ import time - import unittest import opentracing +from uuid import UUID +from instana.util import to_json from instana.singletons import tracer @@ -144,3 +145,44 @@ def test_span_kind(self): span = spans[4] self.assertEqual(3, span.k) + + def test_bad_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" + scope.span.set_tag('uuid', UUID(bytes=b'\x12\x34\x56\x78'*4)) + # Arbitrarily setting an instance of some class + scope.span.set_tag('tracer', tracer) + scope.span.set_tag('none', None) + scope.span.set_tag('mylist', [1, 2, 3]) + + + 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(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]) + + json_data = to_json(test_span) + assert(json_data) + + def test_bad_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') + + 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) + + json_data = to_json(test_span) + assert(json_data) + diff --git a/tests/test_utils.py b/tests/test_utils.py deleted file mode 100644 index ee39cdcf..00000000 --- a/tests/test_utils.py +++ /dev/null @@ -1,53 +0,0 @@ -from __future__ import absolute_import - -from uuid import UUID -from instana.util import to_json -from instana.singletons import tracer - - -def setup_method(): - """ Clear all spans before a test run """ - tracer.recorder.clear_spans() - - -def test_to_json_bad_tag_values(): - 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" - scope.span.set_tag('uuid', UUID(bytes=b'\x12\x34\x56\x78'*4)) - # Arbitrarily setting an instance of some class - scope.span.set_tag('tracer', tracer) - scope.span.set_tag('none', None) - scope.span.set_tag('mylist', [1, 2, 3]) - - - 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(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]) - - json_data = to_json(test_span) - assert(json_data) - - -def test_to_json_bad_tag_names(): - with tracer.start_active_span('test') as scope: - # Tag names (keys) must be strings - scope.span.set_tag(1234567890, 'This should not get set') - - 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) - - json_data = to_json(test_span) - assert(json_data) -