Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add semantic conventions for Elasticsearch client instrumentation #3358
Add semantic conventions for Elasticsearch client instrumentation #3358
Changes from 9 commits
6677724
9916917
16470fc
f3ecf2c
db620a8
0195b6f
86f988f
38f8fc2
4a15793
c3c1285
164fe73
5d3fc99
9df0d56
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
when looking at a given elasticsearch url, what's the best way for instrumentation to detect which part of it should be replaced by
{id}
and which part should be replaced with{target}
? is there a limited set elasticsearch url patterns?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.
Yes, there is a file in the Elasticsearch specification repo that can be used.
As an example, the .NET client uses this generator to create this path lookup file that is used in the instrumentation.
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 this also extracted from the path?
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.
That depends on the implementation of the client. It's possible that the client is written in way such that the target is specified as an argument or part of a hash passed to the function/method. The Ruby client, for example, has the target specified in the hash passed as an arg to the action method.
Example source / usage
So your suggested change makes sense; the target is determined not necessarily from the path, though it can appear as part of the path. I had this wording in there because I wanted to emphasize that the
db.elasticsearch.target
span attribute should match exactly what is in the path and what is replaced by{target}
in the span name.