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

ext_proc: change default sampling to false inheriting from parent decision #37794

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cainelli
Copy link
Contributor

@cainelli cainelli commented Dec 23, 2024

Commit Message: ext_proc: change default sampling decision to false
Fixes: #37783
Additional Description: Default to using the parent span's sampled status instead of true (default in StreamOptions), so that we don't emit orphan spans for all ext_proc streams where the parent span is not sampled. That's the same issue seen in Lua httpCall (#33200) and ext_authz (#19343).

Risk Level: Low
Testing: changed the tracing integration test behavior and validated in production.

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #37794 was opened by cainelli.

see: more, trace.

@cainelli cainelli changed the title ext_proc: change default sampling decision to false ext_proc: change default sampling to false inheriting from parent decision Dec 23, 2024
@cainelli
Copy link
Contributor Author

/retest

@cainelli cainelli marked this pull request as ready for review December 23, 2024 16:32
bool finished_;
bool context_injected_{false};
bool sampled_{false};
bool finished_{false};
Copy link
Contributor Author

@cainelli cainelli Dec 23, 2024

Choose a reason for hiding this comment

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

I noticed there are still some flake tests in CI and wonder if might be it.

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.

ext_proc: GRPC tracing requests are always sampled regardless of the parent decision
2 participants