From 16c041e22bc7caf8b139d7aee00b0b0719ab4d2c Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 2 Jan 2025 20:15:01 +0100 Subject: [PATCH 01/13] Support PEP 561 to `opentelemetry-instrumentation-sqlite3` (#3133) --- CHANGELOG.md | 2 ++ .../src/opentelemetry/instrumentation/sqlite3/py.typed | 0 2 files changed, 2 insertions(+) create mode 100644 instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/py.typed diff --git a/CHANGELOG.md b/CHANGELOG.md index 7de249bf2c..6bc8eb44f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3100](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3100)) - Add support to database stability opt-in in `_semconv` utilities and add tests ([#3111](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3111)) +- `opentelemetry-opentelemetry-sqlite3` Add `py.typed` file to enable PEP 561 + ([#3133](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3133)) - `opentelemetry-instrumentation-falcon` add support version to v4 ([#3086](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3086)) - add support to Python 3.13 diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/py.typed b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/py.typed new file mode 100644 index 0000000000..e69de29bb2 From 3e394a4814527d6cd37aade3ac3b9dc4d2349e14 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 2 Jan 2025 20:30:41 +0100 Subject: [PATCH 02/13] Support PEP 561 to `opentelemetry-instrumentation-system-metrics` (#3132) --- CHANGELOG.md | 2 ++ .../instrumentation/system_metrics/__init__.py | 16 ++++++++-------- .../instrumentation/system_metrics/py.typed | 0 3 files changed, 10 insertions(+), 8 deletions(-) create mode 100644 instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/py.typed diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc8eb44f4..dfe1997d70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3100](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3100)) - Add support to database stability opt-in in `_semconv` utilities and add tests ([#3111](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3111)) +- `opentelemetry-instrumentation-system-metrics` Add `py.typed` file to enable PEP 561 + ([#3132](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3132)) - `opentelemetry-opentelemetry-sqlite3` Add `py.typed` file to enable PEP 561 ([#3133](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3133)) - `opentelemetry-instrumentation-falcon` add support version to v4 diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py b/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py index aff86ea77b..a81c5bf7ff 100644 --- a/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py @@ -76,18 +76,18 @@ --- """ +from __future__ import annotations + import gc import logging import os import sys import threading from platform import python_implementation -from typing import Collection, Dict, Iterable, List, Optional +from typing import Any, Collection, Iterable import psutil -# FIXME Remove this pylint disabling line when Github issue is cleared -# pylint: disable=no-name-in-module from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.system_metrics.package import _instruments from opentelemetry.instrumentation.system_metrics.version import __version__ @@ -96,7 +96,7 @@ _logger = logging.getLogger(__name__) -_DEFAULT_CONFIG = { +_DEFAULT_CONFIG: dict[str, list[str] | None] = { "system.cpu.time": ["idle", "user", "system", "irq"], "system.cpu.utilization": ["idle", "user", "system", "irq"], "system.memory.usage": ["used", "free", "cached"], @@ -129,8 +129,8 @@ class SystemMetricsInstrumentor(BaseInstrumentor): def __init__( self, - labels: Optional[Dict[str, str]] = None, - config: Optional[Dict[str, List[str]]] = None, + labels: dict[str, str] | None = None, + config: dict[str, list[str] | None] | None = None, ): super().__init__() if config is None: @@ -176,7 +176,7 @@ def __init__( def instrumentation_dependencies(self) -> Collection[str]: return _instruments - def _instrument(self, **kwargs): + def _instrument(self, **kwargs: Any): # pylint: disable=too-many-branches meter_provider = kwargs.get("meter_provider") self._meter = get_meter( @@ -408,7 +408,7 @@ def _instrument(self, **kwargs): description="Number of file descriptors in use by the process.", ) - def _uninstrument(self, **__): + def _uninstrument(self, **kwargs: Any): pass def _get_open_file_descriptors( diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/py.typed b/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/py.typed new file mode 100644 index 0000000000..e69de29bb2 From e5eb524e8985a853518507320913ea1defba99aa Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 2 Jan 2025 20:44:58 +0100 Subject: [PATCH 03/13] Improve type hints in `opentelemetry.instrumentation.utils` (#3128) --- .../opentelemetry/instrumentation/utils.py | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index a0d9ae18f9..d5bf5db7fd 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -12,11 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import annotations + import urllib.parse from contextlib import contextmanager from importlib import import_module from re import escape, sub -from typing import Dict, Iterable, Sequence, Union +from typing import Any, Dict, Generator, Sequence from wrapt import ObjectProxy @@ -44,9 +46,9 @@ def extract_attributes_from_object( - obj: any, attributes: Sequence[str], existing: Dict[str, str] = None + obj: Any, attributes: Sequence[str], existing: Dict[str, str] | None = None ) -> Dict[str, str]: - extracted = {} + extracted: dict[str, str] = {} if existing: extracted.update(existing) for attr in attributes: @@ -81,7 +83,7 @@ def http_status_to_status_code( return StatusCode.ERROR -def unwrap(obj: Union[object, str], attr: str): +def unwrap(obj: object, attr: str): """Given a function that was wrapped by wrapt.wrap_function_wrapper, unwrap it The object containing the function to unwrap may be passed as dotted module path string. @@ -152,7 +154,7 @@ def _start_internal_or_server_span( return span, token -def _url_quote(s) -> str: # pylint: disable=invalid-name +def _url_quote(s: Any) -> str: # pylint: disable=invalid-name if not isinstance(s, (str, bytes)): return s quoted = urllib.parse.quote(s) @@ -163,13 +165,13 @@ def _url_quote(s) -> str: # pylint: disable=invalid-name return quoted.replace("%", "%%") -def _get_opentelemetry_values() -> dict: +def _get_opentelemetry_values() -> dict[str, Any]: """ Return the OpenTelemetry Trace and Span IDs if Span ID is set in the OpenTelemetry execution context. """ # Insert the W3C TraceContext generated - _headers = {} + _headers: dict[str, Any] = {} propagator.inject(_headers) return _headers @@ -196,7 +198,7 @@ def is_http_instrumentation_enabled() -> bool: @contextmanager -def _suppress_instrumentation(*keys: str) -> Iterable[None]: +def _suppress_instrumentation(*keys: str) -> Generator[None]: """Suppress instrumentation within the context.""" ctx = context.get_current() for key in keys: @@ -209,7 +211,7 @@ def _suppress_instrumentation(*keys: str) -> Iterable[None]: @contextmanager -def suppress_instrumentation() -> Iterable[None]: +def suppress_instrumentation() -> Generator[None]: """Suppress instrumentation within the context.""" with _suppress_instrumentation( _SUPPRESS_INSTRUMENTATION_KEY, _SUPPRESS_INSTRUMENTATION_KEY_PLAIN @@ -218,7 +220,7 @@ def suppress_instrumentation() -> Iterable[None]: @contextmanager -def suppress_http_instrumentation() -> Iterable[None]: +def suppress_http_instrumentation() -> Generator[None]: """Suppress instrumentation within the context.""" with _suppress_instrumentation(_SUPPRESS_HTTP_INSTRUMENTATION_KEY): yield From 95f14cd8df05b87b564024d47d14f29211f3b98e Mon Sep 17 00:00:00 2001 From: GonzaloGuasch Date: Thu, 2 Jan 2025 16:58:17 -0300 Subject: [PATCH 04/13] wsgi: always record span status code to have it available in metrics (#3148) --- CHANGELOG.md | 2 ++ .../opentelemetry/instrumentation/wsgi/__init__.py | 4 ---- .../tests/test_wsgi_middleware.py | 13 +++++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfe1997d70..8a4de30776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3133](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3133)) - `opentelemetry-instrumentation-falcon` add support version to v4 ([#3086](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3086)) +- `opentelemetry-instrumentation-wsgi` always record span status code to have it available in metrics + ([#3148](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3148)) - add support to Python 3.13 ([#3134](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3134)) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index eb7cbced9c..bd3b2d18db 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -480,11 +480,7 @@ def add_response_attributes( """Adds HTTP response attributes to span using the arguments passed to a PEP3333-conforming start_response callable. """ - if not span.is_recording(): - return status_code_str, _ = start_response_status.split(" ", 1) - - status_code = 0 try: status_code = int(status_code_str) except ValueError: diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index da1a3c2696..9d7d1240a7 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -779,6 +779,19 @@ def test_response_attributes(self): self.span.set_attribute.assert_has_calls(expected, any_order=True) self.span.set_attribute.assert_has_calls(expected_new, any_order=True) + def test_response_attributes_noop(self): + mock_span = mock.Mock() + mock_span.is_recording.return_value = False + + attrs = {} + otel_wsgi.add_response_attributes( + mock_span, "404 Not Found", {}, duration_attrs=attrs + ) + + self.assertEqual(mock_span.set_attribute.call_count, 0) + self.assertEqual(mock_span.is_recording.call_count, 2) + self.assertEqual(attrs[SpanAttributes.HTTP_STATUS_CODE], 404) + def test_credential_removal(self): self.environ["HTTP_HOST"] = "username:password@mock" self.environ["PATH_INFO"] = "/status/200" From 3d5935f4f65f5ead41403d5b172f651ca0e533b3 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 2 Jan 2025 21:18:07 +0100 Subject: [PATCH 05/13] docs: add missing import to aws-lambda (#3154) --- .../src/opentelemetry/instrumentation/aws_lambda/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py b/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py index b31e6dea72..b1f61b9ce8 100644 --- a/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aws-lambda/src/opentelemetry/instrumentation/aws_lambda/__init__.py @@ -55,6 +55,7 @@ def lambda_handler(event, context): .. code:: python from opentelemetry.instrumentation.aws_lambda import AwsLambdaInstrumentor + from opentelemetry.propagate import get_global_textmap def custom_event_context_extractor(lambda_event): # If the `TraceContextTextMapPropagator` is the global propagator, we From 147e3f754e9cc94c16371cc177d7f968c3e60693 Mon Sep 17 00:00:00 2001 From: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> Date: Wed, 8 Jan 2025 01:02:55 -0800 Subject: [PATCH 06/13] Add check before iterating over dist.requires at load_instrumentor (#3168) * Add check before iterate over dist.requires * Changelog * Add test --- CHANGELOG.md | 2 ++ .../instrumentation/dependencies.py | 15 ++++++++------- .../tests/test_dependencies.py | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a4de30776..80d63d0395 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-httpx` Fix `RequestInfo`/`ResponseInfo` type hints ([#3105](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3105)) +- `opentelemetry-instrumentation` Fix `get_dist_dependency_conflicts` if no distribution requires + ([#3168](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3168)) ## Version 1.29.0/0.50b0 (2024-12-11) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py index 8f0a383412..b7e4cff400 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py @@ -47,13 +47,14 @@ def get_dist_dependency_conflicts( extra = "extra" instruments = "instruments" instruments_marker = {extra: instruments} - for dep in dist.requires: - if extra not in dep or instruments not in dep: - continue - - req = Requirement(dep) - if req.marker.evaluate(instruments_marker): - instrumentation_deps.append(req) + if dist.requires: + for dep in dist.requires: + if extra not in dep or instruments not in dep: + continue + + req = Requirement(dep) + if req.marker.evaluate(instruments_marker): + instrumentation_deps.append(req) return get_dependency_conflicts(instrumentation_deps) diff --git a/opentelemetry-instrumentation/tests/test_dependencies.py b/opentelemetry-instrumentation/tests/test_dependencies.py index bdee0f6f01..ca04833181 100644 --- a/opentelemetry-instrumentation/tests/test_dependencies.py +++ b/opentelemetry-instrumentation/tests/test_dependencies.py @@ -86,3 +86,19 @@ def requires(self): str(conflict), 'DependencyConflict: requested: "test-pkg~=1.0; extra == "instruments"" but found: "None"', ) + + def test_get_dist_dependency_conflicts_requires_none(self): + class MockDistribution(Distribution): + def locate_file(self, path): + pass + + def read_text(self, filename): + pass + + @property + def requires(self): + return None + + dist = MockDistribution() + conflict = get_dist_dependency_conflicts(dist) + self.assertTrue(conflict is None) From 908437db5df522697f5504d44fc3d1fec56830be Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 9 Jan 2025 11:29:31 +0100 Subject: [PATCH 07/13] click: ignore click based servers (#3174) * click: ignore click based servers We don't want to create a root span for long running processes like servers otherwise all requests would have the same trace id which is unfortunate. --------- Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> --- CHANGELOG.md | 2 ++ .../instrumentation/click/__init__.py | 30 ++++++++++++++++++- .../test-requirements.txt | 9 ++++++ .../tests/test_click.py | 29 +++++++++++++++++- 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80d63d0395..8c88a0243d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-httpx` Fix `RequestInfo`/`ResponseInfo` type hints ([#3105](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3105)) +- `opentelemetry-instrumentation-click` Disable tracing of well-known server click commands + ([#3174](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3174)) - `opentelemetry-instrumentation` Fix `get_dist_dependency_conflicts` if no distribution requires ([#3168](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3168)) 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 8222bfdf5a..e820ca7d87 100644 --- a/instrumentation/opentelemetry-instrumentation-click/src/opentelemetry/instrumentation/click/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-click/src/opentelemetry/instrumentation/click/__init__.py @@ -13,7 +13,9 @@ # limitations under the License. """ -Instrument `click`_ CLI applications. +Instrument `click`_ CLI applications. The instrumentor will avoid instrumenting +well-known servers (e.g. *flask run* and *uvicorn*) to avoid unexpected effects +like every request having the same Trace ID. .. _click: https://pypi.org/project/click/ @@ -47,6 +49,12 @@ 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__ @@ -66,6 +74,20 @@ 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 @@ -74,6 +96,12 @@ def _command_invoke_wrapper(wrapped, instance, args, kwargs, tracer): return wrapped(*args, **kwargs) 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 _skip_servers(ctx): + return wrapped(*args, **kwargs) + span_name = ctx.info_name span_attributes = { PROCESS_COMMAND_ARGS: sys.argv, diff --git a/instrumentation/opentelemetry-instrumentation-click/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-click/test-requirements.txt index 6e9162ccde..e07bd05bb7 100644 --- a/instrumentation/opentelemetry-instrumentation-click/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-click/test-requirements.txt @@ -1,7 +1,12 @@ asgiref==3.8.1 +blinker==1.7.0 click==8.1.7 Deprecated==1.2.14 +Flask==3.0.2 iniconfig==2.0.0 +itsdangerous==2.1.2 +Jinja2==3.1.4 +MarkupSafe==2.1.2 packaging==24.0 pluggy==1.5.0 py-cpuinfo==9.0.0 @@ -9,7 +14,11 @@ pytest==7.4.4 pytest-asyncio==0.23.5 tomli==2.0.1 typing_extensions==4.12.2 +Werkzeug==3.0.6 wrapt==1.16.0 zipp==3.19.2 -e opentelemetry-instrumentation -e instrumentation/opentelemetry-instrumentation-click +-e instrumentation/opentelemetry-instrumentation-flask +-e instrumentation/opentelemetry-instrumentation-wsgi +-e util/opentelemetry-util-http diff --git a/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py b/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py index 41d01a5bb4..1b07f6ab56 100644 --- a/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py +++ b/instrumentation/opentelemetry-instrumentation-click/tests/test_click.py @@ -16,8 +16,14 @@ 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 @@ -60,7 +66,7 @@ def command(): ) @mock.patch("sys.argv", ["flask", "command"]) - def test_flask_run_command_wrapping(self): + def test_flask_command_wrapping(self): @click.command() def command(): pass @@ -162,6 +168,27 @@ def command_raises(): }, ) + def test_uvicorn_cli_command_ignored(self): + @click.command("uvicorn") + def command_uvicorn(): + pass + + runner = CliRunner() + result = runner.invoke(command_uvicorn) + 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): + runner = CliRunner() + result = runner.invoke( + flask_cli.run_command, obj=flask_cli.ScriptInfo() + ) + self.assertEqual(result.exit_code, 2) + + self.assertFalse(self.memory_exporter.get_finished_spans()) + def test_uninstrument(self): ClickInstrumentor().uninstrument() From 26bcc9347b76eb21f33e0491e4c152d61cbd2390 Mon Sep 17 00:00:00 2001 From: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> Date: Thu, 9 Jan 2025 12:56:14 -0800 Subject: [PATCH 08/13] SQLAlchemy: db.statement inclusion of sqlcomment as opt-in (#3112) --- CHANGELOG.md | 4 + .../instrumentation/sqlalchemy/__init__.py | 44 +++- .../instrumentation/sqlalchemy/engine.py | 90 +++++--- .../tests/test_sqlalchemy.py | 182 ++++++++++++++++ .../tests/test_sqlcommenter.py | 206 +++++++++++++++++- 5 files changed, 497 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c88a0243d..d87091a2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation` Fix `get_dist_dependency_conflicts` if no distribution requires ([#3168](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3168)) +### Breaking changes + +- `opentelemetry-instrumentation-sqlalchemy` including sqlcomment in `db.statement` span attribute value is now opt-in + ([#3112](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3112)) ## Version 1.29.0/0.50b0 (2024-12-11) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py index 4182c0034e..23b68a6c52 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -65,6 +65,30 @@ :: Enabling this flag will add traceparent values /*traceparent='00-03afa25236b8cd948fa853d67038ac79-405ff022e8247c46-01'*/ +SQLComment in span attribute +**************************** +If sqlcommenter is enabled, you can further configure SQLAlchemy instrumentation to append sqlcomment to the `db.statement` span attribute for convenience of your platform. + +.. code:: python + + from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor + + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={}, + enable_attribute_commenter=True, + ) + + +For example, +:: + + Invoking `engine.execute("select * from auth_users")` will lead to sql query "select * from auth_users" but when SQLCommenter and `attribute_commenter` is enabled + the query will get appended with some configurable tags like "select * from auth_users /*tag=value*/;" for both server query and `db.statement` span attribute. + +Warning: capture of sqlcomment in ``db.statement`` may have high cardinality without platform normalization. See `Semantic Conventions for database spans `_ for more information. + + Usage ----- .. code:: python @@ -138,6 +162,7 @@ def _instrument(self, **kwargs): ``meter_provider``: a MeterProvider, defaults to global ``enable_commenter``: bool to enable sqlcommenter, defaults to False ``commenter_options``: dict of sqlcommenter config, defaults to {} + ``enable_attribute_commenter``: bool to enable sqlcomment addition to span attribute, defaults to False. Must also set `enable_commenter`. Returns: An instrumented engine if passed in as an argument or list of instrumented engines, None otherwise. @@ -166,19 +191,30 @@ def _instrument(self, **kwargs): enable_commenter = kwargs.get("enable_commenter", False) commenter_options = kwargs.get("commenter_options", {}) + enable_attribute_commenter = kwargs.get( + "enable_attribute_commenter", False + ) _w( "sqlalchemy", "create_engine", _wrap_create_engine( - tracer, connections_usage, enable_commenter, commenter_options + tracer, + connections_usage, + enable_commenter, + commenter_options, + enable_attribute_commenter, ), ) _w( "sqlalchemy.engine", "create_engine", _wrap_create_engine( - tracer, connections_usage, enable_commenter, commenter_options + tracer, + connections_usage, + enable_commenter, + commenter_options, + enable_attribute_commenter, ), ) # sqlalchemy.engine.create is not present in earlier versions of sqlalchemy (which we support) @@ -191,6 +227,7 @@ def _instrument(self, **kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ), ) _w( @@ -207,6 +244,7 @@ def _instrument(self, **kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ), ) if kwargs.get("engine") is not None: @@ -216,6 +254,7 @@ def _instrument(self, **kwargs): connections_usage, kwargs.get("enable_commenter", False), kwargs.get("commenter_options", {}), + kwargs.get("enable_attribute_commenter", False), ) if kwargs.get("engines") is not None and isinstance( kwargs.get("engines"), Sequence @@ -227,6 +266,7 @@ def _instrument(self, **kwargs): connections_usage, kwargs.get("enable_commenter", False), kwargs.get("commenter_options", {}), + kwargs.get("enable_attribute_commenter", False), ) for engine in kwargs.get("engines") ] diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py index a20e481819..a3312bd77c 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py @@ -43,7 +43,11 @@ def _normalize_vendor(vendor): def _wrap_create_async_engine( - tracer, connections_usage, enable_commenter=False, commenter_options=None + tracer, + connections_usage, + enable_commenter=False, + commenter_options=None, + enable_attribute_commenter=False, ): # pylint: disable=unused-argument def _wrap_create_async_engine_internal(func, module, args, kwargs): @@ -57,6 +61,7 @@ def _wrap_create_async_engine_internal(func, module, args, kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ) return engine @@ -64,7 +69,11 @@ def _wrap_create_async_engine_internal(func, module, args, kwargs): def _wrap_create_engine( - tracer, connections_usage, enable_commenter=False, commenter_options=None + tracer, + connections_usage, + enable_commenter=False, + commenter_options=None, + enable_attribute_commenter=False, ): def _wrap_create_engine_internal(func, _module, args, kwargs): """Trace the SQLAlchemy engine, creating an `EngineTracer` @@ -77,6 +86,7 @@ def _wrap_create_engine_internal(func, _module, args, kwargs): connections_usage, enable_commenter, commenter_options, + enable_attribute_commenter, ) return engine @@ -110,12 +120,14 @@ def __init__( connections_usage, enable_commenter=False, commenter_options=None, + enable_attribute_commenter=False, ): self.tracer = tracer self.connections_usage = connections_usage self.vendor = _normalize_vendor(engine.name) self.enable_commenter = enable_commenter self.commenter_options = commenter_options if commenter_options else {} + self.enable_attribute_commenter = enable_attribute_commenter self._engine_attrs = _get_attributes_from_engine(engine) self._leading_comment_remover = re.compile(r"^/\*.*?\*/") @@ -218,6 +230,32 @@ def _operation_name(self, db_name, statement): return self.vendor return " ".join(parts) + def _get_commenter_data(self, conn) -> dict: + """Calculate sqlcomment contents from conn and configured options""" + commenter_data = { + "db_driver": conn.engine.driver, + # Driver/framework centric information. + "db_framework": f"sqlalchemy:{sqlalchemy.__version__}", + } + + if self.commenter_options.get("opentelemetry_values", True): + commenter_data.update(**_get_opentelemetry_values()) + + # Filter down to just the requested attributes. + commenter_data = { + k: v + for k, v in commenter_data.items() + if self.commenter_options.get(k, True) + } + return commenter_data + + def _set_db_client_span_attributes(self, span, statement, attrs) -> None: + """Uses statement and attrs to set attributes of provided Otel span""" + span.set_attribute(SpanAttributes.DB_STATEMENT, statement) + span.set_attribute(SpanAttributes.DB_SYSTEM, self.vendor) + for key, value in attrs.items(): + span.set_attribute(key, value) + def _before_cur_exec( self, conn, cursor, statement, params, context, _executemany ): @@ -233,30 +271,30 @@ def _before_cur_exec( with trace.use_span(span, end_on_exit=False): if span.is_recording(): if self.enable_commenter: - commenter_data = { - "db_driver": conn.engine.driver, - # Driver/framework centric information. - "db_framework": f"sqlalchemy:{sqlalchemy.__version__}", - } - - if self.commenter_options.get( - "opentelemetry_values", True - ): - commenter_data.update(**_get_opentelemetry_values()) - - # Filter down to just the requested attributes. - commenter_data = { - k: v - for k, v in commenter_data.items() - if self.commenter_options.get(k, True) - } - - statement = _add_sql_comment(statement, **commenter_data) - - span.set_attribute(SpanAttributes.DB_STATEMENT, statement) - span.set_attribute(SpanAttributes.DB_SYSTEM, self.vendor) - for key, value in attrs.items(): - span.set_attribute(key, value) + commenter_data = self._get_commenter_data(conn) + + if self.enable_attribute_commenter: + # sqlcomment is added to executed query and db.statement span attribute + statement = _add_sql_comment( + statement, **commenter_data + ) + self._set_db_client_span_attributes( + span, statement, attrs + ) + + else: + # sqlcomment is only added to executed query + # so db.statement is set before add_sql_comment + self._set_db_client_span_attributes( + span, statement, attrs + ) + statement = _add_sql_comment( + statement, **commenter_data + ) + + else: + # no sqlcomment anywhere + self._set_db_client_span_attributes(span, statement, attrs) context._otel_span = span diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index 27a253decb..8f5d0f2a94 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -209,6 +209,44 @@ def test_create_engine_wrapper_enable_commenter(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_create_engine_wrapper_enable_commenter_stmt_enabled(self): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={"db_framework": False}, + enable_attribute_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + # sqlcommenter + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_create_engine_wrapper_enable_commenter_otel_values_false(self): logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) @@ -229,6 +267,49 @@ def test_create_engine_wrapper_enable_commenter_otel_values_false(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_create_engine_wrapper_enable_commenter_stmt_enabled_otel_values_false( + self, + ): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={ + "db_framework": False, + "opentelemetry_values": False, + }, + enable_attribute_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + # sqlcommenter + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) def test_custom_tracer_provider(self): provider = TracerProvider( @@ -321,6 +402,55 @@ async def run(): self.caplog.records[1].getMessage(), r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + asyncio.get_event_loop().run_until_complete(run()) + + @pytest.mark.skipif( + not sqlalchemy.__version__.startswith("1.4"), + reason="only run async tests for 1.4", + ) + def test_create_async_engine_wrapper_enable_commenter_stmt_enabled(self): + async def run(): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={ + "db_framework": False, + }, + enable_attribute_commenter=True, + ) + from sqlalchemy.ext.asyncio import ( # pylint: disable-all + create_async_engine, + ) + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.connect() as cnx: + await cnx.execute(text("SELECT 1;")) + # sqlcommenter + self.assertRegex( + self.caplog.records[1].getMessage(), + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) asyncio.get_event_loop().run_until_complete(run()) @@ -352,6 +482,58 @@ async def run(): self.caplog.records[1].getMessage(), r"SELECT 1 /\*db_driver='(.*)'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + asyncio.get_event_loop().run_until_complete(run()) + + @pytest.mark.skipif( + not sqlalchemy.__version__.startswith("1.4"), + reason="only run async tests for 1.4", + ) + def test_create_async_engine_wrapper_enable_commenter_stmt_enabled_otel_values_false( + self, + ): + async def run(): + logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) + SQLAlchemyInstrumentor().instrument( + enable_commenter=True, + commenter_options={ + "db_framework": False, + "opentelemetry_values": False, + }, + enable_attribute_commenter=True, + ) + from sqlalchemy.ext.asyncio import ( # pylint: disable-all + create_async_engine, + ) + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with engine.connect() as cnx: + await cnx.execute(text("SELECT 1;")) + # sqlcommenter + self.assertRegex( + self.caplog.records[1].getMessage(), + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)'*/;", + ) asyncio.get_event_loop().run_until_complete(run()) diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py index 8490721e3e..d8144dadc1 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlcommenter.py @@ -61,13 +61,97 @@ def test_sqlcommenter_enabled(self): r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) - def test_sqlcommenter_enabled_matches_db_statement_attribute(self): + def test_sqlcommenter_default_stmt_enabled_no_comments_anywhere(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + # enable_commenter not set + enable_attribute_commenter=True, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + query_log = self.caplog.records[-2].getMessage() + self.assertEqual( + query_log, + "SELECT 1;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_disabled_stmt_enabled_no_comments_anywhere(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=False, + enable_attribute_commenter=True, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + query_log = self.caplog.records[-2].getMessage() + self.assertEqual( + query_log, + "SELECT 1;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_disabled_default( + self, + ): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=True, + commenter_options={"db_framework": False}, + # enable_attribute_commenter not set + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + query_log = self.caplog.records[-2].getMessage() + self.assertRegex( + query_log, + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_enabled_matches_db_statement_attribute( + self, + ): engine = create_engine("sqlite:///:memory:") SQLAlchemyInstrumentor().instrument( engine=engine, tracer_provider=self.tracer_provider, enable_commenter=True, commenter_options={"db_framework": False}, + enable_attribute_commenter=True, ) cnx = engine.connect() cnx.execute(text("SELECT 1;")).fetchall() @@ -110,6 +194,45 @@ def test_sqlcommenter_enabled_otel_values_false(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_enabled_otel_values_false(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=True, + commenter_options={ + "db_framework": False, + "opentelemetry_values": False, + }, + enable_attribute_commenter=True, + ) + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)'*/;", + ) def test_sqlcommenter_flask_integration(self): engine = create_engine("sqlite:///:memory:") @@ -132,6 +255,49 @@ def test_sqlcommenter_flask_integration(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)',flask=1,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_stmt_enabled_flask_integration(self): + engine = create_engine("sqlite:///:memory:") + SQLAlchemyInstrumentor().instrument( + engine=engine, + tracer_provider=self.tracer_provider, + enable_commenter=True, + commenter_options={"db_framework": False}, + enable_attribute_commenter=True, + ) + cnx = engine.connect() + + current_context = context.get_current() + sqlcommenter_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {"flask": 1}, current_context + ) + context.attach(sqlcommenter_context) + + cnx.execute(text("SELECT 1;")).fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)',flask=1,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)',flask=1,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_sqlcommenter_enabled_create_engine_after_instrumentation(self): SQLAlchemyInstrumentor().instrument( @@ -147,6 +313,44 @@ def test_sqlcommenter_enabled_create_engine_after_instrumentation(self): self.caplog.records[-2].getMessage(), r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertEqual( + query_span.attributes[SpanAttributes.DB_STATEMENT], + "SELECT 1;", + ) + + def test_sqlcommenter_enabled_stmt_enabled_create_engine_after_instrumentation( + self, + ): + SQLAlchemyInstrumentor().instrument( + tracer_provider=self.tracer_provider, + enable_commenter=True, + enable_attribute_commenter=True, + ) + from sqlalchemy import create_engine # pylint: disable-all + + engine = create_engine("sqlite:///:memory:") + cnx = engine.connect() + cnx.execute(text("SELECT 1;")).fetchall() + self.assertRegex( + self.caplog.records[-2].getMessage(), + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + # first span is connection to db + self.assertEqual(spans[0].name, "connect") + # second span is query itself + query_span = spans[1] + self.assertRegex( + query_span.attributes[SpanAttributes.DB_STATEMENT], + r"SELECT 1 /\*db_driver='(.*)',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_sqlcommenter_disabled_create_engine_after_instrumentation(self): SQLAlchemyInstrumentor().instrument( From cf6d45e96cd1ea55fca2ff43dc37f0d5c8c878fc Mon Sep 17 00:00:00 2001 From: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:08:46 -0800 Subject: [PATCH 09/13] DB-API: db.statement inclusion of sqlcomment as opt-in (#3115) --- CHANGELOG.md | 6 + .../instrumentation/dbapi/__init__.py | 112 +++-- .../tests/test_dbapi_integration.py | 406 +++++++++++++++++- 3 files changed, 479 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d87091a2bb..47965fc581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-sqlalchemy` including sqlcomment in `db.statement` span attribute value is now opt-in ([#3112](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3112)) +### Breaking changes + +- `opentelemetry-instrumentation-dbapi` including sqlcomment in `db.statement` span attribute value is now opt-in + ([#3115](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3115)) + + ## Version 1.29.0/0.50b0 (2024-12-11) ### Added diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index d8db967f47..9b709c584c 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -71,6 +71,7 @@ def trace_integration( capture_parameters: bool = False, enable_commenter: bool = False, db_api_integration_factory=None, + enable_attribute_commenter: bool = False, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -88,6 +89,7 @@ def trace_integration( enable_commenter: Flag to enable/disable sqlcommenter. db_api_integration_factory: The `DatabaseApiIntegration` to use. If none is passed the default one is used. + enable_attribute_commenter: Flag to enable/disable sqlcomment inclusion in `db.statement` span attribute. Only available if enable_commenter=True. """ wrap_connect( __name__, @@ -100,6 +102,7 @@ def trace_integration( capture_parameters=capture_parameters, enable_commenter=enable_commenter, db_api_integration_factory=db_api_integration_factory, + enable_attribute_commenter=enable_attribute_commenter, ) @@ -115,6 +118,7 @@ def wrap_connect( enable_commenter: bool = False, db_api_integration_factory=None, commenter_options: dict = None, + enable_attribute_commenter: bool = False, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -133,6 +137,7 @@ def wrap_connect( db_api_integration_factory: The `DatabaseApiIntegration` to use. If none is passed the default one is used. commenter_options: Configurations for tags to be appended at the sql query. + enable_attribute_commenter: Flag to enable/disable sqlcomment inclusion in `db.statement` span attribute. Only available if enable_commenter=True. """ db_api_integration_factory = ( @@ -156,6 +161,7 @@ def wrap_connect_( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=connect_module, + enable_attribute_commenter=enable_attribute_commenter, ) return db_integration.wrapped_connection(wrapped, args, kwargs) @@ -191,6 +197,7 @@ def instrument_connection( enable_commenter: bool = False, commenter_options: dict = None, connect_module: typing.Callable[..., typing.Any] = None, + enable_attribute_commenter: bool = False, ): """Enable instrumentation in a database connection. @@ -206,6 +213,7 @@ def instrument_connection( enable_commenter: Flag to enable/disable sqlcommenter. commenter_options: Configurations for tags to be appended at the sql query. connect_module: Module name where connect method is available. + enable_attribute_commenter: Flag to enable/disable sqlcomment inclusion in `db.statement` span attribute. Only available if enable_commenter=True. Returns: An instrumented connection. @@ -224,6 +232,7 @@ def instrument_connection( enable_commenter=enable_commenter, commenter_options=commenter_options, connect_module=connect_module, + enable_attribute_commenter=enable_attribute_commenter, ) db_integration.get_connection_attributes(connection) return get_traced_connection_proxy(connection, db_integration) @@ -257,6 +266,7 @@ def __init__( enable_commenter: bool = False, commenter_options: dict = None, connect_module: typing.Callable[..., typing.Any] = None, + enable_attribute_commenter: bool = False, ): self.connection_attributes = connection_attributes if self.connection_attributes is None: @@ -277,6 +287,7 @@ def __init__( self.capture_parameters = capture_parameters self.enable_commenter = enable_commenter self.commenter_options = commenter_options + self.enable_attribute_commenter = enable_attribute_commenter self.database_system = database_system self.connection_props = {} self.span_attributes = {} @@ -434,9 +445,52 @@ def __init__(self, db_api_integration: DatabaseApiIntegration) -> None: if self._db_api_integration.commenter_options else {} ) + self._enable_attribute_commenter = ( + self._db_api_integration.enable_attribute_commenter + ) self._connect_module = self._db_api_integration.connect_module self._leading_comment_remover = re.compile(r"^/\*.*?\*/") + def _capture_mysql_version(self, cursor) -> None: + """Lazy capture of mysql-connector client version using cursor, if applicable""" + if ( + self._db_api_integration.database_system == "mysql" + and self._db_api_integration.connect_module.__name__ + == "mysql.connector" + and not self._db_api_integration.commenter_data[ + "mysql_client_version" + ] + ): + self._db_api_integration.commenter_data["mysql_client_version"] = ( + cursor._cnx._cmysql.get_client_info() + ) + + def _get_commenter_data(self) -> dict: + """Uses DB-API integration to return commenter data for sqlcomment""" + commenter_data = dict(self._db_api_integration.commenter_data) + if self._commenter_options.get("opentelemetry_values", True): + commenter_data.update(**_get_opentelemetry_values()) + return { + k: v + for k, v in commenter_data.items() + if self._commenter_options.get(k, True) + } + + def _update_args_with_added_sql_comment(self, args, cursor) -> tuple: + """Updates args with cursor info and adds sqlcomment to query statement""" + try: + args_list = list(args) + self._capture_mysql_version(cursor) + commenter_data = self._get_commenter_data() + statement = _add_sql_comment(args_list[0], **commenter_data) + args_list[0] = statement + args = tuple(args_list) + except Exception as exc: # pylint: disable=broad-except + _logger.exception( + "Exception while generating sql comment: %s", exc + ) + return args + def _populate_span( self, span: trace_api.Span, @@ -497,52 +551,22 @@ def traced_execution( ) as span: if span.is_recording(): if args and self._commenter_enabled: - try: - args_list = list(args) - - # lazy capture of mysql-connector client version using cursor - if ( - self._db_api_integration.database_system == "mysql" - and self._db_api_integration.connect_module.__name__ - == "mysql.connector" - and not self._db_api_integration.commenter_data[ - "mysql_client_version" - ] - ): - self._db_api_integration.commenter_data[ - "mysql_client_version" - ] = cursor._cnx._cmysql.get_client_info() - - commenter_data = dict( - self._db_api_integration.commenter_data - ) - if self._commenter_options.get( - "opentelemetry_values", True - ): - commenter_data.update( - **_get_opentelemetry_values() - ) - - # Filter down to just the requested attributes. - commenter_data = { - k: v - for k, v in commenter_data.items() - if self._commenter_options.get(k, True) - } - statement = _add_sql_comment( - args_list[0], **commenter_data + if self._enable_attribute_commenter: + # sqlcomment is added to executed query and db.statement span attribute + args = self._update_args_with_added_sql_comment( + args, cursor ) - - args_list[0] = statement - args = tuple(args_list) - - except Exception as exc: # pylint: disable=broad-except - _logger.exception( - "Exception while generating sql comment: %s", exc + self._populate_span(span, cursor, *args) + else: + # sqlcomment is only added to executed query + # so db.statement is set before add_sql_comment + self._populate_span(span, cursor, *args) + args = self._update_args_with_added_sql_comment( + args, cursor ) - - self._populate_span(span, cursor, *args) - + else: + # no sqlcomment anywhere + self._populate_span(span, cursor, *args) return query_method(*args, **kwargs) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 2ffa2f3d5b..3d531fb791 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=too-many-lines import logging import re @@ -277,6 +278,47 @@ def test_executemany_comment(self): cursor.query, r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = mock.MagicMock() + connect_module.__libpq_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": False, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_executemany_comment_non_pep_249_compliant(self): class MockConnectModule: @@ -306,8 +348,54 @@ def __getattr__(self, name): cursor.query, r"Select 1 /\*dbapi_level='1.0',dbapi_threadsafety='unknown',driver_paramstyle='unknown',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) - def test_executemany_comment_matches_db_statement_attribute(self): + def test_executemany_comment_non_pep_249_compliant_stmt_enabled(self): + class MockConnectModule: + def __getattr__(self, name): + if name == "__name__": + return "test" + if name == "__version__": + return mock.MagicMock() + if name == "__libpq_version__": + return 123 + raise AttributeError("attribute missing") + + connect_module = MockConnectModule() + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + connect_module=connect_module, + commenter_options={"db_driver": False}, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*dbapi_level='1.0',dbapi_threadsafety='unknown',driver_paramstyle='unknown',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*dbapi_level='1.0',dbapi_threadsafety='unknown',driver_paramstyle='unknown',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + + def test_executemany_comment_stmt_enabled_matches_db_statement_attribute( + self, + ): connect_module = mock.MagicMock() connect_module.__version__ = mock.MagicMock() connect_module.__libpq_version__ = 123 @@ -321,6 +409,7 @@ def test_executemany_comment_matches_db_statement_attribute(self): enable_commenter=True, commenter_options={"db_driver": False, "dbapi_level": False}, connect_module=connect_module, + enable_attribute_commenter=True, ) mock_connection = db_integration.wrapped_connection( mock_connect, {}, {} @@ -371,6 +460,50 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self): cursor.query, r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_compatible_build_version_psycopg_psycopg2_libpq_stmt_enabled( + self, + ): + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = mock.MagicMock() + connect_module.pq = mock.MagicMock() + connect_module.pq.__build_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": False, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_executemany_psycopg2_integration_comment(self): connect_module = mock.MagicMock() @@ -397,6 +530,47 @@ def test_executemany_psycopg2_integration_comment(self): cursor.query, r"Select 1 /\*db_driver='psycopg2%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_psycopg2_integration_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "psycopg2" + connect_module.__version__ = "1.2.3" + connect_module.__libpq_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='psycopg2%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*db_driver='psycopg2%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_executemany_psycopg_integration_comment(self): connect_module = mock.MagicMock() @@ -424,6 +598,48 @@ def test_executemany_psycopg_integration_comment(self): cursor.query, r"Select 1 /\*db_driver='psycopg%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_psycopg_integration_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "psycopg" + connect_module.__version__ = "1.2.3" + connect_module.pq = mock.MagicMock() + connect_module.pq.__build_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='psycopg%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*db_driver='psycopg%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_executemany_mysqlconnector_integration_comment(self): connect_module = mock.MagicMock() @@ -450,6 +666,47 @@ def test_executemany_mysqlconnector_integration_comment(self): cursor.query, r"Select 1 /\*db_driver='mysql.connector%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='1.2.3',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_mysqlconnector_integration_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "mysql.connector" + connect_module.__version__ = "1.2.3" + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "mysql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='mysql.connector%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='1.2.3',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*db_driver='mysql.connector%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='1.2.3',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) @mock.patch("opentelemetry.instrumentation.dbapi.util_version") def test_executemany_mysqlclient_integration_comment( @@ -485,6 +742,56 @@ def test_executemany_mysqlclient_integration_comment( cursor.query, r"Select 1 /\*db_driver='MySQLdb%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + @mock.patch("opentelemetry.instrumentation.dbapi.util_version") + def test_executemany_mysqlclient_integration_comment_stmt_enabled( + self, + mock_dbapi_util_version, + ): + mock_dbapi_util_version.return_value = "1.2.3" + connect_module = mock.MagicMock() + connect_module.__name__ = "MySQLdb" + connect_module.__version__ = "1.2.3" + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + connect_module._mysql = mock.MagicMock() + connect_module._mysql.get_client_info = mock.MagicMock( + return_value="123" + ) + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "mysql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='MySQLdb%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*db_driver='MySQLdb%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_executemany_pymysql_integration_comment(self): connect_module = mock.MagicMock() @@ -512,6 +819,48 @@ def test_executemany_pymysql_integration_comment(self): cursor.query, r"Select 1 /\*db_driver='pymysql%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + def test_executemany_pymysql_integration_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "pymysql" + connect_module.__version__ = "1.2.3" + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + connect_module.get_client_info = mock.MagicMock(return_value="123") + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "mysql", + enable_commenter=True, + commenter_options={"db_driver": True, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*db_driver='pymysql%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*db_driver='pymysql%%3A1.2.3',dbapi_threadsafety=123,driver_paramstyle='test',mysql_client_version='123',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) def test_executemany_flask_integration_comment(self): connect_module = mock.MagicMock() @@ -544,6 +893,58 @@ def test_executemany_flask_integration_comment(self): cursor.query, r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',flask=1,libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertEqual( + span.attributes[SpanAttributes.DB_STATEMENT], + "Select 1;", + ) + + clear_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {}, current_context + ) + context.attach(clear_context) + + def test_executemany_flask_integration_comment_stmt_enabled(self): + connect_module = mock.MagicMock() + connect_module.__name__ = "test" + connect_module.__version__ = mock.MagicMock() + connect_module.__libpq_version__ = 123 + connect_module.apilevel = 123 + connect_module.threadsafety = 123 + connect_module.paramstyle = "test" + + db_integration = dbapi.DatabaseApiIntegration( + "testname", + "postgresql", + enable_commenter=True, + commenter_options={"db_driver": False, "dbapi_level": False}, + connect_module=connect_module, + enable_attribute_commenter=True, + ) + current_context = context.get_current() + sqlcommenter_context = context.set_value( + "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {"flask": 1}, current_context + ) + context.attach(sqlcommenter_context) + + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, {} + ) + cursor = mock_connection.cursor() + cursor.executemany("Select 1;") + self.assertRegex( + cursor.query, + r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',flask=1,libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + self.assertRegex( + span.attributes[SpanAttributes.DB_STATEMENT], + r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',flask=1,libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;", + ) clear_context = context.set_value( "SQLCOMMENTER_ORM_TAGS_AND_VALUES", {}, current_context @@ -603,6 +1004,7 @@ def test_instrument_connection_kwargs_defaults(self, mock_dbapiint): self.assertEqual(kwargs["enable_commenter"], False) self.assertEqual(kwargs["commenter_options"], None) self.assertEqual(kwargs["connect_module"], None) + self.assertEqual(kwargs["enable_attribute_commenter"], False) @mock.patch("opentelemetry.instrumentation.dbapi.DatabaseApiIntegration") def test_instrument_connection_kwargs_provided(self, mock_dbapiint): @@ -619,6 +1021,7 @@ def test_instrument_connection_kwargs_provided(self, mock_dbapiint): enable_commenter=True, commenter_options={"foo": "bar"}, connect_module=mock_connect_module, + enable_attribute_commenter=True, ) kwargs = mock_dbapiint.call_args[1] self.assertEqual(kwargs["connection_attributes"], {"foo": "bar"}) @@ -628,6 +1031,7 @@ def test_instrument_connection_kwargs_provided(self, mock_dbapiint): self.assertEqual(kwargs["enable_commenter"], True) self.assertEqual(kwargs["commenter_options"], {"foo": "bar"}) self.assertIs(kwargs["connect_module"], mock_connect_module) + self.assertEqual(kwargs["enable_attribute_commenter"], True) def test_uninstrument_connection(self): connection = mock.Mock() From 9af3136e7fe21af88499621512cb22a77ee9b489 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 9 Jan 2025 22:21:19 +0100 Subject: [PATCH 10/13] Support PEP 561 to `opentelemetry-util-http` (#3127) --- CHANGELOG.md | 2 + .../src/opentelemetry/util/http/__init__.py | 14 +++---- .../src/opentelemetry/util/http/httplib.py | 42 +++++++++++++------ .../src/opentelemetry/util/http/py.typed | 0 4 files changed, 37 insertions(+), 21 deletions(-) create mode 100644 util/opentelemetry-util-http/src/opentelemetry/util/http/py.typed diff --git a/CHANGELOG.md b/CHANGELOG.md index 47965fc581..9a35e342df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3148](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3148)) - add support to Python 3.13 ([#3134](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3134)) +- `opentelemetry-util-http` Add `py.typed` file to enable PEP 561 + ([#3127](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3127)) ### Fixed diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index f5dacf0fff..c7dd9f7b06 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -19,7 +19,7 @@ from re import IGNORECASE as RE_IGNORECASE from re import compile as re_compile from re import search -from typing import Callable, Iterable, Optional +from typing import Callable, Iterable from urllib.parse import urlparse, urlunparse from opentelemetry.semconv.trace import SpanAttributes @@ -121,18 +121,16 @@ def sanitize_header_values( _root = r"OTEL_PYTHON_{}" -def get_traced_request_attrs(instrumentation): +def get_traced_request_attrs(instrumentation: str) -> list[str]: traced_request_attrs = environ.get( - _root.format(f"{instrumentation}_TRACED_REQUEST_ATTRS"), [] + _root.format(f"{instrumentation}_TRACED_REQUEST_ATTRS") ) - if traced_request_attrs: - traced_request_attrs = [ + return [ traced_request_attr.strip() for traced_request_attr in traced_request_attrs.split(",") ] - - return traced_request_attrs + return [] def get_excluded_urls(instrumentation: str) -> ExcludeList: @@ -193,7 +191,7 @@ def normalise_response_header_name(header: str) -> str: return f"http.response.header.{key}" -def sanitize_method(method: Optional[str]) -> Optional[str]: +def sanitize_method(method: str | None) -> str | None: if method is None: return None method = method.upper() diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py index 3d6b875752..f375e2f7c8 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py @@ -17,12 +17,14 @@ not create spans on its own. """ +from __future__ import annotations + import contextlib import http.client import logging import socket # pylint:disable=unused-import # Used for typing import typing -from typing import Collection +from typing import Any, Callable, Collection, TypedDict, cast import wrapt @@ -36,20 +38,22 @@ logger = logging.getLogger(__name__) +R = typing.TypeVar("R") + class HttpClientInstrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return () # This instruments http.client from stdlib; no extra deps. - def _instrument(self, **kwargs): + def _instrument(self, **kwargs: Any): """Instruments the http.client module (not creating spans on its own)""" _instrument() - def _uninstrument(self, **kwargs): + def _uninstrument(self, **kwargs: Any): _uninstrument() -def _remove_nonrecording(spanlist: typing.List[Span]): +def _remove_nonrecording(spanlist: list[Span]) -> bool: idx = len(spanlist) - 1 while idx >= 0: if not spanlist[idx].is_recording(): @@ -67,7 +71,9 @@ def _remove_nonrecording(spanlist: typing.List[Span]): return True -def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: +def trysetip( + conn: http.client.HTTPConnection, loglevel: int = logging.DEBUG +) -> bool: """Tries to set the net.peer.ip semantic attribute on the current span from the given HttpConnection. @@ -110,14 +116,17 @@ def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: def _instrumented_connect( - wrapped, instance: http.client.HTTPConnection, args, kwargs -): + wrapped: Callable[..., R], + instance: http.client.HTTPConnection, + args: tuple[Any, ...], + kwargs: dict[str, Any], +) -> R: result = wrapped(*args, **kwargs) trysetip(instance, loglevel=logging.WARNING) return result -def instrument_connect(module, name="connect"): +def instrument_connect(module: type[Any], name: str = "connect"): """Instrument additional connect() methods, e.g. for derived classes.""" wrapt.wrap_function_wrapper( @@ -129,8 +138,11 @@ def instrument_connect(module, name="connect"): def _instrument(): def instrumented_send( - wrapped, instance: http.client.HTTPConnection, args, kwargs - ): + wrapped: Callable[..., R], + instance: http.client.HTTPConnection, + args: tuple[Any, ...], + kwargs: dict[str, Any], + ) -> R: done = trysetip(instance) result = wrapped(*args, **kwargs) if not done: @@ -147,8 +159,12 @@ def instrumented_send( # No need to instrument HTTPSConnection, as it calls super().connect() -def _getstate() -> typing.Optional[dict]: - return context.get_value(_STATE_KEY) +class _ConnectionState(TypedDict): + need_ip: list[Span] + + +def _getstate() -> _ConnectionState | None: + return cast(_ConnectionState, context.get_value(_STATE_KEY)) @contextlib.contextmanager @@ -163,7 +179,7 @@ def set_ip_on_next_http_connection(span: Span): finally: context.detach(token) else: - spans: typing.List[Span] = state["need_ip"] + spans = state["need_ip"] spans.append(span) try: yield diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/py.typed b/util/opentelemetry-util-http/src/opentelemetry/util/http/py.typed new file mode 100644 index 0000000000..e69de29bb2 From 41e670aeee5c34c5104fa80aeb97a81976d19a5c Mon Sep 17 00:00:00 2001 From: Guspan Tanadi <36249910+guspan-tanadi@users.noreply.github.com> Date: Fri, 10 Jan 2025 04:58:53 +0700 Subject: [PATCH 11/13] docs(README): semantic convention migration links (#3160) --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8582c472ba..2400e5d47a 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ The Python auto-instrumentation libraries for [OpenTelemetry](https://openteleme * [Installation](#installation) * [Releasing](#releasing) * [Releasing a package as `1.0` stable](#releasing-a-package-as-10-stable) +* [Semantic Convention status of instrumentations](#semantic-convention-status-of-instrumentations) * [Contributing](#contributing) * [Thanks to all the people who already contributed](#thanks-to-all-the-people-who-already-contributed) @@ -100,7 +101,7 @@ To release a package as `1.0` stable, the package: ## Semantic Convention status of instrumentations -In our efforts to maintain optimal user experience and prevent breaking changes for transitioning into stable semantic conventions, OpenTelemetry Python is adopting the [semantic convention migration plan](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/migration-guide.md) for several instrumentations. Currently this plan is only being adopted for HTTP-related instrumentations, but will eventually cover all types. Please refer to the `semconv status` column of the [instrumentation README](instrumentation/README.md) of the current status of instrumentations' semantic conventions. The possible values are `experimental`, `stable` and `migration` referring to [status](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.31.0/specification/document-status.md#lifecycle-status) of that particular semantic convention. `Migration` refers to an instrumentation that currently supports the migration plan. +In our efforts to maintain optimal user experience and prevent breaking changes for transitioning into stable semantic conventions, OpenTelemetry Python is adopting the semantic convention migration plan for several instrumentations. Currently this plan is only being adopted for [HTTP-related instrumentations](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/http-migration.md), but will eventually cover all types. Please refer to the `semconv status` column of the [instrumentation README](instrumentation/README.md) of the current status of instrumentations' semantic conventions. The possible values are `experimental`, `stable` and `migration` referring to [status](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.31.0/specification/document-status.md#lifecycle-status) of that particular semantic convention. `Migration` refers to an instrumentation that currently supports the migration plan. ## Contributing From 962a3aecdb3cd99bc0590303106be27c098a71f8 Mon Sep 17 00:00:00 2001 From: Will Li Date: Fri, 10 Jan 2025 06:51:23 +0800 Subject: [PATCH 12/13] fix #991 audit asyncpg instrumentation with nooptracerprovider (#3144) --- .../tests/test_asyncpg_wrapper.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py b/instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py index 7c88b9c005..0fc44d6a23 100644 --- a/instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py +++ b/instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py @@ -5,6 +5,7 @@ from asyncpg import Connection, Record, cursor from wrapt import ObjectProxy +from opentelemetry import trace as trace_api from opentelemetry.instrumentation.asyncpg import AsyncPGInstrumentor from opentelemetry.test.test_base import TestBase @@ -105,3 +106,36 @@ async def exec_mock(*args, **kwargs): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) self.assertEqual([span.status.is_ok for span in spans], [True, True]) + + def test_no_op_tracer_provider(self): + AsyncPGInstrumentor().uninstrument() + AsyncPGInstrumentor().instrument( + tracer_provider=trace_api.NoOpTracerProvider() + ) + + # Mock out all interaction with postgres + async def bind_mock(*args, **kwargs): + return [] + + async def exec_mock(*args, **kwargs): + return [], None, True + + conn = mock.Mock() + conn.is_closed = lambda: False + + conn._protocol = mock.Mock() + conn._protocol.bind = bind_mock + conn._protocol.execute = exec_mock + conn._protocol.bind_execute = exec_mock + conn._protocol.close_portal = bind_mock + + state = mock.Mock() + state.closed = False + + # init the cursor and fetch a single record + crs = cursor.Cursor(conn, "SELECT * FROM test", state, [], Record) + asyncio.run(crs._init(1)) + asyncio.run(crs.fetch(1)) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) From 0ad779a5b3d817111fc65099d1b849995365f4ee Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 10 Jan 2025 02:42:12 +0100 Subject: [PATCH 13/13] Support PEP 561 to `opentelemetry-instrumentation-jinja2` (#3137) --- .../instrumentation/jinja2/__init__.py | 57 +++++++++++++++---- .../instrumentation/jinja2/py.typed | 0 2 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/py.typed diff --git a/instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/__init__.py b/instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/__init__.py index 0b199cbe64..9867992d49 100644 --- a/instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/__init__.py @@ -39,17 +39,21 @@ """ # pylint: disable=no-value-for-parameter +from __future__ import annotations + import logging -from typing import Collection +from types import CodeType +from typing import Any, Callable, Collection, TypeVar import jinja2 +from jinja2.environment import Template from wrapt import wrap_function_wrapper as _wrap from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.jinja2.package import _instruments from opentelemetry.instrumentation.jinja2.version import __version__ from opentelemetry.instrumentation.utils import unwrap -from opentelemetry.trace import SpanKind, get_tracer +from opentelemetry.trace import SpanKind, Tracer, get_tracer logger = logging.getLogger(__name__) @@ -57,12 +61,27 @@ ATTRIBUTE_JINJA2_TEMPLATE_PATH = "jinja2.template_path" DEFAULT_TEMPLATE_NAME = "" +R = TypeVar("R") + -def _with_tracer_wrapper(func): +def _with_tracer_wrapper( + func: Callable[ + [Tracer, Callable[..., R], Any, list[Any], dict[str, Any]], R + ], +) -> Callable[ + [Tracer], Callable[[Callable[..., R], Any, list[Any], dict[str, Any]], R] +]: """Helper for providing tracer for wrapper functions.""" - def _with_tracer(tracer): - def wrapper(wrapped, instance, args, kwargs): + def _with_tracer( + tracer: Tracer, + ) -> Callable[[Callable[..., R], Any, list[Any], dict[str, Any]], R]: + def wrapper( + wrapped: Callable[..., R], + instance: Any, + args: list[Any], + kwargs: dict[str, Any], + ) -> R: return func(tracer, wrapped, instance, args, kwargs) return wrapper @@ -71,7 +90,13 @@ def wrapper(wrapped, instance, args, kwargs): @_with_tracer_wrapper -def _wrap_render(tracer, wrapped, instance, args, kwargs): +def _wrap_render( + tracer: Tracer, + wrapped: Callable[..., Any], + instance: Template, + args: list[Any], + kwargs: dict[str, Any], +): """Wrap `Template.render()` or `Template.generate()`""" with tracer.start_as_current_span( "jinja2.render", @@ -84,7 +109,13 @@ def _wrap_render(tracer, wrapped, instance, args, kwargs): @_with_tracer_wrapper -def _wrap_compile(tracer, wrapped, _, args, kwargs): +def _wrap_compile( + tracer: Tracer, + wrapped: Callable[..., CodeType], + _, + args: list[Any], + kwargs: dict[str, Any], +) -> CodeType: with tracer.start_as_current_span( "jinja2.compile", kind=SpanKind.INTERNAL, @@ -100,7 +131,13 @@ def _wrap_compile(tracer, wrapped, _, args, kwargs): @_with_tracer_wrapper -def _wrap_load_template(tracer, wrapped, _, args, kwargs): +def _wrap_load_template( + tracer: Tracer, + wrapped: Callable[..., Template], + _, + args: list[Any], + kwargs: dict[str, Any], +) -> Template: with tracer.start_as_current_span( "jinja2.load", kind=SpanKind.INTERNAL, @@ -128,7 +165,7 @@ class Jinja2Instrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return _instruments - def _instrument(self, **kwargs): + def _instrument(self, **kwargs: Any): tracer_provider = kwargs.get("tracer_provider") tracer = get_tracer( __name__, @@ -146,7 +183,7 @@ def _instrument(self, **kwargs): _wrap_load_template(tracer), ) - def _uninstrument(self, **kwargs): + def _uninstrument(self, **kwargs: Any): unwrap(jinja2.Template, "render") unwrap(jinja2.Template, "generate") unwrap(jinja2.Environment, "compile") diff --git a/instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/py.typed b/instrumentation/opentelemetry-instrumentation-jinja2/src/opentelemetry/instrumentation/jinja2/py.typed new file mode 100644 index 0000000000..e69de29bb2