-
Notifications
You must be signed in to change notification settings - Fork 704
Add configurable client_sampling and random_sampling for tracing #7373
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
base: main
Are you sure you want to change the base?
Add configurable client_sampling and random_sampling for tracing #7373
Conversation
…or tracing Add ClientSampling and RandomSampling fields to TracingConfig to allow configurable client_sampling and random_sampling in Envoy tracing. Updates projectcontour#7271 Signed-off-by: Muxian Wu <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7373 +/- ##
==========================================
- Coverage 81.85% 81.37% -0.48%
==========================================
Files 130 130
Lines 15747 15787 +40
==========================================
- Hits 12889 12846 -43
- Misses 2574 2606 +32
- Partials 284 335 +51
🚀 New features to boost your workflow:
|
|
hi @wilsonwu @tsaarni @sunjayBhatia, can I get this reviewed? |
tsaarni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @WUMUXIAN for contributing!
It would be great to add more tests. I think you can add the new parameters to this file to help with the test coverage https://github.com/projectcontour/contour/blob/main/internal/featuretests/v3/tracing_test.go.
We should also add the new parameters to the tracing docs here:
contour/site/content/docs/main/config/tracing.md
Lines 12 to 18 in 07a2134
| Contour supports configuring envoy to export data to OpenTelemetry, and allows users to customize some configurations. | |
| - Custom service name, the default is `contour`. | |
| - Custom sampling rate, the default is `100`. | |
| - Custom the maximum length of the request path, the default is `256`. | |
| - Customize span tags from literal or request headers. | |
| - Customize whether to include the pod's hostname and namespace. |
The changelog check is failing. You need to rename the file to ./changelogs/unreleased/7373-WUMUXIAN-minor.md. Right now it uses a different name.
Also, the DCO check is failing. You can fix this by rewriting your git history to remove the extra opencode plan file from your first commit. Or, you can just add a sign-off to the commit that removes it. We squash the PR at the end anyway, but we need all commits to have a sign-off for the check to pass.
| Value: 100.0, | ||
| }, | ||
| ClientSampling: &envoy_type_v3.Percent{ | ||
| Value: 0.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To match the existing OverallSampling in the tests, I think we should set ClientSampling and RandomSampling to 100 in the test setup. This is clearer to read than leaving them uninitialized and having to use 0.0 as the result.
Add configurable client_sampling and random_sampling for tracing
Pull Request Description:
This PR implements the feature requested in issue #7271, allowing users to configure clientSampling and randomSampling rates for Contour's tracing configuration in addition to the existing overallSampling.
Summary
Changes
Fixes #7271
Release note: release-note/minor (new feature for enhanced tracing control)
Signed-off-by: Muxian Wu [email protected]