From 7f139210467b9f46ba903d6b1eab1b3611a0d14d Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 21 Apr 2020 09:53:57 -0700 Subject: [PATCH 1/6] adding lightstep.hostname tag --- lightstep/http_converter.py | 5 ++++- lightstep/thrift_converter.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lightstep/http_converter.py b/lightstep/http_converter.py index c17573d..fd9a910 100644 --- a/lightstep/http_converter.py +++ b/lightstep/http_converter.py @@ -1,8 +1,10 @@ +import os +import sys + from lightstep.collector_pb2 import Auth, ReportRequest, Span, Reporter, KeyValue, Reference, SpanContext from lightstep.converter import Converter from . import util from . import version as tracer_version -import sys from google.protobuf.timestamp_pb2 import Timestamp @@ -29,6 +31,7 @@ def create_runtime(self, component_name, tags, guid): 'lightstep.tracer_version': tracer_version.LIGHTSTEP_PYTHON_TRACER_VERSION, 'lightstep.component_name': component_name, 'lightstep.guid': util._id_to_hex(guid), + 'lightstep.hostname': os.uname()[1], }) # Convert tracer_tags to a list of KeyValue pairs. diff --git a/lightstep/thrift_converter.py b/lightstep/thrift_converter.py index ffcf314..57694df 100644 --- a/lightstep/thrift_converter.py +++ b/lightstep/thrift_converter.py @@ -1,7 +1,9 @@ +import os +import sys + from lightstep import constants from lightstep.converter import Converter from .crouton import ttypes -import sys from . import util from . import version as tracer_version import jsonpickle @@ -29,6 +31,7 @@ def create_runtime(self, component_name, tags, guid): 'lightstep.tracer_version': tracer_version.LIGHTSTEP_PYTHON_TRACER_VERSION, 'lightstep.component_name': component_name, 'lightstep.guid': util._id_to_hex(guid), + 'lightstep.hostname': os.uname()[1], }) # Convert tracer_tags to a list of KeyValue pairs. From eaae4d5340dab5afde84ded7aae237717b52944b Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 21 Apr 2020 12:59:58 -0700 Subject: [PATCH 2/6] re-enabling recorder tests --- tests/recorder_test.py | 211 +++++++++++++++++++---------------------- 1 file changed, 96 insertions(+), 115 deletions(-) diff --git a/tests/recorder_test.py b/tests/recorder_test.py index c514aba..4b3fd7d 100644 --- a/tests/recorder_test.py +++ b/tests/recorder_test.py @@ -15,6 +15,7 @@ class MockConnection(object): """MockConnection is used to debug and test Runtime. """ + def __init__(self): self.reports = [] self.ready = False @@ -40,121 +41,101 @@ def clear(self): @pytest.fixture(params=[True, False]) def recorder(request): - runtime_args = {'collector_encryption': 'none', - 'collector_host': 'localhost', - 'collector_port': 9998, - 'access_token': '{your_access_token}', - 'component_name': 'python/runtime_test', - 'periodic_flush_seconds': 0, - 'use_thrift': request.param, - 'use_http': not request.param} - recorder = lightstep.recorder.Recorder(runtime_args) - yield recorder - - -# ------_ -# HELPERS -# ------_ + runtime_args = { + "collector_encryption": "none", + "collector_host": "localhost", + "collector_port": 9998, + "access_token": "{your_access_token}", + "component_name": "python/runtime_test", + "periodic_flush_seconds": 0, + "use_thrift": request.param, + "use_http": not request.param, + } + yield lightstep.recorder.Recorder(runtime_args) + + +# -------------- +# SHUTDOWN TESTS +# -------------- +def test_send_spans_after_shutdown(recorder): + mock_connection = MockConnection() + mock_connection.open() + # Send 10 spans + for i in range(10): + dummy_basic_span(recorder, i) + assert recorder.flush(mock_connection) + + # Check 10 spans + check_spans(recorder.converter, mock_connection.reports[0]) + + # Delete current logs and shutdown runtime + mock_connection.clear() + recorder.shutdown() + + # Send 10 spans, though none should get through + for i in range(10): + recorder.record_span(dummy_basic_span(recorder, i)) + assert not recorder.flush(mock_connection) + assert len(mock_connection.reports) == 0 + + +def test_shutdown_twice(recorder): + recorder.shutdown() + recorder.shutdown() + + +# ------------ +# STRESS TESTS +# ------------ +def test_stress_logs(recorder): + mock_connection = MockConnection() + mock_connection.open() + for i in range(1000): + dummy_basic_span(recorder, i) + assert recorder.flush(mock_connection) + assert recorder.converter.num_span_records(mock_connection.reports[0]) == 1000 + check_spans(recorder.converter, mock_connection.reports[0]) + + +def test_stress_spans(recorder): + mock_connection = MockConnection() + mock_connection.open() + for i in range(1000): + dummy_basic_span(recorder, i) + assert recorder.flush(mock_connection) + assert recorder.converter.num_span_records(mock_connection.reports[0]) == 1000 + check_spans(recorder.converter, mock_connection.reports[0]) + + +# ------------- +# RUNTIME TESTS +# ------------- +def test_buffer_limits(recorder): + mock_connection = MockConnection() + mock_connection.open() + recorder._max_span_records = 88 + + assert len(recorder._span_records) == 0 + for i in range(0, 100): + dummy_basic_span(recorder, i) + assert len(recorder._span_records) == 88 + assert recorder.flush(mock_connection) + + def check_spans(converter, report): """Checks spans' name. """ - def setUp(self): - self.mock_connection = MockConnection() - self.mock_connection.open() - self.runtime_args = {'collector_encryption': 'none', - 'collector_host': 'localhost', - 'collector_port': 9998, - 'access_token': '{your_access_token}', - 'component_name': 'python/runtime_test', - 'periodic_flush_seconds': 0} - - def create_test_recorder(self): - """Returns a LightStep Recorder based on self.runtime_args. - """ - return lightstep.recorder.Recorder(**self.runtime_args) - - # ------------- - # SHUTDOWN TESTS - # ------------- - def test_send_spans_after_shutdown(self): - recorder = self.create_test_recorder() - - # Send 10 spans - for i in range(10): - recorder.record_span(self.dummy_basic_span(recorder, i)) - self.assertTrue(recorder.flush(self.mock_connection)) - - # Check 10 spans - self.check_spans(self.mock_connection.reports[0].span_records) - - # Delete current logs and shutdown runtime - self.mock_connection.clear() - recorder.shutdown() - - # Send 10 spans, though none should get through - for i in range(10): - recorder.record_span(self.dummy_basic_span(recorder, i)) - self.assertFalse(recorder.flush(self.mock_connection)) - self.assertEqual(len(self.mock_connection.reports), 0) - - def test_shutdown_twice(self): - recorder = self.create_test_recorder() - recorder.shutdown() - recorder.shutdown() - - # ------------ - # STRESS TESTS - # ------------ - def test_stress_logs(self): - recorder = self.create_test_recorder() - for i in range(1000): - recorder.record_span(self.dummy_basic_span(recorder, i)) - self.assertTrue(recorder.flush(self.mock_connection)) - self.assertEqual(len(self.mock_connection.reports[0].span_records), 1000) - self.check_spans(self.mock_connection.reports[0].span_records) - - def test_stress_spans(self): - recorder = self.create_test_recorder() - for i in range(1000): - recorder.record_span(self.dummy_basic_span(recorder, i)) - self.assertTrue(recorder.flush(self.mock_connection)) - self.assertEqual(len(self.mock_connection.reports[0].span_records), 1000) - self.check_spans(self.mock_connection.reports[0].span_records) - - # ------------- - # RUNTIME TESTS - # ------------- - - def test_buffer_limits(self): - self.runtime_args.update({ - 'max_span_records': 88, - }) - recorder = self.create_test_recorder() - - self.assertEqual(len(recorder._span_records), 0) - for i in range(0, 10000): - recorder.record_span(self.dummy_basic_span(recorder, i)) - self.assertEqual(len(recorder._span_records), 88) - self.assertTrue(recorder.flush(self.mock_connection)) - - # ------ - # HELPER - # ------ - def check_spans(self, spans): - """Checks spans' name. - """ - for i, span in enumerate(spans): - self.assertEqual(span.span_name, str(i)) - - def dummy_basic_span(self, recorder, i): - return BasicSpan( - lightstep.tracer._LightstepTracer(False, recorder, None), - operation_name=str(i), - context=SpanContext( - trace_id=1000+i, - span_id=2000+i), - start_time=time.time()) - - -if __name__ == '__main__': - unittest.main() + spans = converter.get_span_records(report) + for i, span in enumerate(spans): + assert converter.get_span_name(span) == str(i) + + +def dummy_basic_span(recorder, i): + span = BasicSpan( + lightstep.tracer._LightstepTracer(False, recorder, None), + operation_name=str(i), + context=SpanContext(trace_id=1000 + i, span_id=2000 + i), + start_time=time.time() - 100, + ) + span.finish() + return span From 94ec3bda9d368c950e9266baea5d78c5693652b0 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 21 Apr 2020 13:25:25 -0700 Subject: [PATCH 3/6] dont override the tag if present --- lightstep/http_converter.py | 2 +- lightstep/thrift_converter.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lightstep/http_converter.py b/lightstep/http_converter.py index fd9a910..1299fe7 100644 --- a/lightstep/http_converter.py +++ b/lightstep/http_converter.py @@ -24,6 +24,7 @@ def create_runtime(self, component_name, tags, guid): if tags is None: tags = {} tracer_tags = tags.copy() + tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname()[1]) tracer_tags.update({ 'lightstep.tracer_platform': 'python', @@ -31,7 +32,6 @@ def create_runtime(self, component_name, tags, guid): 'lightstep.tracer_version': tracer_version.LIGHTSTEP_PYTHON_TRACER_VERSION, 'lightstep.component_name': component_name, 'lightstep.guid': util._id_to_hex(guid), - 'lightstep.hostname': os.uname()[1], }) # Convert tracer_tags to a list of KeyValue pairs. diff --git a/lightstep/thrift_converter.py b/lightstep/thrift_converter.py index 57694df..c577228 100644 --- a/lightstep/thrift_converter.py +++ b/lightstep/thrift_converter.py @@ -24,6 +24,7 @@ def create_runtime(self, component_name, tags, guid): if tags is None: tags = {} tracer_tags = tags.copy() + tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname()[1]) tracer_tags.update({ 'lightstep.tracer_platform': 'python', @@ -31,7 +32,6 @@ def create_runtime(self, component_name, tags, guid): 'lightstep.tracer_version': tracer_version.LIGHTSTEP_PYTHON_TRACER_VERSION, 'lightstep.component_name': component_name, 'lightstep.guid': util._id_to_hex(guid), - 'lightstep.hostname': os.uname()[1], }) # Convert tracer_tags to a list of KeyValue pairs. From 434a11b4ec652eefa4c643ab1c1370317b379f33 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 21 Apr 2020 14:01:35 -0700 Subject: [PATCH 4/6] adding a test to ensure lightstep.hostname is set and overridable --- tests/recorder_test.py | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/recorder_test.py b/tests/recorder_test.py index 4b3fd7d..f25f87a 100644 --- a/tests/recorder_test.py +++ b/tests/recorder_test.py @@ -1,3 +1,4 @@ +import os import time import unittest @@ -51,7 +52,44 @@ def recorder(request): "use_thrift": request.param, "use_http": not request.param, } - yield lightstep.recorder.Recorder(runtime_args) + yield lightstep.recorder.Recorder(**runtime_args) + + +def test_default_tags_set_correctly(recorder): + mock_connection = MockConnection() + mock_connection.open() + if hasattr(recorder._runtime, "tags"): + tags = recorder._runtime.tags + else: + tags = recorder._runtime.attrs + for tag in tags: + if hasattr(tag, "key"): + if tag.key == "lightstep.hostname": + assert tag.string_value == os.uname()[1] + if tag.key == "lightstep.tracer_platform": + assert tag.string_value == "python" + else: + if tag.Key == "lightstep.hostname": + assert tag.Value == os.uname()[1] + if tag.Key == "lightstep.tracer_platform": + assert tag.Value == "python" + assert len(tags) == 6 + runtime_args = { + "collector_encryption": "none", + "collector_host": "localhost", + "collector_port": 9998, + "access_token": "{your_access_token}", + "component_name": "python/runtime_test", + "periodic_flush_seconds": 0, + "tags": { + "lightstep.hostname": "hostname", + }, + } + new_recorder = lightstep.recorder.Recorder(**runtime_args) + for tag in new_recorder._runtime.tags: + if tag.key == "lightstep.hostname": + assert tag.string_value == "hostname" + assert len(new_recorder._runtime.tags) == 6 # -------------- From 95cbae5ee8ace92051caf7c2fd1c507835577484 Mon Sep 17 00:00:00 2001 From: alrex Date: Wed, 26 Aug 2020 17:20:15 -0500 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Diego Hurtado --- lightstep/http_converter.py | 2 +- lightstep/thrift_converter.py | 2 +- tests/recorder_test.py | 20 ++++++++++---------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lightstep/http_converter.py b/lightstep/http_converter.py index 1299fe7..8292163 100644 --- a/lightstep/http_converter.py +++ b/lightstep/http_converter.py @@ -24,7 +24,7 @@ def create_runtime(self, component_name, tags, guid): if tags is None: tags = {} tracer_tags = tags.copy() - tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname()[1]) + tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname().nodename) tracer_tags.update({ 'lightstep.tracer_platform': 'python', diff --git a/lightstep/thrift_converter.py b/lightstep/thrift_converter.py index c577228..9ff4e4a 100644 --- a/lightstep/thrift_converter.py +++ b/lightstep/thrift_converter.py @@ -24,7 +24,7 @@ def create_runtime(self, component_name, tags, guid): if tags is None: tags = {} tracer_tags = tags.copy() - tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname()[1]) + tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname().nodename) tracer_tags.update({ 'lightstep.tracer_platform': 'python', diff --git a/tests/recorder_test.py b/tests/recorder_test.py index f25f87a..cebc8fe 100644 --- a/tests/recorder_test.py +++ b/tests/recorder_test.py @@ -58,20 +58,17 @@ def recorder(request): def test_default_tags_set_correctly(recorder): mock_connection = MockConnection() mock_connection.open() - if hasattr(recorder._runtime, "tags"): - tags = recorder._runtime.tags - else: - tags = recorder._runtime.attrs + tags = getattr(recorder._runtime, "tags", recorder._runtime.attrs) for tag in tags: if hasattr(tag, "key"): if tag.key == "lightstep.hostname": - assert tag.string_value == os.uname()[1] - if tag.key == "lightstep.tracer_platform": + assert tag.string_value == os.uname().nodename + elif tag.key == "lightstep.tracer_platform": assert tag.string_value == "python" else: if tag.Key == "lightstep.hostname": - assert tag.Value == os.uname()[1] - if tag.Key == "lightstep.tracer_platform": + assert tag.Value == os.uname().nodename + elif tag.Key == "lightstep.tracer_platform": assert tag.Value == "python" assert len(tags) == 6 runtime_args = { @@ -118,8 +115,11 @@ def test_send_spans_after_shutdown(recorder): def test_shutdown_twice(recorder): - recorder.shutdown() - recorder.shutdown() + try: + recorder.shutdown() + recorder.shutdown() + except Exception as error: + self.fail("Unexpected exception raised: {}".format(error)) # ------------ From 036799987563a2cd1a397ae7609b8c562ac9c81c Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 27 Aug 2020 08:24:43 -0700 Subject: [PATCH 6/6] fix build --- lightstep/http_converter.py | 4 ++-- lightstep/thrift_converter.py | 4 ++-- tests/recorder_test.py | 10 ++++++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lightstep/http_converter.py b/lightstep/http_converter.py index 8292163..a05c582 100644 --- a/lightstep/http_converter.py +++ b/lightstep/http_converter.py @@ -1,4 +1,4 @@ -import os +import socket import sys from lightstep.collector_pb2 import Auth, ReportRequest, Span, Reporter, KeyValue, Reference, SpanContext @@ -24,7 +24,7 @@ def create_runtime(self, component_name, tags, guid): if tags is None: tags = {} tracer_tags = tags.copy() - tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname().nodename) + tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', socket.gethostname()) tracer_tags.update({ 'lightstep.tracer_platform': 'python', diff --git a/lightstep/thrift_converter.py b/lightstep/thrift_converter.py index 9ff4e4a..72debc5 100644 --- a/lightstep/thrift_converter.py +++ b/lightstep/thrift_converter.py @@ -1,4 +1,4 @@ -import os +import socket import sys from lightstep import constants @@ -24,7 +24,7 @@ def create_runtime(self, component_name, tags, guid): if tags is None: tags = {} tracer_tags = tags.copy() - tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', os.uname().nodename) + tracer_tags['lightstep.hostname'] = tracer_tags.get('lightstep.hostname', socket.gethostname()) tracer_tags.update({ 'lightstep.tracer_platform': 'python', diff --git a/tests/recorder_test.py b/tests/recorder_test.py index cebc8fe..b8d4163 100644 --- a/tests/recorder_test.py +++ b/tests/recorder_test.py @@ -1,4 +1,4 @@ -import os +import socket import time import unittest @@ -58,16 +58,18 @@ def recorder(request): def test_default_tags_set_correctly(recorder): mock_connection = MockConnection() mock_connection.open() - tags = getattr(recorder._runtime, "tags", recorder._runtime.attrs) + tags = getattr(recorder._runtime, "tags", None) + if tags is None: + tags = getattr(recorder._runtime, "attrs") for tag in tags: if hasattr(tag, "key"): if tag.key == "lightstep.hostname": - assert tag.string_value == os.uname().nodename + assert tag.string_value == socket.gethostname() elif tag.key == "lightstep.tracer_platform": assert tag.string_value == "python" else: if tag.Key == "lightstep.hostname": - assert tag.Value == os.uname().nodename + assert tag.Value == socket.gethostname() elif tag.Key == "lightstep.tracer_platform": assert tag.Value == "python" assert len(tags) == 6