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

debuginfo/store: Use tracer.Start instead of SpanFromContext #3270

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

kakkoyun
Copy link
Member

@kakkoyun kakkoyun commented Jun 12, 2023

Signed-off-by: Kemal Akkoyun kakkoyun@gmail.com

Since parca-agent started to create spans from the client side, SpanFromContext has started to create problems.
Even though operations successfully finish since OTel SDK propagates the context from/to the client-side agent receives a timed-out/closed context.
This was causing errors on the agent side.
The PR ensures a new context is created, and the child context is closed when the operation is finished without touching the parent(incoming) context.

Fixes parca-dev/parca-agent#1741

@kakkoyun kakkoyun requested a review from a team as a code owner June 12, 2023 16:06
@kakkoyun kakkoyun changed the title Use tracer.Start instead of SpanFromContext debuginfo/store: Use tracer.Start instead of SpanFromContext Jun 12, 2023
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@metalmatze
Copy link
Member

I wonder if there are some implications here. Would we break the gRPC interceptor to our service implementation chain?

@kakkoyun
Copy link
Member Author

I wonder if there are some implications here. Would we break the gRPC interceptor to our service implementation chain?

Creating a new span should break any existing facilities. This API makes sure we have a new context when we passed down to call chain.

Copy link
Member

@metalmatze metalmatze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, let's try this then!

@kakkoyun kakkoyun merged commit d77cea4 into main Jun 12, 2023
@kakkoyun kakkoyun deleted the fix_context_cancellation_errors branch June 12, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

debuginfoClient.ShouldInitiateUpload returns unexpected context.DeadlineExceeded
2 participants