opentelemetry-python-contrib
opentelemetry-python-contrib copied to clipboard
Support SQLAlchemy 2 (in tests and doc strings)
Description
Fedora 40 will ship with SQLAlchemy 2. A python-sqlalchemy1.4 compat package will be available for now, but the idea is that this should be a temporary stopgap.
Currently, there are 17 test failures with SQLAlchemy 2.0.25, all similar to the one represented at the end of the PR text. This PR fixes them all by ensuring that plain-text query strings in the tests are always wrapped in sqlalchemy.text(). It also updates an example in a doc-string.
Note that no changes were required to the actual library implementation.
Fixes # (I did not file an issue.)
Type of change
It’s hard to say whether to characterize this as a bug fix, a new feature, or neither, since it primarily just expands compatibility in the tests. There is certainly no breaking change, and I don’t think it requires a documentation update.
How Has This Been Tested?
I applied this as a patch to the python-opentelemetry-contrib package and confirmed it fixed the test failures. You can test it by running the sqlalchemy instrumentation tests with SQLAlchemy 2.x in whatever way makes the most sense to you.
I could try adding testing with SQLAlchemy 2.x to tox.ini, but this is a little hard for me to test locally, especially the Docker part.
Does This PR Require a Core Repo Change?
- [ ] Yes. - Link to PR:
- [x] No.
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.
- [x] Followed the style guidelines of this project
- [ ] Changelogs have been updated No, should I?
- [ ] Unit tests have been added N/A, this fixes tests
- [ ] Documentation has been updated N/A
Sample test failure (before this PR):
___________ TestSqlalchemyInstrumentation.test_create_engine_wrapper ___________
self = <sqlalchemy.engine.base.Connection object at 0x7f87397c7c80>
statement = 'SELECT\t1 + 1;', parameters = None
def execute(
self,
statement: Executable,
parameters: Optional[_CoreAnyExecuteParams] = None,
*,
execution_options: Optional[CoreExecuteOptionsParameter] = None,
) -> CursorResult[Any]:
r"""Executes a SQL statement construct and returns a
:class:`_engine.CursorResult`.
:param statement: The statement to be executed. This is always
an object that is in both the :class:`_expression.ClauseElement` and
:class:`_expression.Executable` hierarchies, including:
* :class:`_expression.Select`
* :class:`_expression.Insert`, :class:`_expression.Update`,
:class:`_expression.Delete`
* :class:`_expression.TextClause` and
:class:`_expression.TextualSelect`
* :class:`_schema.DDL` and objects which inherit from
:class:`_schema.ExecutableDDLElement`
:param parameters: parameters which will be bound into the statement.
This may be either a dictionary of parameter names to values,
or a mutable sequence (e.g. a list) of dictionaries. When a
list of dictionaries is passed, the underlying statement execution
will make use of the DBAPI ``cursor.executemany()`` method.
When a single dictionary is passed, the DBAPI ``cursor.execute()``
method will be used.
:param execution_options: optional dictionary of execution options,
which will be associated with the statement execution. This
dictionary can provide a subset of the options that are accepted
by :meth:`_engine.Connection.execution_options`.
:return: a :class:`_engine.Result` object.
"""
distilled_parameters = _distill_params_20(parameters)
try:
> meth = statement._execute_on_connection
E AttributeError: 'str' object has no attribute '_execute_on_connection'
/usr/lib64/python3.12/site-packages/sqlalchemy/engine/base.py:1412: AttributeError
The above exception was the direct cause of the following exception:
self = <tests.test_sqlalchemy.TestSqlalchemyInstrumentation testMethod=test_create_engine_wrapper>
def test_create_engine_wrapper(self):
SQLAlchemyInstrumentor().instrument()
from sqlalchemy import create_engine # pylint: disable-all
engine = create_engine("sqlite:///:memory:")
cnx = engine.connect()
> cnx.execute("SELECT 1 + 1;").fetchall()
instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py:159:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <sqlalchemy.engine.base.Connection object at 0x7f87397c7c80>
statement = 'SELECT\t1 + 1;', parameters = None
def execute(
self,
statement: Executable,
parameters: Optional[_CoreAnyExecuteParams] = None,
*,
execution_options: Optional[CoreExecuteOptionsParameter] = None,
) -> CursorResult[Any]:
r"""Executes a SQL statement construct and returns a
:class:`_engine.CursorResult`.
:param statement: The statement to be executed. This is always
an object that is in both the :class:`_expression.ClauseElement` and
:class:`_expression.Executable` hierarchies, including:
* :class:`_expression.Select`
* :class:`_expression.Insert`, :class:`_expression.Update`,
:class:`_expression.Delete`
* :class:`_expression.TextClause` and
:class:`_expression.TextualSelect`
* :class:`_schema.DDL` and objects which inherit from
:class:`_schema.ExecutableDDLElement`
:param parameters: parameters which will be bound into the statement.
This may be either a dictionary of parameter names to values,
or a mutable sequence (e.g. a list) of dictionaries. When a
list of dictionaries is passed, the underlying statement execution
will make use of the DBAPI ``cursor.executemany()`` method.
When a single dictionary is passed, the DBAPI ``cursor.execute()``
method will be used.
:param execution_options: optional dictionary of execution options,
which will be associated with the statement execution. This
dictionary can provide a subset of the options that are accepted
by :meth:`_engine.Connection.execution_options`.
:return: a :class:`_engine.Result` object.
"""
distilled_parameters = _distill_params_20(parameters)
try:
meth = statement._execute_on_connection
except AttributeError as err:
> raise exc.ObjectNotExecutableError(statement) from err
E sqlalchemy.exc.ObjectNotExecutableError: Not an executable object: 'SELECT\t1 + 1;'
/usr/lib64/python3.12/site-packages/sqlalchemy/engine/base.py:1414: ObjectNotExecutableError
async tests should probably also be changed from 1.4 to 1.4 and 2+. E.g: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py#L283
async tests should probably also be changed from 1.4 to 1.4 and 2+. E.g: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py#L283
Thanks. That’s a great suggestion. I’m pushing an additional commit that enables these for “1.4 and later,” using packaging.version.parse() the same way it’s already used in
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/47caeab7aff47070c565cd90536a39ec5eb06f34/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/init.py#L101
and
https://github.com/open-telemetry/opentelemetry-python-contrib/blob/47caeab7aff47070c565cd90536a39ec5eb06f34/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/init.py#L188
Rebased on main without changes.