Skip to content

Commit 035930f

Browse files
authored
Merge branch 'main' into main
2 parents f4538f7 + f393546 commit 035930f

File tree

7 files changed

+58
-8
lines changed

7 files changed

+58
-8
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4646
([#3022](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3022))
4747
- Replace all instrumentor unit test `assertEqualSpanInstrumentationInfo` calls with `assertEqualSpanInstrumentationScope` calls
4848
([#3037](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3037))
49+
- `opentelemetry-instrumentation-sqlalchemy` Fixes engines from `sqlalchemy.engine_from_config` not being fully instrumented
50+
([#2816](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2816))
4951
- `opentelemetry-instrumentation-sqlalchemy`: Fix a remaining memory leak in EngineTracer
5052
([#3053](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3053))
5153

CONTRIBUTING.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,11 @@ Below is a checklist of things to be mindful of when implementing a new instrume
272272
- Isolate sync and async test
273273
- 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.
274274
- ex. <https://github.com/open-telemetry/opentelemetry-python-contrib/blob/60fb936b7e5371b3e5587074906c49fb873cbd76/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor.py#L84>
275-
- 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.
275+
- 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.
276+
- Documentation
277+
- 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)
278+
- Testing
279+
- When adding a new instrumentation remember to update `tox.ini` adding appropriate rules in `envlist`, `command_pre` and `commands` sections
276280

277281
## Guideline for GenAI instrumentations
278282

_template/autodoc_entry.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
.. include:: ../../../instrumentation/opentelemetry-instrumentation-<REPLACE ME>/README.rst
2+
:end-before: References
3+
4+
.. automodule:: opentelemetry.instrumentation.<REPLACE ME>
5+
:members:
6+
:undoc-members:
7+
:show-inheritance:

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,18 @@ def _instrument(self, **kwargs):
181181
tracer, connections_usage, enable_commenter, commenter_options
182182
),
183183
)
184+
# sqlalchemy.engine.create is not present in earlier versions of sqlalchemy (which we support)
185+
if parse_version(sqlalchemy.__version__).release >= (1, 4):
186+
_w(
187+
"sqlalchemy.engine.create",
188+
"create_engine",
189+
_wrap_create_engine(
190+
tracer,
191+
connections_usage,
192+
enable_commenter,
193+
commenter_options,
194+
),
195+
)
184196
_w(
185197
"sqlalchemy.engine.base",
186198
"Engine.connect",
@@ -224,6 +236,8 @@ def _instrument(self, **kwargs):
224236
def _uninstrument(self, **kwargs):
225237
unwrap(sqlalchemy, "create_engine")
226238
unwrap(sqlalchemy.engine, "create_engine")
239+
if parse_version(sqlalchemy.__version__).release >= (1, 4):
240+
unwrap(sqlalchemy.engine.create, "create_engine")
227241
unwrap(Engine, "connect")
228242
if parse_version(sqlalchemy.__version__).release >= (1, 4):
229243
unwrap(sqlalchemy.ext.asyncio, "create_async_engine")

instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,17 @@ def test_create_engine_wrapper(self):
182182
"opentelemetry.instrumentation.sqlalchemy",
183183
)
184184

185+
def test_instrument_engine_from_config(self):
186+
SQLAlchemyInstrumentor().instrument()
187+
from sqlalchemy import engine_from_config # pylint: disable-all
188+
189+
engine = engine_from_config({"sqlalchemy.url": "sqlite:///:memory:"})
190+
cnx = engine.connect()
191+
cnx.execute(text("SELECT 1 + 1;")).fetchall()
192+
spans = self.memory_exporter.get_finished_spans()
193+
194+
self.assertEqual(len(spans), 2)
195+
185196
def test_create_engine_wrapper_enable_commenter(self):
186197
logging.getLogger("sqlalchemy.engine").setLevel(logging.INFO)
187198
SQLAlchemyInstrumentor().instrument(

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,28 +39,35 @@
3939
---
4040
"""
4141

42+
from __future__ import annotations
43+
4244
import sqlite3
4345
from sqlite3 import dbapi2
44-
from typing import Collection
46+
from typing import Any, Collection, TypeVar, Union
4547

4648
from opentelemetry.instrumentation import dbapi
4749
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
4850
from opentelemetry.instrumentation.sqlite3.package import _instruments
4951
from opentelemetry.instrumentation.sqlite3.version import __version__
52+
from opentelemetry.trace import TracerProvider
5053

5154
# No useful attributes of sqlite3 connection object
5255
_CONNECTION_ATTRIBUTES = {}
5356

5457
_DATABASE_SYSTEM = "sqlite"
5558

59+
SQLite3Connection = TypeVar( # pylint: disable=invalid-name
60+
"SQLite3Connection", bound=Union[sqlite3.Connection, None]
61+
)
62+
5663

5764
class SQLite3Instrumentor(BaseInstrumentor):
5865
_TO_WRAP = [sqlite3, dbapi2]
5966

6067
def instrumentation_dependencies(self) -> Collection[str]:
6168
return _instruments
6269

63-
def _instrument(self, **kwargs):
70+
def _instrument(self, **kwargs: Any) -> None:
6471
"""Integrate with SQLite3 Python library.
6572
https://docs.python.org/3/library/sqlite3.html
6673
"""
@@ -77,13 +84,16 @@ def _instrument(self, **kwargs):
7784
tracer_provider=tracer_provider,
7885
)
7986

80-
def _uninstrument(self, **kwargs):
87+
def _uninstrument(self, **kwargs: Any) -> None:
8188
""" "Disable SQLite3 instrumentation"""
8289
for module in self._TO_WRAP:
8390
dbapi.unwrap_connect(module, "connect")
8491

8592
@staticmethod
86-
def instrument_connection(connection, tracer_provider=None):
93+
def instrument_connection(
94+
connection: SQLite3Connection,
95+
tracer_provider: TracerProvider | None = None,
96+
) -> SQLite3Connection:
8797
"""Enable instrumentation in a SQLite connection.
8898
8999
Args:
@@ -105,7 +115,9 @@ def instrument_connection(connection, tracer_provider=None):
105115
)
106116

107117
@staticmethod
108-
def uninstrument_connection(connection):
118+
def uninstrument_connection(
119+
connection: SQLite3Connection,
120+
) -> SQLite3Connection:
109121
"""Disable instrumentation in a SQLite connection.
110122
111123
Args:

instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/package.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14+
from __future__ import annotations
1415

15-
16-
_instruments = tuple()
16+
_instruments: tuple[str, ...] = tuple()

0 commit comments

Comments
 (0)