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.

Sign up for GitHub

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

Jump to bottom

Ensuring that pymysql follows semantic conventions #2915

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Ensuring that pymysql follows semantic conventions #2915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
afd8c4a
making pymysql follow semantic conventions
Joice-crypto Oct 19, 2024
84bc4da
Remove unwanted file
Joice-crypto Oct 19, 2024
7361aa4
Merge branch 'main' into pymysql-semantic
lzchen Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...entelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@
from opentelemetry.instrumentation.pymysql.version import __version__

_CONNECTION_ATTRIBUTES = {
"database": "db",
"port": "port",
"host": "host",
"user": "user",
"db.namespace": "db",
Copy link
Contributor

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.

Copy link
Author

@Joice-crypto Joice-crypto Oct 19, 2024 •
edited
Loading

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Joice-crypto

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,

"server.port": "port",
"server.address": "host",
"db.user": "user",
}
_DATABASE_SYSTEM = "mysql"

Expand Down
Loading
Loading