Instrumentation layers and suppressing duplicates

[lmolkova]

Proposal to create clarity on instrumentation layers interaction and how to suppress duplicate layers (e.g. multiple instrumented HTTP clients layers).

cc @trask

[lmolkova]

@iNikem I hear your concern to have no suppression at all - I added an open question. I'd still like to understand why we'd need it and how common this need it.

[lmolkova]

Based on the discussions on the instrumentation SIG meeting, I've changed this OTEP to minimize tracing API changes. With the current approach, instrumentation specifies type at span creation time and SDK returns a suppressed span that's very similar to sampled-out, except it never becomes current.

Example: https://github.com/lmolkova/opentelemetry-java/pull/1/files#diff-48014f0437bde243933a6c8902a1f386fab6c78d6777019f24db1951e0070fc4

Open questions:

• messaging spans don't work great with it and I suggest working on it in messaging spec (there are more reasons than clear layering and suppression)
• adding optional optimization API to let instrumentation check first if span should be started
• defaults
• naming improvements are always welcome

[jmacd]

Why are there cases of multiple instrumentation HTTP clients using each other? That sounds like an odd configuration of clients. I wonder if when combining these clients, one can disable instrumentation of the other?

The DB call on top fo a REST call -- you propose to suppress the REST instrumentation, I take it? (Could the DB be configured with an un-instrumented REST client, instead?)

I wonder if it would be possible to replace InstrumentationType with InstrumentationLibrary.Name. Could you accomplish the same functional behavior, if for all spans with a non-remote parent you knew the InstrumentationLibrary of the nearest not-suppressed ancestor? It might make the proposal both simpler and more general: InstrumentationLibrary is already defined; it means you don't need an expandable enum.

[lmolkova]

Why are there cases of multiple instrumentation HTTP clients using each other?

1. One case is manual/native + auto: Client libraries: Manual/programmatic and auto instrumentation duplication problem  opentelemetry-specification#1767
2. Also, HTTP clients test to use each other (python, js, java all have one or another way to suppress HTTP instrumentation)

• in some cases, they are pluggable, i.e. higher level don't really know if the lower level is instrumented - this is more popular in SERVER spans
• in some cases, I guess higher-level clients instrumentation is not necessary

The DB call on top fo a REST call -- you propose to suppress the REST instrumentation, I take it? (Could the DB be configured with an un-instrumented REST client, instead?)

I suggest that

1. DB and REST are two layers, i.e. they are both spans (and not DB attributes stamped on HTTP span)
2. Depending on the configuration we can either
   • suppress nested HTTP (to reduce noise if users/backends don't want it) - note in this case DB span context will be propagated over HTTP
   • not suppress HTTP and have full picture

InstrumentationType with InstrumentationLibrary.Name

I'd love that, however InstrumentationLibrary.Name could be GoogleHttpClient or ApacheHttpClient while they both implement HTTP semantic conventions. The alternative could be to set InstrumentationType on the tracer resource in addition to InstrumentationLibrary.Name and version. I'd also put the semantic convention version, it follows, on the resource as well.

[lmolkova]

InstrumentationType with InstrumentationLibrary.Name

Experimented with this idea: instead of SpanBuilder.setType, I added 'type' parameter to library instrumentation info when tracer is being built:

• it makes tracers type-aware, i.e. tracers should not emit spans of different types. Which makes sense, but a new thing.
• it's even more subtle than the span creation option. App and lib devs would have a hard time paying attention and will end up with multi-type tracers (i.e. no suppression).

I updated the PR lmolkova/opentelemetry-java#1.

Will bring it up at the Tue Instrumentation SIG meeting.

I also entertained the idea of having a single tracer per type and returning noop tracers if one with the same type is registered - but it depends on the tracer creation time and is not stable.

[MovieStoreGuy]

I have left a few questions in sections I'd like some more clarification.

My main points come down to, what data would a service potential discard? Can I still get enough information to investigate issues within my service? What is potential reduction in outgoing are we expecting to see?

[MovieStoreGuy]

Is there ever a case that a user may want this?

[lmolkova]

Suppressed spans are never reported and are basically duplicates. If we'd make them current, following may happen:

• HTTP tracing middleware creates Span-1
• HTTP servlet instrumentation creates Span-2 - suppressed. if instrumentation will make it current, it will lead to attributes stamped on the wrong span
• users do Span.current().setAttribute - they expect it to be on Span1 and not Span2 (which they never even know was suppressed).

[MovieStoreGuy]

What should be the implementing libraries / sdks default?

[lmolkova]

libraries never think about the configuration. I believe suppression by kind + type is a good default since it shows all layers suppressing duplicate instrumentations only.
I don't have too strong an opinion - vendors/users should be able to easily change it.

[MovieStoreGuy]

Can I ask what data is lost in this process of suppression? Is the intent that all the data that would be part of the spans that are now being surpressed, is that being discard, aggregated into the parent span?

[lmolkova]

We assume that two different HTTP clients send almost identical spans (if they follow OTel HTTP spec). The duration might be slightly different, but essentially they report the same thing. So we'll keep the outer span and fully disregard the inner span.

[MovieStoreGuy]

What I'd really like to know is what information is going to be lost because of this?

For example, during an incident the suppressed data could help provide useful clues into the problematic calls to either downstreams or internal calls that would have made up part of the span.

[lmolkova]

Yes, e.g. you have DB running over HTTP.

• CLIENT-only suppression will cause all inner CLIENT spans to be suppressed. Our conventions make each CLIENT span have enough information to identify which endpoint it calls into. DB spans have endpoint info. None of the HTTP details would be recorded (no status codes, tries, etc), only the overall result will be set on DB span. You'd choose this configuration if you only care about the logical results and don't care much about transport or if you're saving costs at all means. If you do HTTP requests without outer DB client call, they will be recorded as usual.
• CLIENT + type suppression would keep DB and nested HTTP spans. If will only suppress duplicate HTTP client spans if multiple instrumentations work together. You'd choose this configuration if you want more details.

I understand your concern about suppression in general, what I hear from users some of them may want to sacrifice a bit of verbosity to save money. And kind + type suppression only suppresses duplicate data for those who can afford it.

[MovieStoreGuy]

I would like to more included in this section, I find it hard to suggest this to internal users without understanding the implications to the data.

[lmolkova]

what do you want to see in this section? How to choose a suppression strategy setting for a service assuming it will make it to the spec or what are the trade-offs of introducing these API changes?

[MovieStoreGuy]

Wooh, sorry. I forgot I had made a comment here.

Something that tells me the risk / potential loss of data that I am expecting.

[pyohannes]

I assume this doesn't work for manual parenting (without using context). However, I also assume that's not a big deal, as context will be used anyway to parent spans across library boundaries?

[lmolkova]

It should work just fine with context propagated manually. The suppression should be done through key on the context and how it's propagated is done is irrelevant.

However, I expect the duplication problem to heavily affect magical instrumentation. Apps that do most of the things manually have enough control to prevent it without type hint or suppression.

[pyohannes]

Could you define the term "instrumentation layer" in this document? I'm not sure if it refers to a single span, or to the whole span tree that's created by a single library.

[pyohannes]

As part of this, we'd also need to define which semantic conventions are composable and which are not.

[lmolkova]

Some semantic conventions define it (e.g. Couch DB wants to have HTTP attributes on DB spans). Unless it's already explicitly defined, we say conventions are not composable.
It looks like we should allow multiple types on the API surface if we want to allow that.

[pyohannes]

Related to the questions about a definition of layers: there are discussing about actually modelling HTTP request with multiple spans (for e. g. retries or forwards). Should we aim to cover those cases with a suppression proposal?

[lmolkova]

nope, for now, I assume we have one layer of HTTP spans (if we'll have more for logical + physical, I'd treat them as two different layers) - they can't carry the same information when both are enabled.

HTTP instrumentation would need configuration to emit 1) logical or 2) physical or 3) both to avoid redundancy and doubling of costs. I don't see a way to achieve it with suppression

[tedsuo]

@trask @lmolkova is this something the HTTP semconv working group would like to pick up again?

[trask]

@trask @lmolkova is this something the HTTP semconv working group would like to pick up again?

the scope of the current incarnation of the HTTP semconv working group is "just" stability. we discussed this and believe it can be addressed post-stability.