-
Notifications
You must be signed in to change notification settings - Fork 607
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
Ensuring that pymysql follows semantic conventions #2915
base: main
Are you sure you want to change the base?
Conversation
@@ -1,685 +0,0 @@ | |||
# Copyright The OpenTelemetry Authors |
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.
Any reason why we are deleting this?
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.
I removed this file from the commit since it wasn't part of the PR I sent by mistake
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.
The diff shows you are deleting the file which exists already. You shouldn't be committing that delete.
"port": "port", | ||
"host": "host", | ||
"user": "user", | ||
"db.namespace": "db", |
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.
These are not the correct attributes to be changing for semantic conventions. Pymysql (as well as several other db instrumentations) use dbapi under the hood for it's instrumentations. You must check whether dbapi span attributes conform to the most recent db semantic conventions. dbapi is also one of the instrumentations needed for the semantic convention migration plan, so it is not as simple as changing the attributes to new values.
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.
So, should I ensure that dbapi follows the conventions defined in the SpanAttributes class, and by doing so, will it also apply to PyMySQL since it uses dbapi? Also, should it follow the semantic conventions defined here as well?
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.
This issue is a bit difficult for new contributors to pick up as it involves implementing the semantic convention migration plan. I would advise picking up a different issue if you are trying to pick up good first issues,
Description
This PR is related to issue: #1628
Type of Change
How Has This Been Tested?
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.