Skip to content

apm: Document tail-based sampling performance #770

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

Merged
merged 33 commits into from
Mar 18, 2025
Merged

Conversation

carsonip
Copy link
Member

@carsonip carsonip commented Mar 13, 2025

Describe TBS requirements and give some numbers to demonstrate perf overhead.

Numbers are from benchmarks done in elastic/apm-server#11346

Fixes elastic/apm-server#11346

@bmorelli25 bmorelli25 mentioned this pull request Mar 13, 2025
bmorelli25 added a commit that referenced this pull request Mar 13, 2025
Removes attributes from anchor tags in `transaction-sampling.md`.

For #770.
@carsonip carsonip requested a review from simitt March 14, 2025 19:26
@carsonip carsonip marked this pull request as ready for review March 14, 2025 19:26
@carsonip carsonip changed the title [WIP] Document APM tail-based sampling performance apm: Document tail-based sampling performance Mar 14, 2025
@carsonip
Copy link
Member Author

image
docs don't like the number of columns

Copy link

@marclop marclop left a comment

Choose a reason for hiding this comment

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

Thanks for the documentation. Only a formatting nit and a correction (I think)

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.

LGTM apart from Marc's comment.

Copy link
Contributor

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

Just some minor suggestions below.

Copy link
Member Author

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

@colleenmcginnis any comment on the table being too wide causing it to be cut off in the UI mentioned in this comment? I can split the apm server version out as a header, but I'm not confident it fixes for all screen sizes

Copy link
Member Author

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

I've moved the APM server version out as a header

  • I'm glad that h4 isn't displayed on the right nav
  • It fixes for some screen sizes, not all. If you resize your browser window, sometimes the last column of the table isn't shown

Copy link
Contributor

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

Looks good to me! I think the table is ok and the last adjustment you made is a nice strategy to make a little easier to read.

If you have thoughts on how tables could be improved (for example allow control over column widths or making it more obvious that there's hidden content you can scroll left/right to see), please open an issue in https://github.com/elastic/docs-builder.

@carsonip
Copy link
Member Author

Thanks. In that case, I'll double check the numbers in the table and get the PR merged

@carsonip carsonip merged commit 8366745 into elastic:main Mar 18, 2025
4 checks passed
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.

docs: Benchmark and document tail-based sampling performance
4 participants