Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tracing information to all components #8804

Open
2 tasks
codeboten opened this issue Nov 6, 2023 · 7 comments
Open
2 tasks

Add tracing information to all components #8804

codeboten opened this issue Nov 6, 2023 · 7 comments
Labels
collector-telemetry healthchecker and other telemetry collection issues

Comments

@codeboten
Copy link
Contributor

codeboten commented Nov 6, 2023

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.

(edit by @mx-psi):

Sub-issues:

@codeboten codeboten added the collector-telemetry healthchecker and other telemetry collection issues label Nov 6, 2023
@codeboten codeboten self-assigned this Nov 9, 2023
@atoulme
Copy link
Contributor

atoulme commented Dec 14, 2023

Cross-reference this issue #6368

@iblancasa
Copy link
Contributor

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

@TylerHelmuth
Copy link
Member

I still like the solution that implemented it in consumer so it automatically happened everywhere: #8833

@iblancasa
Copy link
Contributor

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

iblancasa added a commit to iblancasa/opentelemetry-collector that referenced this issue Aug 9, 2024
@bogdandrutu
Copy link
Member

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

@mackjmr
Copy link
Member

mackjmr commented Oct 17, 2024

@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:

  • Exposes WithObsReport for config
  • Exposes ObsReport object

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
Copy link
Member

mackjmr commented Oct 22, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collector-telemetry healthchecker and other telemetry collection issues
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

6 participants