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
Add Sampling configuration page #14856
Add Sampling configuration page #14856
Conversation
Skipping CI for Draft Pull Request. |
content/en/docs/tasks/observability/distributed-tracing/sampling/index.md
Outdated
Show resolved
Hide resolved
/test all |
content/en/docs/tasks/observability/distributed-tracing/sampling/index.md
Outdated
Show resolved
Hide resolved
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.
LGTM, agree would be nice to have a test if possible.
Yeah I'm not sure about what to do with the tests. The snippets where we use the MeshConfig to set the percentage sampling or the Telemetry API can be tested but as I said above, it's just starting Istio. Not much we can test regarding the rest. For the snippet that uses a custom sampler (The only now is the Dynatrace sampler) also can't be really tested as spans are exported to a Dynatrace environment and I doubt we want that in the tests. So I'm not sure there's much value in it. But if it's required, then I can add some, to at least make sure Istio starts. |
cc @zirain |
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.
LGTM thank you for adding the tests!
Hi all, sorry for the ping, but is there anything else to do here before we merge? |
content/en/docs/tasks/observability/distributed-tracing/sampling/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/tasks/observability/distributed-tracing/sampling/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/tasks/observability/distributed-tracing/sampling/mesh_test.sh
Outdated
Show resolved
Hide resolved
content/en/docs/tasks/observability/distributed-tracing/sampling/telemetry_test.sh
Outdated
Show resolved
Hide resolved
/retest |
content/en/docs/tasks/observability/distributed-tracing/sampling/index.md
Outdated
Show resolved
Hide resolved
LGTM.. you can just take care of Daniel's comments, and we are good to go |
@kfaseela Done! thank you! |
/retest |
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.
LGTM as well.
Description
Fixes #14831
Adds a new 'task' page that describes all the ways trace sampling can be configured.
Reviewers