-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: add opentelemetry tracing #51
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests.
Additionally, this does not not yet add tracing. The trace_call
needs to be called within the code to create the spans. See here for more context.
…n-spanner-sqlalchemy into opentelemetry_tracing
"db.params": parameters, | ||
"db.instance": cursor.connection.database.name, | ||
} | ||
with trace_call("SpannerSqlAlchemy.Executemany", trace_attributes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
with trace_call("SpannerSqlAlchemy.Executemany", trace_attributes): | |
with trace_call("SpannerSqlAlchemy.ExecuteMany", trace_attributes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the suggestion.
I would like some unit tests for this but they can come later. Please create an issue to track this.
Issue for tracking : #76 |
…n-spanner-sqlalchemy into opentelemetry_tracing
No description provided.