diff --git a/CHANGELOG.md b/CHANGELOG.md index 024990c91d..63d9a450d3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/instrumentation/opentelemetry-instrumentation-asyncclick/src/opentelemetry/instrumentation/asyncclick/__init__.py b/instrumentation/opentelemetry-instrumentation-asyncclick/src/opentelemetry/instrumentation/asyncclick/__init__.py index db2c815853..57454663f2 100644 --- a/instrumentation/opentelemetry-instrumentation-asyncclick/src/opentelemetry/instrumentation/asyncclick/__init__.py +++ b/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, diff --git a/instrumentation/opentelemetry-instrumentation-asyncclick/tests/test_asyncclick.py b/instrumentation/opentelemetry-instrumentation-asyncclick/tests/test_asyncclick.py index d7104d5059..68a894880b 100644 --- a/instrumentation/opentelemetry-instrumentation-asyncclick/tests/test_asyncclick.py +++ b/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() diff --git a/instrumentation/opentelemetry-instrumentation-click/src/opentelemetry/instrumentation/click/__init__.py b/instrumentation/opentelemetry-instrumentation-click/src/opentelemetry/instrumentation/click/__init__.py index e820ca7d87..23a62044e5 100644 --- a/instrumentation/opentelemetry-instrumentation-click/src/opentelemetry/instrumentation/click/__init__.py +++ b/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 diff --git a/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py b/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py index 1b07f6ab56..127499f32a 100644 --- a/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py +++ b/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())