-
Notifications
You must be signed in to change notification settings - Fork 1k
Elasticsearch instrumentation metadata #14164
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
Conversation
| DbIncubatingAttributes.DbSystemIncubatingValues.ELASTICSEARCH), | ||
| equalTo(maybeStable(DB_OPERATION), "RefreshAction"))); | ||
|
|
||
| if (Boolean.getBoolean("testExperimental")) { |
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.
if you added testExperimental only to tell whether tests are run with experimental flag then you could have used otel.instrumentation.elasticsearch.experimental-span-attributes too as it is also a system property. Keeping testExperimental is fine too.
| DbIncubatingAttributes.DbSystemIncubatingValues.ELASTICSEARCH), | ||
| equalTo(maybeStable(DB_OPERATION), "ClusterHealthAction"))); | ||
|
|
||
| if (Boolean.getBoolean("testExperimental")) { |
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.
another option would be to use something like equalTo(ELASTICSEARCH_ACTION, experimental("ClusterHealthAction")) where experimental() would return null when the experimental flag is not set. This could result in more compact assertions. Up to you to decide which way you like more.
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 like your approach better, thank you for the suggestion
Related to #13468
This PR covers all of the elasticsearch instrumentation