Do not record non-sampled spans (#109)

Fixes #108

Author: ocelotl

CHANGELOG.md

@@ -1,5 +1,9 @@
 CHANGELOG.md
 
+<a name="4.4.8"></a>
+## [4.4.8](https://github.com/lightstep/lightstep-tracer-python/compare/4.4.7...4.4.8)
+* Do not record non-sampled spans (#108)
+
 <a name="4.4.7"></a>
 ## [4.4.7](https://github.com/lightstep/lightstep-tracer-python/compare/4.4.6...4.4.7)
 * Cast all carrier values to string (#106)

lightstep/http_connection.py

@@ -30,9 +30,11 @@ def report(self, *args, **kwargs):
         with self._lock:
             try:
                 report.auth.access_token = auth.access_token
-                headers = {"Content-Type": "application/octet-stream",
-                           "Accept": "application/octet-stream",
-                           "Lightstep-Access-Token": auth.access_token}
+                headers = {
+                    "Content-Type": "application/octet-stream",
+                    "Accept": "application/octet-stream",
+                    "Lightstep-Access-Token": auth.access_token
+                }
 
                 r = requests.post(
                     self._collector_url,

lightstep/recorder.py

@@ -71,18 +71,19 @@ def __init__(self,
         self._finest("Initialized with Tracer runtime: {0}", (self._runtime,))
         secure = collector_encryption != 'none'  # the default is 'tls'
         self._collector_url = util._collector_url_from_hostport(
-                secure,
-                collector_host,
-                collector_port,
-                self.use_thrift)
+            secure,
+            collector_host,
+            collector_port,
+            self.use_thrift
+        )
         self._timeout_seconds = timeout_seconds
         self._auth = self.converter.create_auth(access_token)
         self._mutex = threading.Lock()
         self._span_records = []
         self._max_span_records = max_span_records
 
         self._disabled_runtime = False
-        
+
         atexit.register(self.shutdown)
 
         self._periodic_flush_seconds = periodic_flush_seconds
@@ -99,7 +100,7 @@ def __init__(self,
     def _maybe_init_flush_thread(self):
         """Start a periodic flush mechanism for this recorder if:
 
-        1. periodic_flush_seconds > 0, and 
+        1. periodic_flush_seconds > 0, and
         2. self._flush_thread is None, indicating that we have not yet
            initialized the background flush thread.
 
@@ -132,7 +133,7 @@ def record_span(self, span):
 
         Will drop a previously-added span if the limit has been reached.
         """
-        if self._disabled_runtime:
+        if self._disabled_runtime or not span.context.sampled:
             return
 
         # Lazy-init the flush loop (if need be).
@@ -241,7 +242,7 @@ def _flush_periodically(self):
     def _flush_worker(self, connection):
         """Use the given connection to transmit the current logs and spans as a
         report request."""
-        if connection == None:
+        if connection is None:
             return False
 
         # If the connection is not ready, try reestablishing it. If that
@@ -268,8 +269,9 @@ def _flush_worker(self, connection):
 
         except Exception as e:
             self._fine(
-                    "Caught exception during report: {0}, stack trace: {1}",
-                    (e, traceback.format_exc()))
+                "Caught exception during report: {0}, stack trace: {1}",
+                (e, traceback.format_exc())
+            )
             self._restore_spans(report_request)
             return False
 

tests/recorder_test.py

@@ -55,6 +55,39 @@ def recorder(request):
     yield lightstep.recorder.Recorder(**runtime_args)
 
 
+def test_non_sampled_span_thrift(recorder):
+
+    mock_connection = MockConnection()
+    mock_connection.open()
+
+    non_sampled_span = BasicSpan(
+        lightstep.tracer._LightstepTracer(False, recorder, None),
+        operation_name="non_sampled",
+        context=SpanContext(trace_id=1, span_id=1, sampled=False),
+        start_time=time.time(),
+    )
+    non_sampled_span.finish()
+
+    sampled_span = BasicSpan(
+        lightstep.tracer._LightstepTracer(False, recorder, None),
+        operation_name="sampled",
+        context=SpanContext(trace_id=1, span_id=2, sampled=True),
+        start_time=time.time(),
+    )
+    sampled_span.finish()
+    recorder.record_span(non_sampled_span)
+    recorder.record_span(sampled_span)
+
+    recorder.flush(mock_connection)
+
+    if recorder.use_thrift:
+        for span_record in mock_connection.reports[0].span_records:
+            assert span_record.span_name == "sampled"
+    else:
+        for span in mock_connection.reports[0].spans:
+            assert span.operation_name == "sampled"
+
+
 def test_default_tags_set_correctly(recorder):
     mock_connection = MockConnection()
     mock_connection.open()
@@ -80,7 +113,7 @@ def test_default_tags_set_correctly(recorder):
         "access_token": "{your_access_token}",
         "component_name": "python/runtime_test",
         "periodic_flush_seconds": 0,
-        "tags": {"lightstep.hostname": "hostname",},
+        "tags": {"lightstep.hostname": "hostname"},
     }
     new_recorder = lightstep.recorder.Recorder(**runtime_args)
     for tag in new_recorder._runtime.tags:
@@ -119,7 +152,7 @@ def test_shutdown_twice(recorder):
         recorder.shutdown()
         recorder.shutdown()
     except Exception as error:
-        self.fail("Unexpected exception raised: {}".format(error))
+        pytest.fail("Unexpected exception raised: {}".format(error))
 
 
 # ------------
@@ -225,7 +258,7 @@ def test_exception_formatting(recorder):
     assert len(recorder._span_records) == 1
     assert recorder.flush(mock_connection)
     spans = recorder.converter.get_span_records(mock_connection.reports[1])
-    
+
     if hasattr(spans[0], "log_records"):
         assert len(spans[0].log_records) == 1
         assert len(spans[0].log_records[0].fields) == 3
@@ -251,4 +284,3 @@ def test_exception_formatting(recorder):
                 assert field.string_value == ""
             else:
                 raise AttributeError("unexpected field: %s".format(field.key))
-