-
Notifications
You must be signed in to change notification settings - Fork 878
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
reuse semconv incubating attributes in test code #12992
base: main
Are you sure you want to change the base?
Conversation
entry(AttributeKey.stringKey("db.namespace"), "potatoes"), | ||
entry(AttributeKey.stringKey("db.query.text"), "SELECT * FROM potato"), | ||
entry(AttributeKey.stringKey("db.operation.name"), "SELECT")); | ||
entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), |
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.
is there any reason not to use static imports for all of these?
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 is only to minimize the diff and keep things consistent, do you think we should migrate and use static imports like in some other files ?
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 think we could skip static imports in this file to reduce diff size. In #12920 @trask used a script to automatically add the static imports, but that script only ran under instrumentation. We could run that script in instrumentation-api-incubator
in a separate PR. Use static imports in the changes that are under instrumentation, all (or at least most) of the usages should already use static imports there.
…nstrumentation into semconv-incubating
Incubating semconv attributes should not be used in production code to prevent introducing compatibility issues.
However in test code the semconv attributes duplication is harmful as it makes maintenance more tedious, so this PR removes duplication for some attributes. This is not meant to be an exhaustive cleanup so leftovers are expected.