Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default SQLite3 cursor is not instrumented #3082

Open
2 tasks
Kludex opened this issue Dec 10, 2024 · 5 comments · Fixed by #3088
Open
2 tasks

Default SQLite3 cursor is not instrumented #3082

Kludex opened this issue Dec 10, 2024 · 5 comments · Fixed by #3088
Labels
bug Something isn't working

Comments

@Kludex
Copy link
Contributor

Kludex commented Dec 10, 2024

Describe your environment

OS: (e.g, Ubuntu)
Python version: (e.g., Python 3.8.10)
Package version: (e.g., 0.46.0)

What happened?

The default sqlite3.Cursor is not instrumented.

Steps to Reproduce

import sqlite3

from opentelemetry.instrumentation.sqlite3 import SQLite3Instrumentor

import logfire

logfire.configure()

SQLite3Instrumentor().instrument()

cnx = sqlite3.connect(':memory:')
cnx.execute('CREATE TABLE test (testField INTEGER)')
cnx.close()

Expected Result

I would expect a span to be created for the table creation.

Actual Result

The span was not created.

Instead, if you do...

import sqlite3

from opentelemetry.instrumentation.sqlite3 import SQLite3Instrumentor

import logfire

logfire.configure()

SQLite3Instrumentor().instrument()

cnx = sqlite3.connect(':memory:')
cursor = cnx.cursor()
cursor.execute('CREATE TABLE test (testField INTEGER)')
cursor.close()
cnx.close()

Additional context

The documentation should be updated... The code there doesn't run as is...

https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/sqlite3/sqlite3.html

It should create the table first!

Would you like to implement a fix?

None

PRs

@Kludex Kludex added the bug Something isn't working label Dec 10, 2024
@Kludex
Copy link
Contributor Author

Kludex commented Dec 10, 2024

@tammy-baylis-swi
Copy link
Contributor

tammy-baylis-swi commented Dec 10, 2024

I think it might be from DB-API integration's TracedCursorProxy wrapping cursor.execute, but TraceConnectionProxy doesn't wrap connection.execute.

It could be a non-trivial fix for the latter to be added directly to DB-API integration, or to a subclass in the sqlite3 instrumentor (latter possibly requiring more subclasses/overrides to work). But anyone is welcome to give it a try 🙂

I've created a small docs update PR to encourage use of explicit cursors (cursor.execute) to make sure tracing is supported: #3088

I've also appended a "PRs" section to OP to prevent automated issue closing if multiple PRs.

@Kludex
Copy link
Contributor Author

Kludex commented Dec 11, 2024

@xrmx Can you reopen this issue? Explaining why this is an issue doesn't solve the issue...

@Kludex
Copy link
Contributor Author

Kludex commented Dec 11, 2024

It could be a non-trivial fix for the latter to be added directly to DB-API integration [...]

That would be wrong anyway. This specific for sqlite3. The other database packages don't support calling execute or the cursor methods from the Connection object.

@xrmx xrmx reopened this Dec 11, 2024
@tammy-baylis-swi
Copy link
Contributor

This specific for sqlite3.

Then one could try to subclass the connection proxy in the sqlite3 instrumentor as I had also suggested in [...]. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants