From d43ef07d52f3509126247a50056577d9eb424043 Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Tue, 1 Feb 2022 18:18:14 +0530 Subject: [PATCH] Logging: Defensively access provider resource Now service name is extracted from the provider defensively and lazily. This accounts for an SDK that does not provide access to "resource" via TracerProviders and for lazy initialization of TracerProviders. Fixes #810 --- CHANGELOG.md | 5 +++ .../instrumentation/logging/__init__.py | 19 +++++++--- .../tests/test_logging.py | 37 ++++++++++++++++++- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd078ca64a..54dfdc6527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.9.1-0.28b1...HEAD) +### Fixed + +- `opentelemetry-instrumentation-logging` retrieves service name defensively. + ([#890](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/890)) + ## [1.9.1-0.28b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.9.1-0.28b1) - 2022-01-29 diff --git a/instrumentation/opentelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py b/instrumentation/opentelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py index 4671f8707f..66d4cb46e5 100644 --- a/instrumentation/opentelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py @@ -16,7 +16,7 @@ import logging # pylint: disable=import-self from os import environ -from typing import Collection +from typing import Collection, Optional from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.logging.constants import ( @@ -76,20 +76,26 @@ def instrumentation_dependencies(self) -> Collection[str]: return _instruments def _instrument(self, **kwargs): - service_name = "" - provider = kwargs.get("tracer_provider", None) or get_tracer_provider() - resource = provider.resource if provider else None - if resource: - service_name = resource.attributes.get("service.name") + provider = kwargs.get("tracer_provider", None) or get_tracer_provider() old_factory = logging.getLogRecordFactory() LoggingInstrumentor._old_factory = old_factory + service_name = None def record_factory(*args, **kwargs): record = old_factory(*args, **kwargs) record.otelSpanID = "0" record.otelTraceID = "0" + + nonlocal service_name + if service_name is None: + resource = getattr(provider, "resource", None) + if resource: + service_name = resource.attributes.get("service.name") or "" + else: + service_name = "" + record.otelServiceName = service_name span = get_current_span() @@ -100,6 +106,7 @@ def record_factory(*args, **kwargs): record.otelTraceID = format(ctx.trace_id, "032x") return record + logging.setLogRecordFactory(record_factory) set_logging_format = kwargs.get( diff --git a/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py b/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py index b5ba4e4a93..f1cf6e9066 100644 --- a/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py +++ b/instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py @@ -14,6 +14,7 @@ import logging from unittest import mock +from typing import Optional import pytest @@ -22,7 +23,41 @@ LoggingInstrumentor, ) from opentelemetry.test.test_base import TestBase -from opentelemetry.trace import get_tracer +from opentelemetry.trace import get_tracer, ProxyTracer + + +class FakeTracerProvider: + def get_tracer( + self, + instrumenting_module_name: str, + instrumenting_library_version: Optional[str] = None, + schema_url: Optional[str] = None, + ) -> ProxyTracer: + return ProxyTracer() + + +class TestLoggingInstrumentorProxyTracerProvider(TestBase): + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self.caplog = caplog # pylint: disable=attribute-defined-outside-init + + def setUp(self): + super().setUp() + LoggingInstrumentor().instrument(tracer_provider=FakeTracerProvider()) + + def tearDown(self): + super().tearDown() + LoggingInstrumentor().uninstrument() + + def test_trace_context_injection(self): + with self.caplog.at_level(level=logging.INFO): + logger = logging.getLogger("test logger") + logger.info("hello") + self.assertEqual(len(self.caplog.records), 1) + record = self.caplog.records[0] + self.assertEqual(record.otelSpanID, '0') + self.assertEqual(record.otelTraceID, '0') + self.assertEqual(record.otelServiceName, "") class TestLoggingInstrumentor(TestBase):