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.

Sign up for GitHub

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

Jump to bottom

docs: Tail-based sampling #7317

Merged
merged 10 commits into from
Mar 9, 2022
Merged

docs: Tail-based sampling #7317

merged 10 commits into from
Mar 9, 2022

Conversation

bmorelli25
Copy link
Member

@bmorelli25 bmorelli25 commented Feb 17, 2022 •
edited
Loading

Summary

This PR adds tail-based sampling documentation.

I tested the feature by spinning up an 8.1 instance on ESS and pointing APM integration testing at it:

./scripts/compose.py start 8.1 --no-elasticsearch --no-kibana --no-apm-server --with-opbeans-go --with-opbeans-java --with-opbeans-node --apm-server-url=${APM_SERVER_URL} --apm-server-secret-token=${APM_SERVER_TOKEN} --opbeans-elasticsearch-url=${ES_URL}

Preview this PR

Related

@bmorelli25 bmorelli25 added Team:Docs Label for the Observability docs team backport-8.1 Automated backport with mergify backport-8.2 Automated backport with mergify labels Feb 17, 2022
@bmorelli25 bmorelli25 requested a review from a team February 17, 2022 03:09
@bmorelli25 bmorelli25 self-assigned this Feb 17, 2022
@bmorelli25 bmorelli25 changed the title Tbs docs docs: Tail-based sampling Feb 17, 2022
bmorelli25
bmorelli25 commented Feb 17, 2022
View reviewed changes
docs/sampling.asciidoc Outdated Show resolved Hide resolved
bmorelli25
bmorelli25 commented Feb 17, 2022
View reviewed changes
docs/sampling.asciidoc Outdated Show resolved Hide resolved
@apmmachine
Copy link
Contributor

apmmachine commented Feb 17, 2022 •
edited
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-08T23:17:34.485+0000

  • Duration: 50 min 30 sec

Test stats 🧪

Test Results
Failed 0
Passed 5652
Skipped 19
Total 5671

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /hey-apm : Run the hey-apm benchmark.

  • /package : Generate and publish the docker images.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

axw
axw reviewed Feb 17, 2022
View reviewed changes
Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Looks great!

docs/sampling.asciidoc Show resolved Hide resolved
docs/sampling.asciidoc Outdated Show resolved Hide resolved
docs/sampling.asciidoc Show resolved Hide resolved
docs/sampling.asciidoc Outdated Show resolved Hide resolved
docs/sampling.asciidoc Outdated Show resolved Hide resolved
docs/sampling.asciidoc Show resolved Hide resolved
@bmorelli25 bmorelli25 requested a review from axw February 17, 2022 21:29
axw
axw reviewed Feb 21, 2022
View reviewed changes
docs/sampling.asciidoc Outdated
Comment on lines 84 to 87
Sampling does not impact aggregated transaction and span <<data-model-metrics,metrics>>.
This means that the visualizations in the APM app, like latency, throughput, time spent by span type, failed transaction rate,
errors, error rate, etc., will always accurately reflect *all* of your application's requests,
regardless of the configured sampling rate.
Copy link
Member

Choose a reason for hiding this comment

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

This might be a bit of an overstatement. Sampling may affect accuracy of latency.

Metrics are based on the sampled traces, and weighted by the inverse sampling rate. e.g. if you are sampling at 5%, then each trace will be counted as 20. Thus, as variance of latency increases, or the sampling rate decreases, your level of error will increase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Updated. I'm still a bit confused about this so please let me know if it still needs work (likely).

Copy link
Contributor

Choose a reason for hiding this comment

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

The current wording sounds good.

docs/sampling.asciidoc Show resolved Hide resolved
@bmorelli25
Copy link
Member Author

bmorelli25 commented Mar 7, 2022 •
edited
Loading

Circling back to this now that PTO and Space, Time are over.

@bmorelli25 bmorelli25 requested review from a team and axw March 7, 2022 20:35
simitt
simitt reviewed Mar 8, 2022
View reviewed changes
docs/sampling.asciidoc Outdated Show resolved Hide resolved
docs/sampling.asciidoc Show resolved Hide resolved
docs/sampling.asciidoc Show resolved Hide resolved
docs/sampling.asciidoc Outdated
Comment on lines 84 to 87
Sampling does not impact aggregated transaction and span <<data-model-metrics,metrics>>.
This means that the visualizations in the APM app, like latency, throughput, time spent by span type, failed transaction rate,
errors, error rate, etc., will always accurately reflect *all* of your application's requests,
regardless of the configured sampling rate.
Copy link
Contributor

Choose a reason for hiding this comment

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

The current wording sounds good.

simitt
simitt approved these changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

Left a couple minor comments, but otherwise looks good. Feel free to merge after addressing the last bit.

@bmorelli25
Copy link
Member Author

@elasticmachine, run elasticsearch-ci/docs

@bmorelli25 bmorelli25 merged commit e6b435e into elastic:main Mar 9, 2022
@bmorelli25 bmorelli25 deleted the tbs-docs branch March 9, 2022 00:40
@bmorelli25 bmorelli25 removed the backport-8.2 Automated backport with mergify label Mar 9, 2022
mergify bot pushed a commit that referenced this pull request Mar 9, 2022
@bmorelli25
docs: Tail-based sampling (#7317)
4281409 
Co-authored-by: Silvia Mitter <silvia.mitter@elastic.co>
(cherry picked from commit e6b435e)
@mergify mergify bot mentioned this pull request Mar 9, 2022
Merged
bmorelli25 added a commit that referenced this pull request Mar 9, 2022
@mergify @bmorelli25
docs: Tail-based sampling (#7317) (#7495)
6cd38d0 
Co-authored-by: Silvia Mitter <silvia.mitter@elastic.co>
(cherry picked from commit e6b435e)

Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simitt simitt simitt approved these changes

@axw axw Awaiting requested review from axw

Assignees

@bmorelli25 bmorelli25

Labels
backport-8.1 Automated backport with mergify Team:Docs Label for the Observability docs team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Request] Document APM tail-based sampling feature Document tail-based sampling
4 participants
@bmorelli25 @apmmachine @axw @simitt