open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.72k stars 888 forks source link

Consider pulling in more url.* fields from ECS as part of alignment #3405

Open trask opened 1 year ago

trask commented 1 year ago

Consider pulling in some or all of:

Based on @AlexanderWert's https://github.com/open-telemetry/opentelemetry-specification/pull/3355#issuecomment-1510995857

So, I would suggest if there are no strong reasons for leaving out certain fields from ECS (especially when they are not conflicting with others in OTel), let's try to adopt them. Otherwise, it would imply unnecessary breaking changes on ECS-based data.

And @tigrannajaryan's https://github.com/open-telemetry/opentelemetry-specification/pull/3355#issuecomment-1511625124:

I believe logs should share many (most) of the semantic conventions with traces. There may be exceptions to this, but I would expect that the vast majority of conventions are the same for traces and logs. This includes http conventions, I don't see why logs need to have different conventions.

lmolkova commented 1 year ago

Logs, metrics, and traces should share common attributes, but most of the attributes can only be applicable to logs given they're verbose, sometimes duplicate information, and require additional logic/resources to populate them.

We should either bring attributes which are not used for traces/metrics as log-only attributes (initially) or make them opt-in by default.

trask commented 1 year ago

Some initial thoughts after discussing in HTTP semconv stability WG today:

AlexanderWert commented 1 year ago

@trask Thanks for creating this separate thread to discuss this!

I think the above really depends a lot on how we see the semantic conventions, i.e. in a very concrete context / scope (such as HTTP spans) or rather generic and signal agnostic. I know it's quite a fundamental question / topic, but I feel discussing it would help ECS alignment efforts going forward.

First of all, I think that one of the biggest values of the semantic conventions is the correlation of different data points and signals (especially with OTel, apart from tracing, expanding more and more into metrics, logs and other signals). So I fully agree with @tigrannajaryan 's comment, i.e. different signals (especially traces and logs) should share as many semantic conventions as possible (unless there are good reasons not to do, such as cardinality for metrics, etc.). Therefore, IMHO per default we should consider attributes and and their namespaces as signal agnostic (and rather do exceptions for attributes that should not apply to certain signals, such as metrics, or should apply only to a specific signal).

For example, in your comment above:

url.domain and url.port - HTTP semconv already captures these in server.domain and server.port (https://github.com/open-telemetry/opentelemetry-specification/pull/3402). We would like to avoid multiple ways to capture the same thing since to make life easier on backends and queries/alerting. Some options: pull these over when they are needed for another semantic convention other than HTTP, or pull them in now as Opt-In attributes for HTTP semconv (as duplicates of server.domain and server.port in the context of HTTP semconv)

This is reasonable when considering just HTTP spans as the scope. But, in some logging and security use cases mixing up url.* and server.* attributes might be confusing or even wrong. As the namespace implies, all the url.* attributes basically describe in detail a URL string. While server.* fields describe the responder in a network connection. In case of a non-HTTP connection, url.* might be not applicable, while server.* is.

Happy to discuss this topic in more detail!

I like the proposal of using the Opt-In option when modeling redundancy, though!

tigrannajaryan commented 1 year ago

We should either bring attributes which are not used for traces/metrics as log-only attributes (initially) or make them opt-in by default.

Yes, sure they should be opt-in, no reason to make these additional attributes required. I maintain that we should aim for uniformness of conventions for logs and traces (exceptions from this need an explanation). Traces don't have to use these attributes, they are just optional attributes that can be used if the user choses so.

trask commented 1 year ago

we discussed in HTTP semconv WG meeting today:

I'm removing HTTP semconv stability blocker tag from this issue (~I'll open a separate issue for url.fragment and tag that as an HTTP semconv stability blocker~ https://github.com/open-telemetry/opentelemetry-specification/pull/3355#issuecomment-1515438316).