Add semantic conventions for Elasticsearch client instrumentation #3358

estolfo · 2023-04-03T11:14:24Z

Fixes open-telemetry/semantic-conventions#706

Changes

Add semantic conventions for Elasticsearch client instrumentation span names and attributes.

Related OTEP(s)

Add semantic conventions for Elasticsearch client instrumentation semantic-conventions#706

specification/trace/semantic_conventions/README.md

specification/trace/semantic_conventions/instrumentation/elasticsearch.md

reyang

LGTM with the understanding of the upcoming changes #3358 (comment).

trask · 2023-04-05T15:44:51Z

@estolfo can you post a comparison of this proposal with the existing Java and Python implementations? I think that would help us pull in and get feedback from Java and Python reviewers.

estolfo · 2023-04-11T11:40:27Z

Python

This is what I see from looking at the source code and the tests:

Span name: Elasticsearch/ + (url || method || “request”)
For example: Elasticsearch/test-index/_doc/:id

doc id is replaced with :id
If the request is _search, the target is replaced with <target>

Special span attributes:

elasticsearch.id is the document id
elasticsearch.method is the http method
elasticsearch.url is the url (the path)
elasticsearch.target is the index or data stream
elasticsearch.params is query params

result fields:
elasticsearch.found
elasticsearch.timed_out
elasticsearch.took

Other fields

db_system is elasticsearch
db_statement is the request body

cc @trask

estolfo · 2023-04-12T07:26:41Z

Java

It's not so clear to me how the Java instrumentation works. I think they create spans for both the high-level rest client and the transport layer. I think it'd be best to get the java group's input on whether the following is accurate.

The span name depends on the level of the instrumentation. From what I can tell, the rest client instrumentation creates a span with the http method as the name, for example GET. I think I see that the transport client instrumentation uses a span name corresponding to the "action". For example, GetAction.

Special Span attributes

elasticsearch.action
elasticsearch.request
elasticsearch.request.indices
elasticsearch.request.search.types
elasticsearch.type
elasticsearch.id
elasticsearch.version
elasticsearch.shard.broadcast.total
elasticsearch.shard.broadcast.successful
elasticsearch.shard.broadcast.failed
elasticsearch.shard.replication.total
elasticsearch.shard.replication.successful
elasticsearch.shard.replication.failed
elasticsearch.response.status
elasticsearch.shard.bulk.id
elasticsearch.shard.bulk.index
elasticsearch.node.failures
elasticsearch.node.cluster.name
elasticsearch.request.write.type
elasticsearch.request.write.routing
elasticsearch.request.write.version

Other fields

db_system is elasticsearch
db_operation is the http method
db_statement is http method + path, for example GET _cluster/health
http attributes
net attributes

cc @trask

estolfo · 2023-04-12T12:09:34Z

.NET

I'm adding another one for good luck :)
Here is a summary of the .NET Elasticsearch client instrumentation, which is native to the client, in case it's helpful.

Span name: One of the URL template values listed here.

Other fields

db.system is elasticsearch
db.statement is request body

http attributes / net attributes

trask

thx!

trask · 2023-04-16T16:24:09Z

specification/trace/semantic_conventions/instrumentation/elasticsearch.md

+The elasticsearch url is modified with placeholders in order to reduce the cardinality of the span name. When the url
+contains a document id, it SHOULD be replaced by the identifier `{id}`. When the url contains a target data stream or
+index, it SHOULD be replaced by `{target}`.
+For example, a request to `/test-index/_doc/123` should have the span name `GET /{target}/_doc/{id}`. 
+When there is no target or document id, the span name will contain the exact url, as in `POST /_search`.


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?

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.

semantic_conventions/trace/database.yaml

specification/trace/semantic_conventions/instrumentation/elasticsearch.md

trask · 2023-04-16T16:33:39Z

specification/trace/semantic_conventions/instrumentation/elasticsearch.md

+| Attribute                  | Type | Description                                                          | Examples                                                | Requirement Level      |
+|----------------------------|---|----------------------------------------------------------------------|---------------------------------------------------------|------------------------|
+| `db.elasticsearch.doc_id`  | string | The document that the request targets, specified in the path.        | `'123'`                                                 | Conditionally Required |
+| `db.elasticsearch.target`  | string | The name of the data stream or index that is targeted.               | `'users'`                                               | Conditionally Required |


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.

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

