-
Notifications
You must be signed in to change notification settings - Fork 107
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
Conversation
Removes attributes from anchor tags in `transaction-sampling.md`. For #770.
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.
Thanks for the documentation. Only a formatting nit and a correction (I think)
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 apart from Marc's 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.
Just some minor suggestions below.
Co-authored-by: Colleen McGinnis <[email protected]>
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.
@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
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.
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
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.
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.
Thanks. In that case, I'll double check the numbers in the table and get the PR merged |
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