Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
emdneto authored Dec 5, 2024
2 parents f4538f7 + f393546 commit 035930f
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
6 changes: 5 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. <https://github.com/open-telemetry/opentelemetry-python-contrib/blob/60fb936b7e5371b3e5587074906c49fb873cbd76/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor.py#L84>
- 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 `<instrumentation>/<instrumentation>.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

Expand Down
7 changes: 7 additions & 0 deletions _template/autodoc_entry.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.. include:: ../../../instrumentation/opentelemetry-instrumentation-<REPLACE ME>/README.rst
:end-before: References

.. automodule:: opentelemetry.instrumentation.<REPLACE ME>
:members:
:undoc-members:
:show-inheritance:
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,35 @@
---
"""

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]

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
"""
Expand All @@ -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:
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit 035930f

Please sign in to comment.