…icsearch.md Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

github-actions · 2023-04-25T03:16:41Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

estolfo · 2023-04-25T08:26:32Z

@trask What are the next steps for this? Have I answered all your questions?

trask · 2023-04-25T14:06:34Z

@open-telemetry/opentelemetry-python-contrib-approvers @open-telemetry/dotnet-contrib-approvers @open-telemetry/java-instrumentation-approvers any thoughts on this, since it affects existing instrumentation in these languages? thx

estolfo · 2023-04-28T13:32:50Z

@trask We will bring it up at the Python, Java, and .NET SIGs next week if we don't hear from the teams before then.

Update: because the .NET instrumentation is native to the Elasitcsearch client itself, I checked with the maintainer of that project and he said that: he can't say with certainty that he can get the information without a reasonable amount of work in the client, but that he suspects it should be possible to add most if not all of these onto the .NET spans in a way that is backwards compatible.

specification/trace/semantic_conventions/instrumentation/elasticsearch.md

…icsearch.md Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>

estolfo · 2023-05-08T14:36:48Z

@trask
The Python group said they have no concerns about using these semantic conventions. They don't consider their consent a blocker and aren't concerned about backwards compatibility. They don't consider the instrumentation stable so are open to changing it.

[Not relevant to the discussion in this PR] For what it's worth, they said they'd rather have the instrumentation done natively in the Elasticsearch python client, rather than in the contrib repo.

reyang · 2023-05-09T04:38:06Z

@estolfo heads up - most likely this PR will be closed, and we'll ask you to resubmit the PR in a new repo, please refer to #3474 (comment).

estolfo · 2023-05-09T10:14:49Z

@reyang Sure, no problem, thanks for letting me know. I see that the repo isn't created yet so I'll open a PR when it's available.

jsuereth

Can you move this to https://github.com/open-telemetry/semantic-conventions?

estolfo · 2023-05-15T10:12:47Z

Opened new PR here

github-actions · 2023-05-23T03:16:43Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

AlexanderWert · 2023-05-26T12:19:04Z

semantic_conventions/trace/database.yaml

+        tag: call-level-tech-specific
+        brief: >
+          The query params of the request, as a json string.
+        examples: [ '"{\"q\":\"test\"}", "{\"refresh\":true}"' ]


Suggested change

examples: [ '"{\"q\":\"test\"}", "{\"refresh\":true}"' ]

examples: [ 'q=test&refresh=true' ]

github-actions · 2023-06-03T03:16:28Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

estolfo requested review from a team April 3, 2023 11:14

github-actions bot assigned tigrannajaryan Apr 3, 2023

reyang reviewed Apr 3, 2023

View reviewed changes

specification/trace/semantic_conventions/README.md Show resolved Hide resolved

reyang reviewed Apr 3, 2023

View reviewed changes

specification/trace/semantic_conventions/instrumentation/elasticsearch.md Outdated Show resolved Hide resolved

reyang reviewed Apr 3, 2023

View reviewed changes

specification/trace/semantic_conventions/instrumentation/elasticsearch.md Outdated Show resolved Hide resolved

arminru added spec:trace Related to the specification/trace directory semconv:database area:semantic-conventions Related to semantic conventions labels Apr 4, 2023

reyang approved these changes Apr 5, 2023

View reviewed changes

estolfo added 9 commits April 6, 2023 14:04

Add Elasticsearch semantic conventions and specification

6677724

Add HEAD HTTP request to ES spec

9916917

Remove config options descriptions

16470fc

Add reference to Elasticsearch semantic conventions to README

f3ecf2c

Make db.elasticsearch.method a string type instead of closed list

db620a8

Remove reference to closed list of http methods

0195b6f

Remove reference to config options

86f988f

Updates based on review

38f8fc2

Use alphabetical ordering

4a15793

estolfo force-pushed the elasticsearch branch from a0fa771 to 4a15793 Compare April 6, 2023 12:09

trask reviewed Apr 16, 2023

View reviewed changes

estolfo and others added 3 commits April 17, 2023 12:18

Update semantic_conventions/trace/database.yaml

c3c1285

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

Update specification/trace/semantic_conventions/instrumentation/elast…

164fe73

…icsearch.md Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

Update specification/trace/semantic_conventions/instrumentation/elast…

5d3fc99

…icsearch.md Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

github-actions bot added the Stale label Apr 25, 2023

trask removed the Stale label Apr 25, 2023

estolfo mentioned this pull request May 4, 2023

feat: Elasticsearch client auto instrumentation open-telemetry/opentelemetry-ruby-contrib#357

Closed

4 tasks

AlexanderWert reviewed May 5, 2023

View reviewed changes

specification/trace/semantic_conventions/instrumentation/elasticsearch.md Outdated Show resolved Hide resolved

Update specification/trace/semantic_conventions/instrumentation/elast…

9df0d56

…icsearch.md Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>

AlexanderWert mentioned this pull request May 12, 2023

Adapt existing Elasticsearch instrumentation to comply with semconv proposal open-telemetry/opentelemetry-java-instrumentation#8484

Closed

jsuereth requested changes May 12, 2023

View reviewed changes

estolfo mentioned this pull request May 15, 2023

Add semantic conventions for Elasticsearch client instrumentation open-telemetry/semantic-conventions#23

Merged

github-actions bot added the Stale label May 23, 2023

AlexanderWert reviewed May 26, 2023

View reviewed changes

github-actions bot closed this Jun 3, 2023

estolfo mentioned this pull request Jun 13, 2023

REQUEST: New membership for @estolfo open-telemetry/community#1544

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add semantic conventions for Elasticsearch client instrumentation #3358

Add semantic conventions for Elasticsearch client instrumentation #3358

estolfo commented Apr 3, 2023

reyang left a comment

trask commented Apr 5, 2023

estolfo commented Apr 11, 2023 •

edited

Loading

estolfo commented Apr 12, 2023 •

edited

Loading

estolfo commented Apr 12, 2023

trask left a comment

trask Apr 16, 2023

estolfo Apr 17, 2023

trask Apr 16, 2023

estolfo Apr 17, 2023

github-actions bot commented Apr 25, 2023

estolfo commented Apr 25, 2023

trask commented Apr 25, 2023

estolfo commented Apr 28, 2023 •

edited

Loading

estolfo commented May 8, 2023

reyang commented May 9, 2023

estolfo commented May 9, 2023

jsuereth left a comment

estolfo commented May 15, 2023

github-actions bot commented May 23, 2023

AlexanderWert May 26, 2023

github-actions bot commented Jun 3, 2023

	\| `db.elasticsearch.target` \| string \| The name of the data stream or index that is targeted. \| `'users'` \| Conditionally Required \|
	\| `db.elasticsearch.target` \| string \| The name of the data stream or index that is targeted, specified in the path. \| `'users'` \| Conditionally Required \|

	examples: [ '"{\"q\":\"test\"}", "{\"refresh\":true}"' ]
	examples: [ 'q=test&refresh=true' ]

Add semantic conventions for Elasticsearch client instrumentation #3358

Add semantic conventions for Elasticsearch client instrumentation #3358

Conversation

estolfo commented Apr 3, 2023

Changes

reyang left a comment

Choose a reason for hiding this comment

trask commented Apr 5, 2023

estolfo commented Apr 11, 2023 • edited Loading

Python

Special span attributes:

Other fields

estolfo commented Apr 12, 2023 • edited Loading

Java

Special Span attributes

Other fields

estolfo commented Apr 12, 2023

.NET

Other fields

trask left a comment

Choose a reason for hiding this comment

trask Apr 16, 2023

Choose a reason for hiding this comment

estolfo Apr 17, 2023

Choose a reason for hiding this comment

trask Apr 16, 2023

Choose a reason for hiding this comment

estolfo Apr 17, 2023

Choose a reason for hiding this comment

github-actions bot commented Apr 25, 2023

estolfo commented Apr 25, 2023

trask commented Apr 25, 2023

estolfo commented Apr 28, 2023 • edited Loading

estolfo commented May 8, 2023

reyang commented May 9, 2023

estolfo commented May 9, 2023

jsuereth left a comment

Choose a reason for hiding this comment

estolfo commented May 15, 2023

github-actions bot commented May 23, 2023

AlexanderWert May 26, 2023

Choose a reason for hiding this comment

github-actions bot commented Jun 3, 2023

estolfo commented Apr 11, 2023 •

edited

Loading

estolfo commented Apr 12, 2023 •

edited

Loading

estolfo commented Apr 28, 2023 •

edited

Loading