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

[exporterhelper] alternative tracing of components #8910

Closed

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Nov 14, 2023

Instead of providing an interface that is then implemented by each helper, this proposes using the existing components as is and wrapping ConsumeFunc in each helper. This reduces new APIs, but it does mean that the wrapping code will need to be implemented inside each helper.

Alternative to #8833

Part of #8804

Instead of providing an interface that is then implemented
by each helper, this proposes using the existing components as
is and wrapping Consume<Signal>Func in each helper. This
reduces new APIs, but it does mean that the wrapping code will
need to be implemented inside each helper.

Signed-off-by: Alex Boten <aboten@lightstep.com>
@TylerHelmuth
Copy link
Member

@codeboten with this solution, any component that is not using the helper would have to manually instrument themselves to show up in the trace correct?

@codeboten
Copy link
Contributor Author

@TylerHelmuth correct

@dmitryax
Copy link
Member

The persistent queue drops the context, so this will break tracing if persistent queue is enabled

@codeboten
Copy link
Contributor Author

@bogdandrutu @TylerHelmuth @dmitryax @djaglowski is there a preference between this and #8833? This one has less surface API, but I guess it does mean that anyone who isn't using helpers would have to implement this call themselves.

@TylerHelmuth
Copy link
Member

@codeboten the helpers are pretty ubiquitous right? All the components in Core and Contrib would be instrumented?

@dmitryax
Copy link
Member

@codeboten, unfortunately, neither of them will work with the enabled persistent queue. I think we need to keep it after the queue until we find a way to propagate the context through the persistent queue if we want to. But it may introduce a significant overhead.

@dmitryax
Copy link
Member

But I may not realize all the consequences of the context being dropped down the pipeline. I assume we lose connection with the gPRC export span. Maybe it's not as bad. Just wanted to make sure we are not making it worse with the persistent queue enabled

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 12, 2023
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants