fix(click): issue with environment sniffing for click instrumentation

Author: lannuttia

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `opentelemetry-instrumentation-click`: fix issue where starting uvicorn via `python -m` would cause the click instrumentation to give all requests the same trace id
 - `opentelemetry-instrumentation-botocore`: migrate off the deprecated events API to use the logs API
   ([#3624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3624))
 - `opentelemetry-instrumentation-dbapi`: fix crash retrieving libpq version when enabling commenter with psycopg

instrumentation/opentelemetry-instrumentation-click/src/opentelemetry/instrumentation/click/__init__.py

@@ -47,21 +47,8 @@ def hello():
 from typing import Collection
 
 import click
-from wrapt import wrap_function_wrapper
-
-try:
-    from flask.cli import ScriptInfo as FlaskScriptInfo
-except ImportError:
-    FlaskScriptInfo = None
-
-
-from opentelemetry import trace
-from opentelemetry.instrumentation.click.package import _instruments
-from opentelemetry.instrumentation.click.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
-from opentelemetry.instrumentation.utils import (
-    unwrap,
-)
+from opentelemetry.instrumentation.utils import unwrap
 from opentelemetry.semconv._incubating.attributes.process_attributes import (
     PROCESS_COMMAND_ARGS,
     PROCESS_EXECUTABLE_NAME,
@@ -70,22 +57,13 @@ def hello():
 )
 from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
 from opentelemetry.trace.status import StatusCode
+from wrapt import wrap_function_wrapper
 
-_logger = getLogger(__name__)
-
+from opentelemetry import trace
+from opentelemetry.instrumentation.click.package import _instruments
+from opentelemetry.instrumentation.click.version import __version__
 
-def _skip_servers(ctx: click.Context):
-    # flask run
-    if (
-        ctx.info_name == "run"
-        and FlaskScriptInfo
-        and isinstance(ctx.obj, FlaskScriptInfo)
-    ):
-        return True
-    # uvicorn
-    if ctx.info_name == "uvicorn":
-        return True
-    return False
+_logger = getLogger(__name__)
 
 
 def _command_invoke_wrapper(wrapped, instance, args, kwargs, tracer):
@@ -99,7 +77,10 @@ def _command_invoke_wrapper(wrapped, instance, args, kwargs, tracer):
 
     # we don't want to create a root span for long running processes like servers
     # otherwise all requests would have the same trace id
-    if _skip_servers(ctx):
+    if (
+        "opentelemetry.instrumentation.asgi" in sys.modules
+        or "opentelemetry.instrumentation.wsgi" in sys.modules
+    ):
         return wrapped(*args, **kwargs)
 
     span_name = ctx.info_name
@@ -121,9 +102,7 @@ def _command_invoke_wrapper(wrapped, instance, args, kwargs, tracer):
             span.set_status(StatusCode.ERROR, str(exc))
             if span.is_recording():
                 span.set_attribute(ERROR_TYPE, exc.__class__.__qualname__)
-                span.set_attribute(
-                    PROCESS_EXIT_CODE, getattr(exc, "exit_code", 1)
-                )
+                span.set_attribute(PROCESS_EXIT_CODE, getattr(exc, "exit_code", 1))
             raise
 
 

instrumentation/opentelemetry-instrumentation-click/tests/test_click.py

@@ -13,22 +13,18 @@
 # limitations under the License.
 
 import os
+import sys
 from unittest import mock
 
 import click
 import pytest
 from click.testing import CliRunner
-
-try:
-    from flask import cli as flask_cli
-except ImportError:
-    flask_cli = None
-
-from opentelemetry.instrumentation.click import ClickInstrumentor
 from opentelemetry.test.test_base import TestBase
 from opentelemetry.trace import SpanKind
 from opentelemetry.trace.status import StatusCode
 
+from opentelemetry.instrumentation.click import ClickInstrumentor
+
 
 class ClickTestCase(TestBase):
     # pylint: disable=unbalanced-tuple-unpacking
@@ -168,24 +164,35 @@ def command_raises():
             },
         )
 
-    def test_uvicorn_cli_command_ignored(self):
-        @click.command("uvicorn")
-        def command_uvicorn():
+    @mock.patch("sys.argv", ["command.py"])
+    def test_disabled_when_asgi_instrumentation_loaded(self):
+        @click.command()
+        def command():
             pass
 
         runner = CliRunner()
-        result = runner.invoke(command_uvicorn)
+        with mock.patch.dict(
+            sys.modules,
+            {**sys.modules, "opentelemetry.instrumentation.asgi": mock.Mock()},
+        ):
+            result = runner.invoke(command)
         self.assertEqual(result.exit_code, 0)
 
         self.assertFalse(self.memory_exporter.get_finished_spans())
 
-    @pytest.mark.skipif(flask_cli is None, reason="requires flask")
-    def test_flask_run_command_ignored(self):
+    @mock.patch("sys.argv", ["command.py"])
+    def test_disabled_when_wsgi_instrumentation_loaded(self):
+        @click.command()
+        def command():
+            pass
+
         runner = CliRunner()
-        result = runner.invoke(
-            flask_cli.run_command, obj=flask_cli.ScriptInfo()
-        )
-        self.assertEqual(result.exit_code, 2)
+        with mock.patch.dict(
+            sys.modules,
+            {**sys.modules, "opentelemetry.instrumentation.wsgi": mock.Mock()},
+        ):
+            result = runner.invoke(command)
+        self.assertEqual(result.exit_code, 0)
 
         self.assertFalse(self.memory_exporter.get_finished_spans())