elastic / apm-agent-python

https://www.elastic.co/guide/en/apm/agent/python/current/index.html
BSD 3-Clause "New" or "Revised" License
415 stars 220 forks source link

When APM is disabled, OTEL's `start_span` causes exception #2163

Open Arvinje opened 15 hours ago

Arvinje commented 15 hours ago

Describe the bug: ... When ELASTIC_APM_ENABLED is set to false, start_span() should not result in an exception. This happens when the Tracer from from elasticapm.contrib.opentelemetry.trace is used.

To Reproduce

  1. Set ELASTIC_APM_ENABLED to false
  2. Call start_as_current_span or start_span (from Tracer of elasticapm.contrib.opentelemetry.trace)
  3. Notice an exception is thrown

Environment (please complete the following information)

Additional context

Here's the exception:

 with tracer.start_as_current_span(name) as span:\\n    
 |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\\n    
 |   File \\\"/usr/local/lib/python3.12/contextlib.py\\\", line 137, in __enter__\\n    
 |     return next(self.gen)\\n    
 |            ^^^^^^^^^^^^^^\\n    
 |   File \\\"/app/.venv/lib/python3.12/site-packages/elasticapm/contrib/opentelemetry/trace.py\\\", line 230, in start_as_current_span\\n    
 |     span = self.start_span(\\n    
 |            ^^^^^^^^^^^^^^^^\\n    
 |   File \\\"/app/.venv/lib/python3.12/site-packages/elasticapm/contrib/opentelemetry/trace.py\\\", line 150, in start_span\\n    
 |     span = Span(\\n    
 |            ^^^^^\\n    
 |   File \\\"/app/.venv/lib/python3.12/site-packages/elasticapm/contrib/opentelemetry/span.py\\\", line 65, in __init__\\n    
 |     elastic_span.otel_wrapper = self\\n    
 |     ^^^^^^^^^^^^^^^^^^^^^^^^^\\n    
 | AttributeError: 'NoneType' object has no attribute 'otel_wrapper'\\n
xrmx commented 8 hours ago

Thanks for reporting. Wrote a quick test for reproducing this ~but it still passes, anyway looking at the code I see how it can fail~:

diff --git a/tests/contrib/opentelemetry/tests.py b/tests/contrib/opentelemetry/tests.py
index 3a302289..a3e0a705 100755
--- a/tests/contrib/opentelemetry/tests.py
+++ b/tests/contrib/opentelemetry/tests.py
@@ -48,6 +48,19 @@ def tracer(elasticapm_client) -> Tracer:
     yield Tracer("test", elasticapm_client=elasticapm_client)

+@pytest.mark.parametrize("elasticapm_client", [{"enabled": False}], indirect=True)
+def test_root_transaction_not_enabled(elasticapm_client, tracer):
+    with tracer.start_as_current_span("test"):
+        pass
+
+
 def test_root_transaction(tracer: Tracer):
     with tracer.start_as_current_span("test"):
         pass
xrmx commented 8 hours ago

@Arvinje Does setting OTEL_SDK_DISABLED=true env var help?

Arvinje commented 3 hours ago

@xrmx no it doesn't. I hoped similar to the NodeJS package, ELASTIC_APM_ENABLED=false would turn methods of the OTel bridge into noop.