open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
739 stars 613 forks source link

`Psycopg2Instrumentor().instrument_connection` raises `AttributeError: 'psycopg2.extensions.connection' object has no attribute '_is_instrumented_by_opentelemetry'` #2522

Open alexmojaki opened 6 months ago

alexmojaki commented 6 months ago

Describe your environment

opentelemetry-instrumentation-psycopg2==0.45b0
psycopg2-binary==2.9.9

Steps to reproduce

Use Psycopg2Instrumentor().instrument_connection. For example:

import psycopg2
from opentelemetry.instrumentation.psycopg2 import Psycopg2Instrumentor

connection = psycopg2.connect(database='database', user='user', password='secret', host='0.0.0.0', port=5432)

Psycopg2Instrumentor().instrument_connection(connection)

and run a database with:

docker run --name postgres \
    -e POSTGRES_USER=user \
    -e POSTGRES_PASSWORD=secret \
    -e POSTGRES_DB=database \
    -p 5432:5432 -d postgres

What is the actual behavior?

Traceback (most recent call last):
  File "test.py", line 6, in <module>
    Psycopg2Instrumentor().instrument_connection(connection)
  File "/home/alex/work/logfire/.venv/lib/python3.12/site-packages/opentelemetry/instrumentation/psycopg2/__init__.py", line 164, in instrument_connection
    connection._is_instrumented_by_opentelemetry = False
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'psycopg2.extensions.connection' object has no attribute '_is_instrumented_by_opentelemetry'

Additional context

Here's where the error happens:

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/46d2ce6acea9a1a6cb1a4d4c863077002f5f7d21/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py#L164

The code seems tested here:

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/46d2ce6acea9a1a6cb1a4d4c863077002f5f7d21/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py#L210

but it doesn't get the error because it's a mock connection.

xrmx commented 6 months ago

@alexmojaki would be great if you can open a PR

qiuge615 commented 3 months ago

@xrmx regarding this issue, shall I reference mysql/sqlite3 to fix this issue? Tested this approach locally, the instrument works and the trace can be displayed. Shall I create another issue to add the two parameters _otel_orig_cursor_factory and _is_instrumented_by_opentelemetry in connection object? The same approach may applies for psycopg.

xrmx commented 3 months ago

@qiuge615 If you have already some code open a PR so we can discuss looking at it :) Thanks!

qiuge615 commented 3 months ago

@xrmx Appreciate your quick reply, I will create a PR with the changes, thank you!

qiuge615 commented 3 months ago

@xrmx Created PR with the fix, please review, thanks.