elastic / apm

Elastic Application Performance Monitoring - resources and general issue tracking for Elastic APM.
https://www.elastic.co/apm
Apache License 2.0
384 stars 114 forks source link

Create specification for OpenTelemetry span attributes for Elastic-specific APM features #645

Open SylvainJuge opened 2 years ago

SylvainJuge commented 2 years ago

In order to provide compatibility with Elastic-only features through OpenTelemetry (either API, API+SDK, API + agent bridge or OpenTelemetry agent), we should define the span attributes and a namespace prefix for that purppose.

For example, the span.context.destination.service.resource value can be controlled by end-user through agent APIs (specific to Elastic) but we can't do the same when using the OpenTelemetry API, hence forcing users to choose between a vendor-neutral API or having the flexibility to customize the captured data.

Proposal for attribute prefix: elastic.otel.

We can split the properties in 2 categories:

eyalkoren commented 2 years ago

We need to consider how proper it is to use attributes for behavioral configurations, or even using specially-prefixed attributes altogether. The major incentive for using OTel API is vendor neutrality. Using elastic.otel may be conceived as defying that. With behavioral attributes, this may be specifically more problematic, as we are going to act on those, but not send them as part of the spans metadata. However, if a user switches to another OTel-compliant implementation, such attributes will be added and sent as data. Not sure if this should hold us back, but worth the discussion.

SylvainJuge commented 2 years ago

The only example I've found so far of a non-standard attribute added to OTel spans can be found in Splunk OTel distribution: https://github.com/signalfx/splunk-otel-java/blob/7619f57606c173eeedb97445647068a467e39c68/docs/webengine-attributes.md#webengine-attributes.

felixbarny commented 2 years ago

Proposal for attribute prefix: elastic.otel.

Based on OTel naming schema recommendations,

To avoid clashes with names introduced by other companies (in a distributed system that uses applications from multiple vendors) it is recommended to prefix the new name by your company's reverse domain name, e.g. com.acme.shopname.

I suggest using co.elastic. as a prefix. Using otel seems redundant as OTel is implied for OTel attributes.

The major incentive for using OTel API is vendor neutrality. Using elastic.otel may be conceived as defying that.

I don't think that using Elastic-specific attribute names defies the whole purpose around vendor neutrality. If a users is concerned about using vendor-specific functionality, they can just choose to not use it.

Whenever they make sense across OTel implementation, we should, however try to contribute attributes and API changes to upstream OTel. In this case, however, it seems like a very specific feature that not even a quorum of the Elastic agents support.

However, if a user switches to another OTel-compliant implementation, such attributes will be added and sent as data.

While that is a side-effect when switching to another backend, it doesn't seem like a big issue. I wouldn't hold back because of that.

eyalkoren commented 2 years ago

Following the feedback above, the attribute name used in the Java agent was changed to co.elastic.discardable.