diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a1039f83a..33f83bb0ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3022](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3022)) - Replace all instrumentor unit test `assertEqualSpanInstrumentationInfo` calls with `assertEqualSpanInstrumentationScope` calls ([#3037](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3037)) +- `opentelemetry-instrumentation-sqlalchemy` Fixes engines from `sqlalchemy.engine_from_config` not being fully instrumented + ([#2816](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2816)) - `opentelemetry-instrumentation-sqlalchemy`: Fix a remaining memory leak in EngineTracer ([#3053](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3053)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 633a356c40..e90402edc2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -272,7 +272,11 @@ Below is a checklist of things to be mindful of when implementing a new instrume - Isolate sync and async test - For synchronous tests, the typical test case class is inherited from `opentelemetry.test.test_base.TestBase`. However, if you want to write asynchronous tests, the test case class should inherit also from `IsolatedAsyncioTestCase`. Adding asynchronous tests to a common test class can lead to tests passing without actually running, which can be misleading. - ex. -- All instrumentations have the same version. If you are going to develop a new instrumentation it would probably have `X.Y.dev` version and depends on `opentelemetry-instrumentation` and `opentelemetry-semantic-conventions` for the same version. That means that if you want to install your instrumentation you need to install its dependencies from this repo and the core repo also from git. +- Most of the instrumentations have the same version. If you are going to develop a new instrumentation it would probably have `X.Y.dev` version and depends on `opentelemetry-instrumentation` and `opentelemetry-semantic-conventions` for a [compatible version](https://peps.python.org/pep-0440/#compatible-release). That means that you may need to install the instrumentation dependencies from this repo and the core repo from git. +- Documentation + - When adding a new instrumentation remember to add an entry in `docs/instrumentation/` named `/.rst` to have the instrumentation documentation referenced from the index. You can use the entry template available [here](./_template/autodoc_entry.rst) +- Testing + - When adding a new instrumentation remember to update `tox.ini` adding appropriate rules in `envlist`, `command_pre` and `commands` sections ## Guideline for GenAI instrumentations diff --git a/_template/autodoc_entry.rst b/_template/autodoc_entry.rst new file mode 100644 index 0000000000..ee2688bacd --- /dev/null +++ b/_template/autodoc_entry.rst @@ -0,0 +1,7 @@ +.. include:: ../../../instrumentation/opentelemetry-instrumentation-/README.rst + :end-before: References + +.. automodule:: opentelemetry.instrumentation. + :members: + :undoc-members: + :show-inheritance: 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 9889e18b5a..4182c0034e 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/__init__.py @@ -181,6 +181,18 @@ def _instrument(self, **kwargs): tracer, connections_usage, enable_commenter, commenter_options ), ) + # sqlalchemy.engine.create is not present in earlier versions of sqlalchemy (which we support) + if parse_version(sqlalchemy.__version__).release >= (1, 4): + _w( + "sqlalchemy.engine.create", + "create_engine", + _wrap_create_engine( + tracer, + connections_usage, + enable_commenter, + commenter_options, + ), + ) _w( "sqlalchemy.engine.base", "Engine.connect", @@ -224,6 +236,8 @@ def _instrument(self, **kwargs): def _uninstrument(self, **kwargs): unwrap(sqlalchemy, "create_engine") unwrap(sqlalchemy.engine, "create_engine") + if parse_version(sqlalchemy.__version__).release >= (1, 4): + unwrap(sqlalchemy.engine.create, "create_engine") unwrap(Engine, "connect") if parse_version(sqlalchemy.__version__).release >= (1, 4): unwrap(sqlalchemy.ext.asyncio, "create_async_engine") diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py index 18b9fa65f7..27a253decb 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py +++ b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py @@ -182,6 +182,17 @@ def test_create_engine_wrapper(self): "opentelemetry.instrumentation.sqlalchemy", ) + def test_instrument_engine_from_config(self): + SQLAlchemyInstrumentor().instrument() + from sqlalchemy import engine_from_config # pylint: disable-all + + engine = engine_from_config({"sqlalchemy.url": "sqlite:///:memory:"}) + cnx = engine.connect() + cnx.execute(text("SELECT 1 + 1;")).fetchall() + spans = self.memory_exporter.get_finished_spans() + + self.assertEqual(len(spans), 2) + def test_create_engine_wrapper_enable_commenter(self): logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO) SQLAlchemyInstrumentor().instrument( diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py index 9b3ae28a95..1ec4794e96 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py @@ -39,20 +39,27 @@ --- """ +from __future__ import annotations + import sqlite3 from sqlite3 import dbapi2 -from typing import Collection +from typing import Any, Collection, TypeVar, Union from opentelemetry.instrumentation import dbapi from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.sqlite3.package import _instruments from opentelemetry.instrumentation.sqlite3.version import __version__ +from opentelemetry.trace import TracerProvider # No useful attributes of sqlite3 connection object _CONNECTION_ATTRIBUTES = {} _DATABASE_SYSTEM = "sqlite" +SQLite3Connection = TypeVar( # pylint: disable=invalid-name + "SQLite3Connection", bound=Union[sqlite3.Connection, None] +) + class SQLite3Instrumentor(BaseInstrumentor): _TO_WRAP = [sqlite3, dbapi2] @@ -60,7 +67,7 @@ class SQLite3Instrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return _instruments - def _instrument(self, **kwargs): + def _instrument(self, **kwargs: Any) -> None: """Integrate with SQLite3 Python library. https://docs.python.org/3/library/sqlite3.html """ @@ -77,13 +84,16 @@ def _instrument(self, **kwargs): tracer_provider=tracer_provider, ) - def _uninstrument(self, **kwargs): + def _uninstrument(self, **kwargs: Any) -> None: """ "Disable SQLite3 instrumentation""" for module in self._TO_WRAP: dbapi.unwrap_connect(module, "connect") @staticmethod - def instrument_connection(connection, tracer_provider=None): + def instrument_connection( + connection: SQLite3Connection, + tracer_provider: TracerProvider | None = None, + ) -> SQLite3Connection: """Enable instrumentation in a SQLite connection. Args: @@ -105,7 +115,9 @@ def instrument_connection(connection, tracer_provider=None): ) @staticmethod - def uninstrument_connection(connection): + def uninstrument_connection( + connection: SQLite3Connection, + ) -> SQLite3Connection: """Disable instrumentation in a SQLite connection. Args: diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/package.py b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/package.py index 7a66a17a93..0de6133b4a 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/package.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/package.py @@ -11,6 +11,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import annotations - -_instruments = tuple() +_instruments: tuple[str, ...] = tuple()