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

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `opentelemetry-instrumentation-asyncclick`: fix issue where servers using asyncclick would not get a separate span per-request
+- `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-flask`: Do not record `http.server.duration` metrics for excluded URLs.
   ([#3794](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3794))
 - `opentelemetry-instrumentation-botocore`: migrate off the deprecated events API to use the logs API

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

@@ -49,14 +49,7 @@ async def hello():
 import sys
 from functools import partial
 from logging import getLogger
-from typing import (
-    TYPE_CHECKING,
-    Any,
-    Awaitable,
-    Callable,
-    Collection,
-    TypeVar,
-)
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, Collection, TypeVar
 
 import asyncclick
 from typing_extensions import ParamSpec, Unpack
@@ -68,9 +61,7 @@ async def hello():
 from opentelemetry.instrumentation.asyncclick.package import _instruments
 from opentelemetry.instrumentation.asyncclick.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,
@@ -112,6 +103,14 @@ async def _command_invoke_wrapper(
 
     ctx = args[0]
 
+    # 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 (
+        "opentelemetry.instrumentation.asgi" in sys.modules
+        or "opentelemetry.instrumentation.wsgi" in sys.modules
+    ):
+        return await wrapped(*args, **kwargs)
+
     span_name = ctx.info_name
     span_attributes = {
         PROCESS_COMMAND_ARGS: sys.argv,

instrumentation/opentelemetry-instrumentation-asyncclick/tests/test_asyncclick.py

@@ -16,6 +16,7 @@
 
 import asyncio
 import os
+import sys
 from typing import Any
 from unittest import IsolatedAsyncioTestCase, mock
 
@@ -158,6 +159,36 @@ async def command_raises() -> None:
             },
         )
 
+    @mock.patch("sys.argv", ["command.py"])
+    def test_disabled_when_asgi_instrumentation_loaded(self):
+        @asyncclick.command()
+        async def command():
+            pass
+
+        with mock.patch.dict(
+            sys.modules,
+            {**sys.modules, "opentelemetry.instrumentation.asgi": mock.Mock()},
+        ):
+            result = run_asyncclick_command_test(command)
+        self.assertEqual(result.exit_code, 0)
+
+        self.assertFalse(self.memory_exporter.get_finished_spans())
+
+    @mock.patch("sys.argv", ["command.py"])
+    def test_disabled_when_wsgi_instrumentation_loaded(self):
+        @asyncclick.command()
+        async def command():
+            pass
+
+        with mock.patch.dict(
+            sys.modules,
+            {**sys.modules, "opentelemetry.instrumentation.wsgi": mock.Mock()},
+        ):
+            result = run_asyncclick_command_test(command)
+        self.assertEqual(result.exit_code, 0)
+
+        self.assertFalse(self.memory_exporter.get_finished_spans())
+
     def test_uninstrument(self):
         AsyncClickInstrumentor().uninstrument()
 

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

@@ -49,19 +49,11 @@ def hello():
 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,
@@ -74,20 +66,6 @@ def hello():
 _logger = getLogger(__name__)
 
 
-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
-
-
 def _command_invoke_wrapper(wrapped, instance, args, kwargs, tracer):
     # Subclasses of Command include groups and CLI runners, but
     # we only want to instrument the actual commands which are
@@ -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

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

@@ -13,17 +13,12 @@
 # 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
@@ -168,24 +163,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())