From c673fae1a3e00c8382dd71b82bac1e1df2eb2111 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Tue, 30 Apr 2024 19:56:02 +0000 Subject: [PATCH 01/13] feat: OpenTelemetry trace/spanID integration for Python handlers --- google/cloud/logging_v2/handlers/_helpers.py | 50 ++++++- google/cloud/logging_v2/handlers/handlers.py | 4 +- owlbot.py | 1 + setup.py | 1 + tests/system/test_system.py | 22 +++ tests/unit/handlers/test__helpers.py | 143 +++++++++++++++++++ 6 files changed, 218 insertions(+), 3 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index f0c301ceb..c20af1340 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -24,6 +24,8 @@ except ImportError: # pragma: NO COVER flask = None +import opentelemetry.trace + from google.cloud.logging_v2.handlers.middleware.request import _get_django_request _DJANGO_CONTENT_LENGTH = "CONTENT_LENGTH" @@ -191,9 +193,31 @@ def _parse_xcloud_trace(header): return trace_id, span_id, trace_sampled +def _parse_current_open_telemetry_span(): + """Helper to extract trace, span ID, and trace sampled information from the current + OpenTelemetry span. + + Returns: + Tuple[Optional[str], Optional[str], bool]: + Data related to the current trace_id, span_id, and trace_sampled for the + current OpenTelemetry span. If a span is not found, return None/False for all + fields. + """ + span = opentelemetry.trace.get_current_span() + if span != opentelemetry.trace.span.INVALID_SPAN: + context = span.get_span_context() + trace_id = opentelemetry.trace.format_trace_id(context.trace_id) + span_id = opentelemetry.trace.format_span_id(context.span_id) + trace_sampled = context.trace_flags.sampled + + return trace_id, span_id, trace_sampled + + return None, None, False + + def get_request_data(): """Helper to get http_request and trace data from supported web - frameworks (currently supported: Flask and Django). + frameworks (currently supported: Flask and Django) Returns: Tuple[Optional[dict], Optional[str], Optional[str], bool]: @@ -211,3 +235,27 @@ def get_request_data(): return http_request, trace_id, span_id, trace_sampled return None, None, None, False + +def get_request_and_trace_data(): + """Helper to get http_request and trace data from supported web + frameworks (currently supported: Flask and Django), as well as OpenTelemetry. Attempts + to parse trace/spanID from OpenTelemetry first, before going to Traceparent then XCTC. + HTTP request data is taken from a supporting web framework (currently Flask or Django). + Because HTTP request data is decoupled from OpenTelemetry, it is possible to get as a + return value the HTTP request from the web framework of choice, and trace/span data from + OpenTelemetry, even if trace data is present in the HTTP request headers. + + Returns: + Tuple[Optional[dict], Optional[str], Optional[str], bool]: + Data related to the current http request, trace_id, span_id, and trace_sampled + for the request. All fields will be None if a http request isn't found. + """ + + otel_trace_id, otel_span_id, otel_trace_sampled = _parse_current_open_telemetry_span() + http_request, request_trace_id, request_span_id, request_trace_sampled = get_request_data() + + # otel_trace_id existing means the other return values are non-null + if otel_trace_id: + return http_request, otel_trace_id, otel_span_id, otel_trace_sampled + else: + return http_request, request_trace_id, request_span_id, request_trace_sampled diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index 3d6ab9d1e..5441eca72 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -20,7 +20,7 @@ from google.cloud.logging_v2.handlers.transports import BackgroundThreadTransport from google.cloud.logging_v2.handlers._monitored_resources import detect_resource -from google.cloud.logging_v2.handlers._helpers import get_request_data +from google.cloud.logging_v2.handlers._helpers import get_request_and_trace_data DEFAULT_LOGGER_NAME = "python" @@ -91,7 +91,7 @@ def filter(self, record): inferred_trace, inferred_span, inferred_sampled, - ) = get_request_data() + ) = get_request_and_trace_data() if inferred_trace is not None and self.project is not None: # add full path for detected trace inferred_trace = f"projects/{self.project}/traces/{inferred_trace}" diff --git a/owlbot.py b/owlbot.py index 8666de9e0..f1a5b697e 100644 --- a/owlbot.py +++ b/owlbot.py @@ -93,6 +93,7 @@ def place_before(path, text, *before_text, escape=None): "google-cloud-pubsub", "google-cloud-storage", "google-cloud-testutils", + "opentelemetry-sdk" ], unit_test_external_dependencies=["flask", "webob", "django"], samples=True, diff --git a/setup.py b/setup.py index db7b392d5..9d6b8e2b6 100644 --- a/setup.py +++ b/setup.py @@ -44,6 +44,7 @@ "google-cloud-audit-log >= 0.1.0, < 1.0.0dev", "google-cloud-core >= 2.0.0, <3.0.0dev", "grpc-google-iam-v1 >=0.12.4, <1.0.0dev", + "opentelemetry-api >= 1.22.0", "proto-plus >= 1.22.0, <2.0.0dev", "proto-plus >= 1.22.2, <2.0.0dev; python_version>='3.11'", "protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5", diff --git a/tests/system/test_system.py b/tests/system/test_system.py index c5000f146..766675a28 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -40,6 +40,14 @@ from google.protobuf.struct_pb2 import Struct, Value, ListValue, NullValue +from opentelemetry import trace +from opentelemetry.sdk.trace import TracerProvider +from opentelemetry.sdk.trace.export import ( + BatchSpanProcessor, + ConsoleSpanExporter, +) + + from test_utils.retry import RetryErrors from test_utils.retry import RetryResult from test_utils.system import unique_resource_id @@ -662,6 +670,20 @@ def test_log_root_handler(self): self.assertEqual(len(entries), 1) self.assertEqual(entries[0].payload, expected_payload) + def test_log_handler_otel_integration(self): + # Set up OTel SDK + provider = TracerProvider() + processor = BatchSpanProcessor(ConsoleSpanExporter()) + provider.add_span_processor(processor) + + tracer = trace.get_tracer("test_system", tracer_provider=provider) + with tracer.start_as_current_span("test-span") as span: + context = span.get_span_context() + expected_trace_id = trace.format_trace_id(context.trace_id) + expected_span_id = trace.format_span_id(context.span_id) + + + def test_create_metric(self): METRIC_NAME = "test-create-metric%s" % (_RESOURCE_ID,) metric = Config.CLIENT.metric( diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 5eeae4ba4..23b684eb7 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -14,8 +14,14 @@ import unittest +import contextlib import mock +import opentelemetry.context +import opentelemetry.trace + +from opentelemetry.trace.span import TraceFlags + _FLASK_TRACE_ID = "flask0id" _FLASK_SPAN_ID = "span0flask" _FLASK_HTTP_REQUEST = {"requestUrl": "https://flask.palletsprojects.com/en/1.1.x/"} @@ -23,6 +29,14 @@ _DJANGO_SPAN_ID = "span0django" _DJANGO_HTTP_REQUEST = {"requestUrl": "https://www.djangoproject.com/"} +_OTEL_SPAN_CONTEXT_TRACE_ID = 0x123456789123456789 +_OTEL_SPAN_CONTEXT_SPAN_ID = 0x123456789 +_OTEL_SPAN_CONTEXT_TRACEFLAGS = TraceFlags(TraceFlags.SAMPLED) + +_EXPECTED_OTEL_TRACE_ID = "00000000000000123456789123456789" +_EXPECTED_OTEL_SPAN_ID = "0000000123456789" +_EXPECTED_OTEL_TRACESAMPLED = True + class Test_get_request_data_from_flask(unittest.TestCase): @staticmethod @@ -477,3 +491,132 @@ def test_invalid_headers(self): self.assertIsNone(trace_id) self.assertIsNone(span_id) self.assertEqual(sampled, False) + + +@contextlib.contextmanager +def _setup_otel_span_context(): + """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned + by opentelemetry.trace.get_current_span, and returns it as a contextmanager + """ + span_context = opentelemetry.trace.SpanContext( + _OTEL_SPAN_CONTEXT_TRACE_ID, + _OTEL_SPAN_CONTEXT_SPAN_ID, + False, + trace_flags=_OTEL_SPAN_CONTEXT_TRACEFLAGS, + ) + span = opentelemetry.trace.NonRecordingSpan(span_context) + with mock.patch("opentelemetry.trace.get_current_span", return_value=span) as m: + yield m + + +class Test__parse_open_telemetry_data(unittest.TestCase): + @staticmethod + def _call_fut(): + from google.cloud.logging_v2.handlers import _helpers + + trace, span, sampled = _helpers._parse_current_open_telemetry_span() + return trace, span, sampled + + def test_no_op(self): + trace_id, span_id, sampled = self._call_fut() + self.assertIsNone(trace_id) + self.assertIsNone(span_id) + self.assertEqual(sampled, False) + + def test_span_exists(self): + with _setup_otel_span_context() as _: + trace_id, span_id, sampled = self._call_fut() + self.assertEqual(trace_id, _EXPECTED_OTEL_TRACE_ID) + self.assertEqual(span_id, _EXPECTED_OTEL_SPAN_ID) + self.assertEqual(sampled, _EXPECTED_OTEL_TRACESAMPLED) + + +class Test_get_request_and_trace_data(Test_get_request_data): + @staticmethod + def _call_fut(): + from google.cloud.logging_v2.handlers import _helpers + + http, trace, span, sampled = _helpers.get_request_and_trace_data() + return http, trace, span, sampled + + def test_otel_span_exists_no_request(self): + flask_expected = (None, None, None, False) + django_expected = (None, None, None, False) + + with _setup_otel_span_context() as _: + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, (None, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + + def test_otel_span_exists_django_request(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (None, None, None, False) + + with _setup_otel_span_context() as _: + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, (_DJANGO_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + + def test_otel_span_exists_flask_request(self): + django_expected = (None, None, None, False) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + + with _setup_otel_span_context() as _: + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, (_FLASK_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + + def test_otel_span_exists_both_django_and_flask(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + + with _setup_otel_span_context() as _: + _, _, output = self._helper(django_expected, flask_expected) + + # Django wins + self.assertEqual(output, (_DJANGO_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + + def test_no_otel_span_no_requests(self): + flask_expected = (None, None, None, False) + django_expected = (None, None, None, False) + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, (None, None, None, False)) + + def test_no_otel_span_django_request(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (None, None, None, False) + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, django_expected) + + def test_no_otel_span_flask_request(self): + django_expected = (None, None, None, False) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + _, _, output = self._helper(django_expected, flask_expected) + + # Django wins + self.assertEqual(output, flask_expected) + + def test_no_otel_span_both_django_and_flask(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + _, _, output = self._helper(django_expected, flask_expected) + + # Django wins + self.assertEqual(output, django_expected) \ No newline at end of file From 48c21b65a1a3a172760306802ac5953f774c70fa Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 30 Apr 2024 19:59:29 +0000 Subject: [PATCH 02/13] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- google/cloud/logging_v2/handlers/_helpers.py | 14 +++++- noxfile.py | 1 + tests/system/test_system.py | 2 - tests/unit/handlers/test__helpers.py | 46 +++++++++++++++++--- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index c20af1340..aebadf0ca 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -236,6 +236,7 @@ def get_request_data(): return None, None, None, False + def get_request_and_trace_data(): """Helper to get http_request and trace data from supported web frameworks (currently supported: Flask and Django), as well as OpenTelemetry. Attempts @@ -251,8 +252,17 @@ def get_request_and_trace_data(): for the request. All fields will be None if a http request isn't found. """ - otel_trace_id, otel_span_id, otel_trace_sampled = _parse_current_open_telemetry_span() - http_request, request_trace_id, request_span_id, request_trace_sampled = get_request_data() + ( + otel_trace_id, + otel_span_id, + otel_trace_sampled, + ) = _parse_current_open_telemetry_span() + ( + http_request, + request_trace_id, + request_span_id, + request_trace_sampled, + ) = get_request_data() # otel_trace_id existing means the other return values are non-null if otel_trace_id: diff --git a/noxfile.py b/noxfile.py index 9478ab93c..db5e78a61 100644 --- a/noxfile.py +++ b/noxfile.py @@ -63,6 +63,7 @@ "google-cloud-pubsub", "google-cloud-storage", "google-cloud-testutils", + "opentelemetry-sdk", ] SYSTEM_TEST_LOCAL_DEPENDENCIES: List[str] = [] SYSTEM_TEST_DEPENDENCIES: List[str] = [] diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 766675a28..e447e25e4 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -682,8 +682,6 @@ def test_log_handler_otel_integration(self): expected_trace_id = trace.format_trace_id(context.trace_id) expected_span_id = trace.format_span_id(context.span_id) - - def test_create_metric(self): METRIC_NAME = "test-create-metric%s" % (_RESOURCE_ID,) metric = Config.CLIENT.metric( diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 23b684eb7..d99a2e791 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -495,7 +495,7 @@ def test_invalid_headers(self): @contextlib.contextmanager def _setup_otel_span_context(): - """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned + """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned by opentelemetry.trace.get_current_span, and returns it as a contextmanager """ span_context = opentelemetry.trace.SpanContext( @@ -545,7 +545,15 @@ def test_otel_span_exists_no_request(self): with _setup_otel_span_context() as _: _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (None, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + self.assertEqual( + output, + ( + None, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) def test_otel_span_exists_django_request(self): django_expected = ( @@ -558,7 +566,15 @@ def test_otel_span_exists_django_request(self): with _setup_otel_span_context() as _: _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (_DJANGO_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + self.assertEqual( + output, + ( + _DJANGO_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) def test_otel_span_exists_flask_request(self): django_expected = (None, None, None, False) @@ -566,7 +582,15 @@ def test_otel_span_exists_flask_request(self): with _setup_otel_span_context() as _: _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (_FLASK_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + self.assertEqual( + output, + ( + _FLASK_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) def test_otel_span_exists_both_django_and_flask(self): django_expected = ( @@ -581,8 +605,16 @@ def test_otel_span_exists_both_django_and_flask(self): _, _, output = self._helper(django_expected, flask_expected) # Django wins - self.assertEqual(output, (_DJANGO_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) - + self.assertEqual( + output, + ( + _DJANGO_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) + def test_no_otel_span_no_requests(self): flask_expected = (None, None, None, False) django_expected = (None, None, None, False) @@ -619,4 +651,4 @@ def test_no_otel_span_both_django_and_flask(self): _, _, output = self._helper(django_expected, flask_expected) # Django wins - self.assertEqual(output, django_expected) \ No newline at end of file + self.assertEqual(output, django_expected) From 5869017d8b88055464a190fe0d2e8801a68b82b4 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 30 Apr 2024 20:00:00 +0000 Subject: [PATCH 03/13] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- google/cloud/logging_v2/handlers/_helpers.py | 14 +++++- noxfile.py | 1 + tests/system/test_system.py | 2 - tests/unit/handlers/test__helpers.py | 46 +++++++++++++++++--- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index c20af1340..aebadf0ca 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -236,6 +236,7 @@ def get_request_data(): return None, None, None, False + def get_request_and_trace_data(): """Helper to get http_request and trace data from supported web frameworks (currently supported: Flask and Django), as well as OpenTelemetry. Attempts @@ -251,8 +252,17 @@ def get_request_and_trace_data(): for the request. All fields will be None if a http request isn't found. """ - otel_trace_id, otel_span_id, otel_trace_sampled = _parse_current_open_telemetry_span() - http_request, request_trace_id, request_span_id, request_trace_sampled = get_request_data() + ( + otel_trace_id, + otel_span_id, + otel_trace_sampled, + ) = _parse_current_open_telemetry_span() + ( + http_request, + request_trace_id, + request_span_id, + request_trace_sampled, + ) = get_request_data() # otel_trace_id existing means the other return values are non-null if otel_trace_id: diff --git a/noxfile.py b/noxfile.py index 9478ab93c..db5e78a61 100644 --- a/noxfile.py +++ b/noxfile.py @@ -63,6 +63,7 @@ "google-cloud-pubsub", "google-cloud-storage", "google-cloud-testutils", + "opentelemetry-sdk", ] SYSTEM_TEST_LOCAL_DEPENDENCIES: List[str] = [] SYSTEM_TEST_DEPENDENCIES: List[str] = [] diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 766675a28..e447e25e4 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -682,8 +682,6 @@ def test_log_handler_otel_integration(self): expected_trace_id = trace.format_trace_id(context.trace_id) expected_span_id = trace.format_span_id(context.span_id) - - def test_create_metric(self): METRIC_NAME = "test-create-metric%s" % (_RESOURCE_ID,) metric = Config.CLIENT.metric( diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 23b684eb7..d99a2e791 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -495,7 +495,7 @@ def test_invalid_headers(self): @contextlib.contextmanager def _setup_otel_span_context(): - """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned + """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned by opentelemetry.trace.get_current_span, and returns it as a contextmanager """ span_context = opentelemetry.trace.SpanContext( @@ -545,7 +545,15 @@ def test_otel_span_exists_no_request(self): with _setup_otel_span_context() as _: _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (None, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + self.assertEqual( + output, + ( + None, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) def test_otel_span_exists_django_request(self): django_expected = ( @@ -558,7 +566,15 @@ def test_otel_span_exists_django_request(self): with _setup_otel_span_context() as _: _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (_DJANGO_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + self.assertEqual( + output, + ( + _DJANGO_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) def test_otel_span_exists_flask_request(self): django_expected = (None, None, None, False) @@ -566,7 +582,15 @@ def test_otel_span_exists_flask_request(self): with _setup_otel_span_context() as _: _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (_FLASK_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) + self.assertEqual( + output, + ( + _FLASK_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) def test_otel_span_exists_both_django_and_flask(self): django_expected = ( @@ -581,8 +605,16 @@ def test_otel_span_exists_both_django_and_flask(self): _, _, output = self._helper(django_expected, flask_expected) # Django wins - self.assertEqual(output, (_DJANGO_HTTP_REQUEST, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, _EXPECTED_OTEL_TRACESAMPLED)) - + self.assertEqual( + output, + ( + _DJANGO_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) + def test_no_otel_span_no_requests(self): flask_expected = (None, None, None, False) django_expected = (None, None, None, False) @@ -619,4 +651,4 @@ def test_no_otel_span_both_django_and_flask(self): _, _, output = self._helper(django_expected, flask_expected) # Django wins - self.assertEqual(output, django_expected) \ No newline at end of file + self.assertEqual(output, django_expected) From ba90fd05895d44e1c03160728ca6ca865abedb3f Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Wed, 1 May 2024 18:27:10 +0000 Subject: [PATCH 04/13] Added more tests for OTel Python integration --- tests/system/test_system.py | 25 +++- tests/unit/handlers/__init__.py | 35 ++++++ tests/unit/handlers/test__helpers.py | 45 ++----- tests/unit/handlers/test_handlers.py | 134 +++++++++++++++++++++ tests/unit/handlers/test_structured_log.py | 101 ++++++++++++++++ 5 files changed, 304 insertions(+), 36 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index e447e25e4..a5380f3f1 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -47,7 +47,6 @@ ConsoleSpanExporter, ) - from test_utils.retry import RetryErrors from test_utils.retry import RetryResult from test_utils.system import unique_resource_id @@ -671,6 +670,17 @@ def test_log_root_handler(self): self.assertEqual(entries[0].payload, expected_payload) def test_log_handler_otel_integration(self): + LOG_MESSAGE = "This is a test of OpenTelemetry" + LOGGER_NAME = "otel-integration" + handler_name = self._logger_name(LOGGER_NAME) + + handler = CloudLoggingHandler( + Config.CLIENT, name=handler_name, transport=SyncTransport + ) + # only create the logger to delete, hidden otherwise + logger = Config.CLIENT.logger(handler.name) + self.to_delete.append(logger) + # Set up OTel SDK provider = TracerProvider() processor = BatchSpanProcessor(ConsoleSpanExporter()) @@ -679,9 +689,20 @@ def test_log_handler_otel_integration(self): tracer = trace.get_tracer("test_system", tracer_provider=provider) with tracer.start_as_current_span("test-span") as span: context = span.get_span_context() - expected_trace_id = trace.format_trace_id(context.trace_id) + expected_trace_id = f"projects/{Config.CLIENT.project}/traces/{trace.format_trace_id(context.trace_id)}" expected_span_id = trace.format_span_id(context.span_id) + expected_tracesampled = context.trace_flags.sampled + cloud_logger = logging.getLogger(LOGGER_NAME) + cloud_logger.addHandler(handler) + cloud_logger.warning(LOG_MESSAGE) + + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].trace, expected_trace_id) + self.assertEqual(entries[0].span_id, expected_span_id) + self.assertTrue(entries[0].trace_sampled, expected_tracesampled) + def test_create_metric(self): METRIC_NAME = "test-create-metric%s" % (_RESOURCE_ID,) metric = Config.CLIENT.metric( diff --git a/tests/unit/handlers/__init__.py b/tests/unit/handlers/__init__.py index df379f1e9..b872536c9 100644 --- a/tests/unit/handlers/__init__.py +++ b/tests/unit/handlers/__init__.py @@ -11,3 +11,38 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + + +# Utility functions to setup mock OpenTelemetry spans, needed by multiple test +# suites. + +import contextlib +import mock + +import opentelemetry.context +import opentelemetry.trace + +from opentelemetry.trace.span import TraceFlags + +_OTEL_SPAN_CONTEXT_TRACE_ID = 0x123456789123456789 +_OTEL_SPAN_CONTEXT_SPAN_ID = 0x123456789 +_OTEL_SPAN_CONTEXT_TRACEFLAGS = TraceFlags(TraceFlags.SAMPLED) + +_EXPECTED_OTEL_TRACE_ID = "00000000000000123456789123456789" +_EXPECTED_OTEL_SPAN_ID = "0000000123456789" +_EXPECTED_OTEL_TRACESAMPLED = True + +@contextlib.contextmanager +def _setup_otel_span_context(): + """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned + by opentelemetry.trace.get_current_span, and returns it as a contextmanager + """ + span_context = opentelemetry.trace.SpanContext( + _OTEL_SPAN_CONTEXT_TRACE_ID, + _OTEL_SPAN_CONTEXT_SPAN_ID, + False, + trace_flags=_OTEL_SPAN_CONTEXT_TRACEFLAGS, + ) + span = opentelemetry.trace.NonRecordingSpan(span_context) + with mock.patch("opentelemetry.trace.get_current_span", return_value=span) as m: + yield m \ No newline at end of file diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index d99a2e791..874fb3388 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -14,14 +14,8 @@ import unittest -import contextlib import mock -import opentelemetry.context -import opentelemetry.trace - -from opentelemetry.trace.span import TraceFlags - _FLASK_TRACE_ID = "flask0id" _FLASK_SPAN_ID = "span0flask" _FLASK_HTTP_REQUEST = {"requestUrl": "https://flask.palletsprojects.com/en/1.1.x/"} @@ -29,13 +23,12 @@ _DJANGO_SPAN_ID = "span0django" _DJANGO_HTTP_REQUEST = {"requestUrl": "https://www.djangoproject.com/"} -_OTEL_SPAN_CONTEXT_TRACE_ID = 0x123456789123456789 -_OTEL_SPAN_CONTEXT_SPAN_ID = 0x123456789 -_OTEL_SPAN_CONTEXT_TRACEFLAGS = TraceFlags(TraceFlags.SAMPLED) - -_EXPECTED_OTEL_TRACE_ID = "00000000000000123456789123456789" -_EXPECTED_OTEL_SPAN_ID = "0000000123456789" -_EXPECTED_OTEL_TRACESAMPLED = True +from tests.unit.handlers import ( + _setup_otel_span_context, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED +) class Test_get_request_data_from_flask(unittest.TestCase): @@ -493,22 +486,6 @@ def test_invalid_headers(self): self.assertEqual(sampled, False) -@contextlib.contextmanager -def _setup_otel_span_context(): - """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned - by opentelemetry.trace.get_current_span, and returns it as a contextmanager - """ - span_context = opentelemetry.trace.SpanContext( - _OTEL_SPAN_CONTEXT_TRACE_ID, - _OTEL_SPAN_CONTEXT_SPAN_ID, - False, - trace_flags=_OTEL_SPAN_CONTEXT_TRACEFLAGS, - ) - span = opentelemetry.trace.NonRecordingSpan(span_context) - with mock.patch("opentelemetry.trace.get_current_span", return_value=span) as m: - yield m - - class Test__parse_open_telemetry_data(unittest.TestCase): @staticmethod def _call_fut(): @@ -524,7 +501,7 @@ def test_no_op(self): self.assertEqual(sampled, False) def test_span_exists(self): - with _setup_otel_span_context() as _: + with _setup_otel_span_context(): trace_id, span_id, sampled = self._call_fut() self.assertEqual(trace_id, _EXPECTED_OTEL_TRACE_ID) self.assertEqual(span_id, _EXPECTED_OTEL_SPAN_ID) @@ -543,7 +520,7 @@ def test_otel_span_exists_no_request(self): flask_expected = (None, None, None, False) django_expected = (None, None, None, False) - with _setup_otel_span_context() as _: + with _setup_otel_span_context(): _, _, output = self._helper(django_expected, flask_expected) self.assertEqual( output, @@ -564,7 +541,7 @@ def test_otel_span_exists_django_request(self): ) flask_expected = (None, None, None, False) - with _setup_otel_span_context() as _: + with _setup_otel_span_context(): _, _, output = self._helper(django_expected, flask_expected) self.assertEqual( output, @@ -580,7 +557,7 @@ def test_otel_span_exists_flask_request(self): django_expected = (None, None, None, False) flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) - with _setup_otel_span_context() as _: + with _setup_otel_span_context(): _, _, output = self._helper(django_expected, flask_expected) self.assertEqual( output, @@ -601,7 +578,7 @@ def test_otel_span_exists_both_django_and_flask(self): ) flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) - with _setup_otel_span_context() as _: + with _setup_otel_span_context(): _, _, output = self._helper(django_expected, flask_expected) # Django wins diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index c301327a9..493e639d7 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -28,6 +28,12 @@ _GAE_ENV_VARS, ) +from tests.unit.handlers import ( + _setup_otel_span_context, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED +) class TestCloudLoggingFilter(unittest.TestCase): PROJECT = "PROJECT" @@ -230,6 +236,134 @@ def test_record_with_traceparent_request(self): self.assertEqual(record._http_request, expected_request) self.assertEqual(record._http_request_str, json.dumps(expected_request)) + def test_record_with_opentelemetry_span_no_request(self): + filter_obj = self._make_one() + record = logging.LogRecord( + None, + logging.INFO, + None, + None, + None, + None, + None, + ) + record.created = None + + with _setup_otel_span_context(): + success = filter_obj.filter(record) + self.assertTrue(success) + + self.assertEqual(record._trace, _EXPECTED_OTEL_TRACE_ID) + self.assertEqual(record._trace_str, _EXPECTED_OTEL_TRACE_ID) + self.assertEqual(record._span_id, _EXPECTED_OTEL_SPAN_ID) + self.assertEqual(record._span_id_str, _EXPECTED_OTEL_SPAN_ID) + self.assertTrue(record._trace_sampled) + self.assertEqual(record._trace_sampled_str, "true") + self.assertIsNone(record._http_request) + self.assertEqual(record._http_request_str, "{}") + + def test_record_with_opentelemetry_span_and_request(self): + filter_obj = self._make_one() + record = logging.LogRecord( + None, + logging.INFO, + None, + None, + None, + None, + None, + ) + record.created = None + http_path = "http://testserver/123" + http_agent = "Mozilla/5.0" + http_trace = "123" + http_span = "456" + combined_trace = f"{http_trace}/{http_span};o=1" + expected_request = { + "requestMethod": "GET", + "requestUrl": http_path, + "userAgent": http_agent, + "protocol": "HTTP/1.1", + } + + app = self.create_app() + with app.test_request_context( + http_path, + headers={ + "User-Agent": http_agent, + "X_CLOUD_TRACE_CONTEXT": combined_trace, + }, + ): + with _setup_otel_span_context(): + success = filter_obj.filter(record) + self.assertTrue(success) + + self.assertEqual(record._trace, _EXPECTED_OTEL_TRACE_ID) + self.assertEqual(record._trace_str, _EXPECTED_OTEL_TRACE_ID) + self.assertEqual(record._span_id, _EXPECTED_OTEL_SPAN_ID) + self.assertEqual(record._span_id_str, _EXPECTED_OTEL_SPAN_ID) + self.assertTrue(record._trace_sampled) + self.assertEqual(record._trace_sampled_str, "true") + + self.assertEqual(record._http_request, expected_request) + self.assertEqual(record._http_request_str, json.dumps(expected_request)) + + def test_record_with_opentelemetry_span_and_request_with_overrides(self): + """ + sort of does what the test after this one does, but more in the context of OTel precedence + """ + filter_obj = self._make_one() + record = logging.LogRecord( + None, + logging.INFO, + None, + None, + None, + None, + None, + ) + record.created = None + http_path = "http://testserver/123" + http_agent = "Mozilla/5.0" + http_trace = "123" + http_span = "456" + combined_trace = f"{http_trace}/{http_span};o=1" + expected_request = { + "requestMethod": "GET", + "requestUrl": http_path, + "userAgent": http_agent, + "protocol": "HTTP/1.1", + } + + overwritten_trace = "01234" + overwritten_span = "43210" + overwritten_tracesampled = False + record.trace = overwritten_trace + record.span_id = overwritten_span + record.trace_sampled = overwritten_tracesampled + + app = self.create_app() + with app.test_request_context( + http_path, + headers={ + "User-Agent": http_agent, + "X_CLOUD_TRACE_CONTEXT": combined_trace, + }, + ): + with _setup_otel_span_context(): + success = filter_obj.filter(record) + self.assertTrue(success) + + self.assertEqual(record._trace, overwritten_trace) + self.assertEqual(record._trace_str, overwritten_trace) + self.assertEqual(record._span_id, overwritten_span) + self.assertEqual(record._span_id_str, overwritten_span) + self.assertFalse(record._trace_sampled) + self.assertEqual(record._trace_sampled_str, json.dumps(overwritten_tracesampled)) + + self.assertEqual(record._http_request, expected_request) + self.assertEqual(record._http_request_str, json.dumps(expected_request)) + def test_user_overrides(self): """ ensure user can override fields diff --git a/tests/unit/handlers/test_structured_log.py b/tests/unit/handlers/test_structured_log.py index fc6b7c598..160ec9faf 100644 --- a/tests/unit/handlers/test_structured_log.py +++ b/tests/unit/handlers/test_structured_log.py @@ -512,6 +512,107 @@ def test_format_overrides(self): for key, value in expected_payload.items(): self.assertEqual(value, result[key]) + def test_format_with_opentelemetry_span(self): + import logging + import json + + from tests.unit.handlers import ( + _setup_otel_span_context, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED + ) + + handler = self._make_one() + logname = "loggername" + message = "hello world，嗨 世界" + record = logging.LogRecord(logname, logging.INFO, "", 0, message, None, None) + expected_payload = { + "logging.googleapis.com/trace": _EXPECTED_OTEL_TRACE_ID, + "logging.googleapis.com/spanId": _EXPECTED_OTEL_SPAN_ID, + "logging.googleapis.com/trace_sampled": _EXPECTED_OTEL_TRACESAMPLED + } + + with _setup_otel_span_context(): + handler.filter(record) + result = json.loads(handler.format(record)) + for key, value in expected_payload.items(): + self.assertEqual(value, result[key]) + + def test_format_with_opentelemetry_span_and_request(self): + import logging + import json + + from tests.unit.handlers import ( + _setup_otel_span_context, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED + ) + + handler = self._make_one() + logname = "loggername" + message = "hello world，嗨 世界" + record = logging.LogRecord(logname, logging.INFO, "", 0, message, None, None) + expected_path = "http://testserver/123" + expected_agent = "Mozilla/5.0" + http_trace = "123" + http_span = "456" + trace_header = f"{http_trace}/{http_span};o=1" + expected_payload = { + "logging.googleapis.com/trace": _EXPECTED_OTEL_TRACE_ID, + "logging.googleapis.com/spanId": _EXPECTED_OTEL_SPAN_ID, + "logging.googleapis.com/trace_sampled": _EXPECTED_OTEL_TRACESAMPLED, + "httpRequest": { + "requestMethod": "GET", + "requestUrl": expected_path, + "userAgent": expected_agent, + "protocol": "HTTP/1.1", + }, + } + + app = self.create_app() + with app.test_request_context( + expected_path, + headers={ + "User-Agent": expected_agent, + "X_CLOUD_TRACE_CONTEXT": trace_header, + }, + ): + with _setup_otel_span_context(): + handler.filter(record) + result = json.loads(handler.format(record)) + for key, value in expected_payload.items(): + self.assertEqual(value, result[key]) + + def test_format_with_opentelemetry_span_and_overrides(self): + import logging + import json + + from tests.unit.handlers import _setup_otel_span_context + + handler = self._make_one() + logname = "loggername" + message = "hello world，嗨 世界" + record = logging.LogRecord(logname, logging.INFO, "", 0, message, None, None) + overwrite_trace = "abc" + overwrite_span = "123" + overwrite_tracesampled = False + record.trace = overwrite_trace + record.span_id = overwrite_span + record.trace_sampled = overwrite_tracesampled + expected_payload = { + "logging.googleapis.com/trace": overwrite_trace, + "logging.googleapis.com/spanId": overwrite_span, + "logging.googleapis.com/trace_sampled": overwrite_tracesampled, + } + + with _setup_otel_span_context(): + handler.filter(record) + result = json.loads(handler.format(record)) + for key, value in expected_payload.items(): + self.assertEqual(value, result[key]) + def test_format_with_json_fields(self): """ User can add json_fields to the record, which should populate the payload From 479384874b550f5068ac4cefff30992281d4be31 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Wed, 1 May 2024 18:49:06 +0000 Subject: [PATCH 05/13] linting --- tests/system/test_system.py | 2 +- tests/unit/handlers/__init__.py | 3 ++- tests/unit/handlers/test__helpers.py | 2 +- tests/unit/handlers/test_handlers.py | 7 +++++-- tests/unit/handlers/test_structured_log.py | 6 +++--- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index a5380f3f1..900af2e36 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -702,7 +702,7 @@ def test_log_handler_otel_integration(self): self.assertEqual(entries[0].trace, expected_trace_id) self.assertEqual(entries[0].span_id, expected_span_id) self.assertTrue(entries[0].trace_sampled, expected_tracesampled) - + def test_create_metric(self): METRIC_NAME = "test-create-metric%s" % (_RESOURCE_ID,) metric = Config.CLIENT.metric( diff --git a/tests/unit/handlers/__init__.py b/tests/unit/handlers/__init__.py index b872536c9..74749229d 100644 --- a/tests/unit/handlers/__init__.py +++ b/tests/unit/handlers/__init__.py @@ -32,6 +32,7 @@ _EXPECTED_OTEL_SPAN_ID = "0000000123456789" _EXPECTED_OTEL_TRACESAMPLED = True + @contextlib.contextmanager def _setup_otel_span_context(): """Sets up a nonrecording OpenTelemetry span with a mock span context that gets returned @@ -45,4 +46,4 @@ def _setup_otel_span_context(): ) span = opentelemetry.trace.NonRecordingSpan(span_context) with mock.patch("opentelemetry.trace.get_current_span", return_value=span) as m: - yield m \ No newline at end of file + yield m diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 874fb3388..5f64adca9 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -27,7 +27,7 @@ _setup_otel_span_context, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED + _EXPECTED_OTEL_TRACESAMPLED, ) diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index 493e639d7..f5e3b202a 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -32,9 +32,10 @@ _setup_otel_span_context, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED + _EXPECTED_OTEL_TRACESAMPLED, ) + class TestCloudLoggingFilter(unittest.TestCase): PROJECT = "PROJECT" @@ -359,7 +360,9 @@ def test_record_with_opentelemetry_span_and_request_with_overrides(self): self.assertEqual(record._span_id, overwritten_span) self.assertEqual(record._span_id_str, overwritten_span) self.assertFalse(record._trace_sampled) - self.assertEqual(record._trace_sampled_str, json.dumps(overwritten_tracesampled)) + self.assertEqual( + record._trace_sampled_str, json.dumps(overwritten_tracesampled) + ) self.assertEqual(record._http_request, expected_request) self.assertEqual(record._http_request_str, json.dumps(expected_request)) diff --git a/tests/unit/handlers/test_structured_log.py b/tests/unit/handlers/test_structured_log.py index 160ec9faf..310c3fb4c 100644 --- a/tests/unit/handlers/test_structured_log.py +++ b/tests/unit/handlers/test_structured_log.py @@ -520,7 +520,7 @@ def test_format_with_opentelemetry_span(self): _setup_otel_span_context, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED + _EXPECTED_OTEL_TRACESAMPLED, ) handler = self._make_one() @@ -530,7 +530,7 @@ def test_format_with_opentelemetry_span(self): expected_payload = { "logging.googleapis.com/trace": _EXPECTED_OTEL_TRACE_ID, "logging.googleapis.com/spanId": _EXPECTED_OTEL_SPAN_ID, - "logging.googleapis.com/trace_sampled": _EXPECTED_OTEL_TRACESAMPLED + "logging.googleapis.com/trace_sampled": _EXPECTED_OTEL_TRACESAMPLED, } with _setup_otel_span_context(): @@ -547,7 +547,7 @@ def test_format_with_opentelemetry_span_and_request(self): _setup_otel_span_context, _EXPECTED_OTEL_TRACE_ID, _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED + _EXPECTED_OTEL_TRACESAMPLED, ) handler = self._make_one() From bfc84dccacf26811fa016183f4e5cb7013c5d404 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Wed, 1 May 2024 19:28:16 +0000 Subject: [PATCH 06/13] more linting --- tests/unit/handlers/test__helpers.py | 14 +++++++------- tests/unit/handlers/test_handlers.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 5f64adca9..69c338f87 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -16,13 +16,6 @@ import mock -_FLASK_TRACE_ID = "flask0id" -_FLASK_SPAN_ID = "span0flask" -_FLASK_HTTP_REQUEST = {"requestUrl": "https://flask.palletsprojects.com/en/1.1.x/"} -_DJANGO_TRACE_ID = "django0id" -_DJANGO_SPAN_ID = "span0django" -_DJANGO_HTTP_REQUEST = {"requestUrl": "https://www.djangoproject.com/"} - from tests.unit.handlers import ( _setup_otel_span_context, _EXPECTED_OTEL_TRACE_ID, @@ -30,6 +23,13 @@ _EXPECTED_OTEL_TRACESAMPLED, ) +_FLASK_TRACE_ID = "flask0id" +_FLASK_SPAN_ID = "span0flask" +_FLASK_HTTP_REQUEST = {"requestUrl": "https://flask.palletsprojects.com/en/1.1.x/"} +_DJANGO_TRACE_ID = "django0id" +_DJANGO_SPAN_ID = "span0django" +_DJANGO_HTTP_REQUEST = {"requestUrl": "https://www.djangoproject.com/"} + class Test_get_request_data_from_flask(unittest.TestCase): @staticmethod diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index f5e3b202a..ca2a25de2 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -258,7 +258,7 @@ def test_record_with_opentelemetry_span_no_request(self): self.assertEqual(record._trace_str, _EXPECTED_OTEL_TRACE_ID) self.assertEqual(record._span_id, _EXPECTED_OTEL_SPAN_ID) self.assertEqual(record._span_id_str, _EXPECTED_OTEL_SPAN_ID) - self.assertTrue(record._trace_sampled) + self.assertEqual(record._trace_sampled, _EXPECTED_OTEL_TRACESAMPLED) self.assertEqual(record._trace_sampled_str, "true") self.assertIsNone(record._http_request) self.assertEqual(record._http_request_str, "{}") @@ -303,7 +303,7 @@ def test_record_with_opentelemetry_span_and_request(self): self.assertEqual(record._trace_str, _EXPECTED_OTEL_TRACE_ID) self.assertEqual(record._span_id, _EXPECTED_OTEL_SPAN_ID) self.assertEqual(record._span_id_str, _EXPECTED_OTEL_SPAN_ID) - self.assertTrue(record._trace_sampled) + self.assertEqual(record._trace_sampled, _EXPECTED_OTEL_TRACESAMPLED) self.assertEqual(record._trace_sampled_str, "true") self.assertEqual(record._http_request, expected_request) From 418910894e70fe3da71a6a31274b1cd0cf29c4e5 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Wed, 8 May 2024 19:37:08 +0000 Subject: [PATCH 07/13] renamed _parse_current_open_telemetry_span and fixed otel testcases --- google/cloud/logging_v2/handlers/_helpers.py | 8 ++++---- setup.py | 2 +- tests/system/test_system.py | 8 +------- tests/unit/handlers/__init__.py | 15 +++++++++++---- tests/unit/handlers/test__helpers.py | 2 +- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index aebadf0ca..fc5de7144 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -193,8 +193,8 @@ def _parse_xcloud_trace(header): return trace_id, span_id, trace_sampled -def _parse_current_open_telemetry_span(): - """Helper to extract trace, span ID, and trace sampled information from the current +def _retrieve_current_open_telemetry_span(): + """Helper to retrieve trace, span ID, and trace sampled information from the current OpenTelemetry span. Returns: @@ -240,7 +240,7 @@ def get_request_data(): def get_request_and_trace_data(): """Helper to get http_request and trace data from supported web frameworks (currently supported: Flask and Django), as well as OpenTelemetry. Attempts - to parse trace/spanID from OpenTelemetry first, before going to Traceparent then XCTC. + to retrieve trace/spanID from OpenTelemetry first, before going to Traceparent then XCTC. HTTP request data is taken from a supporting web framework (currently Flask or Django). Because HTTP request data is decoupled from OpenTelemetry, it is possible to get as a return value the HTTP request from the web framework of choice, and trace/span data from @@ -256,7 +256,7 @@ def get_request_and_trace_data(): otel_trace_id, otel_span_id, otel_trace_sampled, - ) = _parse_current_open_telemetry_span() + ) = _retrieve_current_open_telemetry_span() ( http_request, request_trace_id, diff --git a/setup.py b/setup.py index 9d6b8e2b6..5414f1b08 100644 --- a/setup.py +++ b/setup.py @@ -44,7 +44,7 @@ "google-cloud-audit-log >= 0.1.0, < 1.0.0dev", "google-cloud-core >= 2.0.0, <3.0.0dev", "grpc-google-iam-v1 >=0.12.4, <1.0.0dev", - "opentelemetry-api >= 1.22.0", + "opentelemetry-api >= 1.0.0", "proto-plus >= 1.22.0, <2.0.0dev", "proto-plus >= 1.22.2, <2.0.0dev; python_version>='3.11'", "protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5", diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 900af2e36..207a9709c 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -42,10 +42,6 @@ from opentelemetry import trace from opentelemetry.sdk.trace import TracerProvider -from opentelemetry.sdk.trace.export import ( - BatchSpanProcessor, - ConsoleSpanExporter, -) from test_utils.retry import RetryErrors from test_utils.retry import RetryResult @@ -683,10 +679,8 @@ def test_log_handler_otel_integration(self): # Set up OTel SDK provider = TracerProvider() - processor = BatchSpanProcessor(ConsoleSpanExporter()) - provider.add_span_processor(processor) - tracer = trace.get_tracer("test_system", tracer_provider=provider) + tracer = provider.get_tracer("test_system") with tracer.start_as_current_span("test-span") as span: context = span.get_span_context() expected_trace_id = f"projects/{Config.CLIENT.project}/traces/{trace.format_trace_id(context.trace_id)}" diff --git a/tests/unit/handlers/__init__.py b/tests/unit/handlers/__init__.py index 74749229d..98f6a6acd 100644 --- a/tests/unit/handlers/__init__.py +++ b/tests/unit/handlers/__init__.py @@ -17,11 +17,11 @@ # suites. import contextlib -import mock import opentelemetry.context import opentelemetry.trace +from opentelemetry.trace import NonRecordingSpan from opentelemetry.trace.span import TraceFlags _OTEL_SPAN_CONTEXT_TRACE_ID = 0x123456789123456789 @@ -44,6 +44,13 @@ def _setup_otel_span_context(): False, trace_flags=_OTEL_SPAN_CONTEXT_TRACEFLAGS, ) - span = opentelemetry.trace.NonRecordingSpan(span_context) - with mock.patch("opentelemetry.trace.get_current_span", return_value=span) as m: - yield m + ctx = opentelemetry.trace.set_span_in_context(NonRecordingSpan(span_context)) + tracer = opentelemetry.trace.NoOpTracer() + token = opentelemetry.context.attach(ctx) + try: + with tracer.start_as_current_span('test-span', context=ctx) as span: + print("testubg 123") + print(opentelemetry.trace.get_current_span().get_span_context().span_id) + yield + finally: + opentelemetry.context.detach(token) \ No newline at end of file diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 69c338f87..6a8c19564 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -491,7 +491,7 @@ class Test__parse_open_telemetry_data(unittest.TestCase): def _call_fut(): from google.cloud.logging_v2.handlers import _helpers - trace, span, sampled = _helpers._parse_current_open_telemetry_span() + trace, span, sampled = _helpers._retrieve_current_open_telemetry_span() return trace, span, sampled def test_no_op(self): From 64766ed4524fb4e0c53ab8418fc85893a804f41b Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 8 May 2024 19:40:09 +0000 Subject: [PATCH 08/13] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/handlers/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/handlers/__init__.py b/tests/unit/handlers/__init__.py index 98f6a6acd..9da321078 100644 --- a/tests/unit/handlers/__init__.py +++ b/tests/unit/handlers/__init__.py @@ -48,9 +48,9 @@ def _setup_otel_span_context(): tracer = opentelemetry.trace.NoOpTracer() token = opentelemetry.context.attach(ctx) try: - with tracer.start_as_current_span('test-span', context=ctx) as span: + with tracer.start_as_current_span("test-span", context=ctx) as span: print("testubg 123") print(opentelemetry.trace.get_current_span().get_span_context().span_id) yield finally: - opentelemetry.context.detach(token) \ No newline at end of file + opentelemetry.context.detach(token) From 196749d4abb75beb06a69fde59d6c5eb2dec2c0d Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Thu, 9 May 2024 15:30:16 +0000 Subject: [PATCH 09/13] linting + removed print statements --- tests/unit/handlers/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unit/handlers/__init__.py b/tests/unit/handlers/__init__.py index 9da321078..32eba185f 100644 --- a/tests/unit/handlers/__init__.py +++ b/tests/unit/handlers/__init__.py @@ -48,9 +48,7 @@ def _setup_otel_span_context(): tracer = opentelemetry.trace.NoOpTracer() token = opentelemetry.context.attach(ctx) try: - with tracer.start_as_current_span("test-span", context=ctx) as span: - print("testubg 123") - print(opentelemetry.trace.get_current_span().get_span_context().span_id) + with tracer.start_as_current_span("test-span", context=ctx): yield finally: opentelemetry.context.detach(token) From 3dab0c3e43b18be9013dd8ad9cfa2dac6cf22d9d Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Fri, 10 May 2024 20:50:05 +0000 Subject: [PATCH 10/13] added opentelemetry sdk module cleanup to system test --- tests/system/test_system.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 207a9709c..aae3d0d92 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -19,6 +19,7 @@ import numbers import os import pytest +import sys import unittest import uuid @@ -40,9 +41,6 @@ from google.protobuf.struct_pb2 import Struct, Value, ListValue, NullValue -from opentelemetry import trace -from opentelemetry.sdk.trace import TracerProvider - from test_utils.retry import RetryErrors from test_utils.retry import RetryResult from test_utils.system import unique_resource_id @@ -119,6 +117,22 @@ def setUpModule(): Config.use_mtls == "always", reason="Skip the test case for mTLS testing" ) +def _cleanup_otel_sdk_modules(f): + """ + Decorator to delete all references to opentelemetry SDK modules after a + testcase is run. Test case should import opentelemetry SDK modules inside + the function. This is to test situations where the opentelemetry SDK + is not imported at all. + """ + def wrapped(*args, **kwargs): + f(*args, **kwargs) + + # Deleting from sys.modules should be good enough in this use case + for module_name in list(sys.modules.keys()): + if module_name.startswith('opentelemetry.sdk'): + sys.modules.pop(module_name) + + return wrapped class TestLogging(unittest.TestCase): JSON_PAYLOAD = { @@ -665,7 +679,12 @@ def test_log_root_handler(self): self.assertEqual(len(entries), 1) self.assertEqual(entries[0].payload, expected_payload) + @_cleanup_otel_sdk_modules def test_log_handler_otel_integration(self): + # Doing OTel imports here to not taint the other tests with OTel SDK imports + from opentelemetry import trace + from opentelemetry.sdk.trace import TracerProvider + LOG_MESSAGE = "This is a test of OpenTelemetry" LOGGER_NAME = "otel-integration" handler_name = self._logger_name(LOGGER_NAME) From 785c837a520a0c9ba40b6a6a1ea9f06881f9d171 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 10 May 2024 20:52:25 +0000 Subject: [PATCH 11/13] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/system/test_system.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index aae3d0d92..801cab341 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -117,6 +117,7 @@ def setUpModule(): Config.use_mtls == "always", reason="Skip the test case for mTLS testing" ) + def _cleanup_otel_sdk_modules(f): """ Decorator to delete all references to opentelemetry SDK modules after a @@ -124,16 +125,18 @@ def _cleanup_otel_sdk_modules(f): the function. This is to test situations where the opentelemetry SDK is not imported at all. """ + def wrapped(*args, **kwargs): f(*args, **kwargs) # Deleting from sys.modules should be good enough in this use case for module_name in list(sys.modules.keys()): - if module_name.startswith('opentelemetry.sdk'): + if module_name.startswith("opentelemetry.sdk"): sys.modules.pop(module_name) return wrapped + class TestLogging(unittest.TestCase): JSON_PAYLOAD = { "message": "System test: test_log_struct", @@ -684,7 +687,7 @@ def test_log_handler_otel_integration(self): # Doing OTel imports here to not taint the other tests with OTel SDK imports from opentelemetry import trace from opentelemetry.sdk.trace import TracerProvider - + LOG_MESSAGE = "This is a test of OpenTelemetry" LOGGER_NAME = "otel-integration" handler_name = self._logger_name(LOGGER_NAME) From ff22f7ff72e4815900aa14ad520634b6afe7804c Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Thu, 16 May 2024 15:00:42 +0000 Subject: [PATCH 12/13] Refactored get_request_and_trace_data back into get_request_data --- google/cloud/logging_v2/handlers/_helpers.py | 45 ++-- google/cloud/logging_v2/handlers/handlers.py | 4 +- tests/unit/handlers/test__helpers.py | 238 +++++++++---------- 3 files changed, 133 insertions(+), 154 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index fc5de7144..1886f753b 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -216,28 +216,6 @@ def _retrieve_current_open_telemetry_span(): def get_request_data(): - """Helper to get http_request and trace data from supported web - frameworks (currently supported: Flask and Django) - - Returns: - Tuple[Optional[dict], Optional[str], Optional[str], bool]: - Data related to the current http request, trace_id, span_id, and trace_sampled - for the request. All fields will be None if a http request isn't found. - """ - checkers = ( - get_request_data_from_django, - get_request_data_from_flask, - ) - - for checker in checkers: - http_request, trace_id, span_id, trace_sampled = checker() - if http_request is not None: - return http_request, trace_id, span_id, trace_sampled - - return None, None, None, False - - -def get_request_and_trace_data(): """Helper to get http_request and trace data from supported web frameworks (currently supported: Flask and Django), as well as OpenTelemetry. Attempts to retrieve trace/spanID from OpenTelemetry first, before going to Traceparent then XCTC. @@ -257,15 +235,24 @@ def get_request_and_trace_data(): otel_span_id, otel_trace_sampled, ) = _retrieve_current_open_telemetry_span() - ( - http_request, - request_trace_id, - request_span_id, - request_trace_sampled, - ) = get_request_data() + + # Get HTTP request data + checkers = ( + get_request_data_from_django, + get_request_data_from_flask, + ) + + http_request, http_trace_id, http_span_id, http_trace_sampled = None, None, None, False + + for checker in checkers: + http_request, http_trace_id, http_span_id, http_trace_sampled = checker() + if http_request is None: + http_trace_id, http_span_id, http_trace_sampled = None, None, False + else: + break # otel_trace_id existing means the other return values are non-null if otel_trace_id: return http_request, otel_trace_id, otel_span_id, otel_trace_sampled else: - return http_request, request_trace_id, request_span_id, request_trace_sampled + return http_request, http_trace_id, http_span_id, http_trace_sampled diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index ba4761012..06e131442 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -23,7 +23,7 @@ detect_resource, add_resource_labels, ) -from google.cloud.logging_v2.handlers._helpers import get_request_and_trace_data +from google.cloud.logging_v2.handlers._helpers import get_request_data DEFAULT_LOGGER_NAME = "python" @@ -88,7 +88,7 @@ def filter(self, record): inferred_trace, inferred_span, inferred_sampled, - ) = get_request_and_trace_data() + ) = get_request_data() if inferred_trace is not None and self.project is not None: # add full path for detected trace inferred_trace = f"projects/{self.project}/traces/{inferred_trace}" diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 6a8c19564..850902056 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -362,6 +362,121 @@ def test_missing_both(self): def test_wo_libraries(self): output = self._call_fut() self.assertEqual(output, (None, None, None, False)) + + def test_otel_span_exists_no_request(self): + flask_expected = (None, None, None, False) + django_expected = (None, None, None, False) + + with _setup_otel_span_context(): + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual( + output, + ( + None, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) + + def test_otel_span_exists_django_request(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (None, None, None, False) + + with _setup_otel_span_context(): + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual( + output, + ( + _DJANGO_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) + + def test_otel_span_exists_flask_request(self): + django_expected = (None, None, None, False) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + + with _setup_otel_span_context(): + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual( + output, + ( + _FLASK_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) + + def test_otel_span_exists_both_django_and_flask(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + + with _setup_otel_span_context(): + _, _, output = self._helper(django_expected, flask_expected) + + # Django wins + self.assertEqual( + output, + ( + _DJANGO_HTTP_REQUEST, + _EXPECTED_OTEL_TRACE_ID, + _EXPECTED_OTEL_SPAN_ID, + _EXPECTED_OTEL_TRACESAMPLED, + ), + ) + + def test_no_otel_span_no_requests(self): + flask_expected = (None, None, None, False) + django_expected = (None, None, None, False) + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, (None, None, None, False)) + + def test_no_otel_span_django_request(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (None, None, None, False) + _, _, output = self._helper(django_expected, flask_expected) + self.assertEqual(output, django_expected) + + def test_no_otel_span_flask_request(self): + django_expected = (None, None, None, False) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + _, _, output = self._helper(django_expected, flask_expected) + + # Django wins + self.assertEqual(output, flask_expected) + + def test_no_otel_span_both_django_and_flask(self): + django_expected = ( + _DJANGO_HTTP_REQUEST, + _DJANGO_TRACE_ID, + _DJANGO_SPAN_ID, + False, + ) + flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) + _, _, output = self._helper(django_expected, flask_expected) + + # Django wins + self.assertEqual(output, django_expected) + class Test__parse_xcloud_trace(unittest.TestCase): @@ -506,126 +621,3 @@ def test_span_exists(self): self.assertEqual(trace_id, _EXPECTED_OTEL_TRACE_ID) self.assertEqual(span_id, _EXPECTED_OTEL_SPAN_ID) self.assertEqual(sampled, _EXPECTED_OTEL_TRACESAMPLED) - - -class Test_get_request_and_trace_data(Test_get_request_data): - @staticmethod - def _call_fut(): - from google.cloud.logging_v2.handlers import _helpers - - http, trace, span, sampled = _helpers.get_request_and_trace_data() - return http, trace, span, sampled - - def test_otel_span_exists_no_request(self): - flask_expected = (None, None, None, False) - django_expected = (None, None, None, False) - - with _setup_otel_span_context(): - _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual( - output, - ( - None, - _EXPECTED_OTEL_TRACE_ID, - _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED, - ), - ) - - def test_otel_span_exists_django_request(self): - django_expected = ( - _DJANGO_HTTP_REQUEST, - _DJANGO_TRACE_ID, - _DJANGO_SPAN_ID, - False, - ) - flask_expected = (None, None, None, False) - - with _setup_otel_span_context(): - _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual( - output, - ( - _DJANGO_HTTP_REQUEST, - _EXPECTED_OTEL_TRACE_ID, - _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED, - ), - ) - - def test_otel_span_exists_flask_request(self): - django_expected = (None, None, None, False) - flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) - - with _setup_otel_span_context(): - _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual( - output, - ( - _FLASK_HTTP_REQUEST, - _EXPECTED_OTEL_TRACE_ID, - _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED, - ), - ) - - def test_otel_span_exists_both_django_and_flask(self): - django_expected = ( - _DJANGO_HTTP_REQUEST, - _DJANGO_TRACE_ID, - _DJANGO_SPAN_ID, - False, - ) - flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) - - with _setup_otel_span_context(): - _, _, output = self._helper(django_expected, flask_expected) - - # Django wins - self.assertEqual( - output, - ( - _DJANGO_HTTP_REQUEST, - _EXPECTED_OTEL_TRACE_ID, - _EXPECTED_OTEL_SPAN_ID, - _EXPECTED_OTEL_TRACESAMPLED, - ), - ) - - def test_no_otel_span_no_requests(self): - flask_expected = (None, None, None, False) - django_expected = (None, None, None, False) - _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, (None, None, None, False)) - - def test_no_otel_span_django_request(self): - django_expected = ( - _DJANGO_HTTP_REQUEST, - _DJANGO_TRACE_ID, - _DJANGO_SPAN_ID, - False, - ) - flask_expected = (None, None, None, False) - _, _, output = self._helper(django_expected, flask_expected) - self.assertEqual(output, django_expected) - - def test_no_otel_span_flask_request(self): - django_expected = (None, None, None, False) - flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) - _, _, output = self._helper(django_expected, flask_expected) - - # Django wins - self.assertEqual(output, flask_expected) - - def test_no_otel_span_both_django_and_flask(self): - django_expected = ( - _DJANGO_HTTP_REQUEST, - _DJANGO_TRACE_ID, - _DJANGO_SPAN_ID, - False, - ) - flask_expected = (_FLASK_HTTP_REQUEST, _FLASK_TRACE_ID, _FLASK_SPAN_ID, False) - _, _, output = self._helper(django_expected, flask_expected) - - # Django wins - self.assertEqual(output, django_expected) From 8ae84932d72d23430dc48c62a585290d9c58488a Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Thu, 16 May 2024 15:03:24 +0000 Subject: [PATCH 13/13] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- google/cloud/logging_v2/handlers/_helpers.py | 7 ++++++- tests/unit/handlers/test__helpers.py | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_helpers.py b/google/cloud/logging_v2/handlers/_helpers.py index 1886f753b..98bf0cd29 100644 --- a/google/cloud/logging_v2/handlers/_helpers.py +++ b/google/cloud/logging_v2/handlers/_helpers.py @@ -242,7 +242,12 @@ def get_request_data(): get_request_data_from_flask, ) - http_request, http_trace_id, http_span_id, http_trace_sampled = None, None, None, False + http_request, http_trace_id, http_span_id, http_trace_sampled = ( + None, + None, + None, + False, + ) for checker in checkers: http_request, http_trace_id, http_span_id, http_trace_sampled = checker() diff --git a/tests/unit/handlers/test__helpers.py b/tests/unit/handlers/test__helpers.py index 850902056..b8c8fc99d 100644 --- a/tests/unit/handlers/test__helpers.py +++ b/tests/unit/handlers/test__helpers.py @@ -362,7 +362,7 @@ def test_missing_both(self): def test_wo_libraries(self): output = self._call_fut() self.assertEqual(output, (None, None, None, False)) - + def test_otel_span_exists_no_request(self): flask_expected = (None, None, None, False) django_expected = (None, None, None, False) @@ -478,7 +478,6 @@ def test_no_otel_span_both_django_and_flask(self): self.assertEqual(output, django_expected) - class Test__parse_xcloud_trace(unittest.TestCase): @staticmethod def _call_fut(header):