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

fix otel trace handler WithTracing, use another func #638

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

KubeKyrie
Copy link
Contributor

What type of PR is this?
/kind bug

What this PR does / why we need it:

Fixed OTEL trace interruption when other services requested clusterpedia apiserver. And this is caused by otelhttp.WithPublicEndpoint(). As shown here https://github.com/kubernetes/kubernetes/blob/09a5049ca785024edd4955eb82e855d9b5657491/staging/src/k8s.io/apiserver/pkg/endpoints/filters/traces.go#L33C20-L33C20

And I think it's a bug of kubernetes, so I replaced it with WithTracing in pkg k8s.io/component-base/tracing.

Which issue(s) this PR fixes:
Fixes #None

Special notes for your reviewer:

The two WithTracing is almost same and we can customize the span name by the new one. So it's more flexible.

Does this PR introduce a user-facing change?:

fix otel trace handler WithTracing

@clusterpedia-bot
Copy link

Hi @KubeKyrie,
Thanks for your pull request!
If the PR is ready, use the /auto-cc command to assign Reviewer to Review.
We will review it shortly.

Details

Instructions for interacting with me using comments are available here.
If you have questions or suggestions related to my behavior, please file an issue against the gh-ci-bot repository.

@clusterpedia-bot clusterpedia-bot added the kind/bug Something isn't working label Jan 5, 2024
@KubeKyrie
Copy link
Contributor Author

The otelhttp.WithPublicEndpoint() option causes spans generated by the API Server handler to be the start of a new trace (with a link to the incoming trace), rather than be children of the incoming request.
The reason why kube-apiserver uses WithPublicEndpoint is to prevent callers from influencing sampling decisions, see detail kubernetes/kubernetes#109829. And the discussion about the reentrant problem is still in progress, see kubernetes/kubernetes#103186.

So, I think the current solution is to use another handler filter func, just as this PR shows.

Copy link
Member

@Iceber Iceber left a comment

Choose a reason for hiding this comment

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

LGTM

@Iceber Iceber merged commit 1bc7cf6 into clusterpedia-io:main Jan 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants