open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.44k stars 1.46k forks source link

Add tracing information to all components #8804

Open codeboten opened 1 year ago

codeboten commented 1 year ago

Now that it is possible to emit traces from the Collector, it would be great produce spans consistently across components. This issue is a general issue that will include a checklist for all components in this repo.

Describe the solution you'd like Each component produces a span using the StartTracesOp and EndTracesOp methods available in obsreport.

atoulme commented 11 months ago

Cross-reference this issue https://github.com/open-telemetry/opentelemetry-collector/issues/6368

iblancasa commented 3 months ago

What needs to be done? Any hints? I would love to contribute.

TylerHelmuth commented 3 months ago

I still like the solution that implemented it in consumer so it automatically happened everywhere: https://github.com/open-telemetry/opentelemetry-collector/pull/8833

iblancasa commented 3 months ago

I'll take a look and try to send a PR. Thanks @TylerHelmuth

bogdandrutu commented 3 weeks ago

I would be concern to add traces for each individual component. Think about a simple processor that would not benefit.

mackjmr commented 3 weeks ago

@codeboten @iblancasa @TylerHelmuth I am interested in helping this issue move forward.

There is already some tracing enabled in some parts of core:

Seems like there are currently 2 POCs to add traces to all components:

1. Add obsreport to consumer (#8833, #10847)

The benefits of this approach is that this adds tracing to all components, specifically to the Consume<Signal>Func which actually does the work.

One point I am unclear on in these POCs is that the obsreport is expected to be created and implemented in each of the components using the consumer. Why not create the obsreport within the consumer package ? Is this made in this way so that the individual components could add some custom logic to the tracing ? If so, perhaps this could be done via configuration options of the obsreport within the consumer.

The main downside of this approach is that it exposes APIs/objects:

The rest of the exposed APIs are in an internal package, at least for #10847.

2. Wrap ConsumeFunc in each helper (#8910)

The benefits of this approach are that it requires exposing less APIs.

The cons are that it is seemingly its more manual than adding tracing to the consumer as the wrapper need to be added in all helpers. However, in the current approach of the consumer tracing, the obsreport implementation would need to be added in each helper anyways, so I'm not sure how much more work this one would actually be.

Other Concerns:

It seems that for both approaches, @dmitryax expressed concerns that when the persistent queue is enabled, tracing will break as the persistent queue drops the context. The requirement here is to find a way to still propagate trace context when persistent queue is enabled, or we may find it acceptable that the trace context is broken in this case if there are no solutions.

Is my understanding correct here ? If so, I can try putting up a POC of approach 1 or 2 considering Dmitrii's concern. I could also try to create a POC where the obsreport is created within the consumer package.

mackjmr commented 3 weeks ago

I ran a quick test instrumenting the consumer with/ without persistent queue enabled.

Without Persistent Queue:

We get two traces:

2024-10-22_16-54-50 2024-10-22_16-55-23

With Persistent Queue:

We get three traces:

2024-10-22_16-51-14 2024-10-22_16-51-30 2024-10-22_16-51-43

We get 3 traces instead of two because context is lost between the consumer span and exporter span, so these do not have the parent/child relation that we have in the first case.