Span.set_tag: validate tag name and value types (#245)

* Span.set_tag: validate tag name and value types

* Test set_tag bad data types

* Add list to the valid types

* Move tests to the opentracing area

Author: pglombardo

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, 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, list 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.

tests/test_logging.py renamed to tests/clients/test_logging.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)
+