From ab3895aabbc496baad476fd25b881d885ae9b32f Mon Sep 17 00:00:00 2001 From: Aidan McMahon-Smith Date: Thu, 7 Nov 2024 12:59:37 +0100 Subject: [PATCH 1/2] Make open-telemetry a soft dependency [RHELDST-27627] Several client tools (e.g. rhsm-tools) make use of various parts of pubtools for interaction with pulp. This has caused issues with rcm-dev, as we have specifically avoided making open-telemetry packages available for CLI clients. This change introduces a new TracingWrapper class that acts as a pass through for when opentelemetry isn't installed. --- requirements.txt | 2 -- setup.py | 3 +++ src/pubtools/_impl/tracing.py | 31 +++++++++++++++--------- test-requirements.in | 2 ++ test-requirements.txt | 14 ++++++----- tests/tracing/test_instrument_tracing.py | 19 +++++++++++++++ 6 files changed, 52 insertions(+), 19 deletions(-) diff --git a/requirements.txt b/requirements.txt index a38c65c..c0ec2d3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,2 @@ pluggy setuptools -opentelemetry-api -opentelemetry-sdk diff --git a/setup.py b/setup.py index 30b44fe..64e72e4 100644 --- a/setup.py +++ b/setup.py @@ -54,6 +54,9 @@ def get_requirements(): "Topic :: Software Development :: Libraries :: Python Modules", ], install_requires=get_requirements(), + extras_require={ + "tracing": ["opentelemetry-api", "opentelemetry-sdk"], + }, python_requires=">=3.6", project_urls={ "Documentation": "https://release-engineering.github.io/pubtools/", diff --git a/src/pubtools/_impl/tracing.py b/src/pubtools/_impl/tracing.py index b37ecd5..2445f5f 100644 --- a/src/pubtools/_impl/tracing.py +++ b/src/pubtools/_impl/tracing.py @@ -12,14 +12,20 @@ def func(): import os import threading -from opentelemetry import baggage, context, trace -from opentelemetry.baggage.propagation import W3CBaggagePropagator -from opentelemetry.propagate import set_global_textmap -from opentelemetry.sdk.resources import SERVICE_NAME, Resource -from opentelemetry.sdk.trace import TracerProvider -from opentelemetry.sdk.trace.export import BatchSpanProcessor, ConsoleSpanExporter -from opentelemetry.trace import Status, StatusCode -from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator +try: + from opentelemetry import baggage, context, trace + from opentelemetry.baggage.propagation import W3CBaggagePropagator + from opentelemetry.propagate import set_global_textmap + from opentelemetry.sdk.resources import SERVICE_NAME, Resource + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import BatchSpanProcessor, ConsoleSpanExporter + from opentelemetry.trace import Status, StatusCode + from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator + OPENTELEMETRY_AVAILABLE = True +except ImportError: # pragma: no cover + # Clients aren't expected to have open-telemetry. This flag will be used in + # TracingWrapper to provide pass through functions. + OPENTELEMETRY_AVAILABLE = False from pubtools.pluggy import pm @@ -60,7 +66,11 @@ def _reset(self): # trace.set_tracer_provider global singleton set below can *never* be set up # more than once in a single process. - self._enabled_trace = os.getenv("OTEL_TRACING", "").lower() == "true" + self._enabled_trace = (os.getenv("OTEL_TRACING", "").lower() == "true") \ + and OPENTELEMETRY_AVAILABLE + if (os.getenv("OTEL_TRACING", "").lower() == "true") and not OPENTELEMETRY_AVAILABLE: + log.debug("Tracing is enabled but the open telemetry package is " + "unavailable. Tracing functionality will be disabled.") if self._enabled_trace and not self._processor: log.info("Creating TracingWrapper instance") exporter = pm.hook.otel_exporter() or ConsoleSpanExporter() @@ -87,8 +97,6 @@ def instrument_func(self, span_name=None, carrier=None, args_to_attr=False): Returns: The decorated function """ - tracer = trace.get_tracer(__name__) - def _instrument_func(func): @functools.wraps(func) def wrap(*args, **kwargs): @@ -104,6 +112,7 @@ def wrap(*args, **kwargs): if not self._enabled_trace: return func(*args, **kwargs) + tracer = trace.get_tracer(__name__) trace_ctx = None token = None if not context.get_current(): diff --git a/test-requirements.in b/test-requirements.in index 9955dec..405e5c7 100644 --- a/test-requirements.in +++ b/test-requirements.in @@ -1,2 +1,4 @@ pytest pytest-cov +opentelemetry-api +opentelemetry-sdk diff --git a/test-requirements.txt b/test-requirements.txt index 357b032..0c43023 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -701,6 +701,7 @@ opentelemetry-api==1.20.0 \ --hash=sha256:06abe351db7572f8afdd0fb889ce53f3c992dbf6f6262507b385cc1963e06983 \ --hash=sha256:982b76036fec0fdaf490ae3dfd9f28c81442a33414f737abc687a32758cdcba5 # via + # -r test-requirements.in # opentelemetry-exporter-otlp-proto-grpc # opentelemetry-exporter-otlp-proto-http # opentelemetry-sdk @@ -735,6 +736,7 @@ opentelemetry-sdk==1.20.0 \ --hash=sha256:702e432a457fa717fd2ddfd30640180e69938f85bb7fec3e479f85f61c1843f8 \ --hash=sha256:f2230c276ff4c63ea09b3cb2e2ac6b1265f90af64e8d16bbf275c81a9ce8e804 # via + # -r test-requirements.in # opentelemetry-exporter-otlp-proto-grpc # opentelemetry-exporter-otlp-proto-http # pubtools @@ -898,9 +900,9 @@ pyrsistent==0.20.0 \ --hash=sha256:f920385a11207dc372a028b3f1e1038bb244b3ec38d448e6d8e43c6b3ba20e98 \ --hash=sha256:fed2c3216a605dc9a6ea50c7e84c82906e3684c4e80d2908208f662a6cbf9022 # via pubtools-quay -pyspnego[kerberos]==0.11.1 \ - --hash=sha256:129a4294f2c4d681d5875240ef87accc6f1d921e8983737fb0b59642b397951e \ - --hash=sha256:e92ed8b0a62765b9d6abbb86a48cf871228ddb97678598dc01c9c39a626823f6 +pyspnego[kerberos]==0.11.2 \ + --hash=sha256:74abc1fb51e59360eb5c5c9086e5962174f1072c7a50cf6da0bda9a4bcfdfbd4 \ + --hash=sha256:994388d308fb06e4498365ce78d222bf4f3570b6df4ec95738431f61510c971b # via requests-kerberos pytest==8.3.3 \ --hash=sha256:70b98107bd648308a7952b06e6ca9a50bc660be218d53c257cc1fc94fda10181 \ @@ -1165,9 +1167,9 @@ tenacity==9.0.0 \ --hash=sha256:807f37ca97d62aa361264d497b0e31e92b8027044942bfa756160d908320d73b \ --hash=sha256:93de0c98785b27fcf659856aa9f54bfbd399e29969b0621bc7f762bd441b4539 # via iiblib -tomli==2.0.2 \ - --hash=sha256:2ebe24485c53d303f690b0ec092806a085f07af5a5aa1464f3931eec36caaa38 \ - --hash=sha256:d46d457a85337051c36524bc5349dd91b1877838e2979ac5ced3e710ed8a60ed +tomli==2.1.0 \ + --hash=sha256:3f646cae2aec94e17d04973e4249548320197cfabdf130015d023de4b74d8ab8 \ + --hash=sha256:a5c57c3d1c56f5ccdf89f6523458f60ef716e210fc47c4cfb188c5ba473e0391 # via # coverage # pytest diff --git a/tests/tracing/test_instrument_tracing.py b/tests/tracing/test_instrument_tracing.py index 206c39d..4882d16 100644 --- a/tests/tracing/test_instrument_tracing.py +++ b/tests/tracing/test_instrument_tracing.py @@ -5,7 +5,9 @@ from opentelemetry.trace.status import StatusCode from pubtools._impl.tracing import TracingWrapper +from pubtools._impl import tracing from pubtools.tracing import get_trace_wrapper +import logging def test_instrument_func_in_context(monkeypatch, fake_span_exporter): @@ -154,3 +156,20 @@ def foo(): assert foo() == 1 assert tw.provider is None + +def test_otel_not_available(caplog, monkeypatch): + monkeypatch.setenv("OTEL_TRACING", "true") + monkeypatch.setattr(tracing, "OPENTELEMETRY_AVAILABLE", False) + caplog.set_level(logging.DEBUG) + + # Recreate an instance of TracingWrapper + tw = TracingWrapper() + + @tw.instrument_func() + def foo(): + return 1 + + assert foo() == 1 + assert tw.provider is None + assert "Tracing is enabled but the open telemetry package is unavailable. " \ + "Tracing functionality will be disabled." in caplog.text From 8928f6fdf7a6a6aba30b531ec9c279dc8fb934b3 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 13 Nov 2024 12:42:18 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/pubtools/_impl/tracing.py | 21 +++++++++++++++------ tests/tracing/test_instrument_tracing.py | 11 +++++++---- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/pubtools/_impl/tracing.py b/src/pubtools/_impl/tracing.py index 2445f5f..f494712 100644 --- a/src/pubtools/_impl/tracing.py +++ b/src/pubtools/_impl/tracing.py @@ -20,7 +20,10 @@ def func(): from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor, ConsoleSpanExporter from opentelemetry.trace import Status, StatusCode - from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator + from opentelemetry.trace.propagation.tracecontext import ( + TraceContextTextMapPropagator, + ) + OPENTELEMETRY_AVAILABLE = True except ImportError: # pragma: no cover # Clients aren't expected to have open-telemetry. This flag will be used in @@ -66,11 +69,16 @@ def _reset(self): # trace.set_tracer_provider global singleton set below can *never* be set up # more than once in a single process. - self._enabled_trace = (os.getenv("OTEL_TRACING", "").lower() == "true") \ - and OPENTELEMETRY_AVAILABLE - if (os.getenv("OTEL_TRACING", "").lower() == "true") and not OPENTELEMETRY_AVAILABLE: - log.debug("Tracing is enabled but the open telemetry package is " - "unavailable. Tracing functionality will be disabled.") + self._enabled_trace = ( + os.getenv("OTEL_TRACING", "").lower() == "true" + ) and OPENTELEMETRY_AVAILABLE + if ( + os.getenv("OTEL_TRACING", "").lower() == "true" + ) and not OPENTELEMETRY_AVAILABLE: + log.debug( + "Tracing is enabled but the open telemetry package is " + "unavailable. Tracing functionality will be disabled." + ) if self._enabled_trace and not self._processor: log.info("Creating TracingWrapper instance") exporter = pm.hook.otel_exporter() or ConsoleSpanExporter() @@ -97,6 +105,7 @@ def instrument_func(self, span_name=None, carrier=None, args_to_attr=False): Returns: The decorated function """ + def _instrument_func(func): @functools.wraps(func) def wrap(*args, **kwargs): diff --git a/tests/tracing/test_instrument_tracing.py b/tests/tracing/test_instrument_tracing.py index 4882d16..aeddcdc 100644 --- a/tests/tracing/test_instrument_tracing.py +++ b/tests/tracing/test_instrument_tracing.py @@ -1,13 +1,13 @@ +import logging from concurrent.futures import ThreadPoolExecutor, as_completed import pytest from opentelemetry import trace from opentelemetry.trace.status import StatusCode -from pubtools._impl.tracing import TracingWrapper from pubtools._impl import tracing +from pubtools._impl.tracing import TracingWrapper from pubtools.tracing import get_trace_wrapper -import logging def test_instrument_func_in_context(monkeypatch, fake_span_exporter): @@ -157,6 +157,7 @@ def foo(): assert foo() == 1 assert tw.provider is None + def test_otel_not_available(caplog, monkeypatch): monkeypatch.setenv("OTEL_TRACING", "true") monkeypatch.setattr(tracing, "OPENTELEMETRY_AVAILABLE", False) @@ -171,5 +172,7 @@ def foo(): assert foo() == 1 assert tw.provider is None - assert "Tracing is enabled but the open telemetry package is unavailable. " \ - "Tracing functionality will be disabled." in caplog.text + assert ( + "Tracing is enabled but the open telemetry package is unavailable. " + "Tracing functionality will be disabled." in caplog.text + )