-
Notifications
You must be signed in to change notification settings - Fork 626
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
[DOC] Restructure Pyroscope documentation and share content #3798
[DOC] Restructure Pyroscope documentation and share content #3798
Conversation
docs/sources/configure-server/storage/configure-disk-storage.md
Outdated
Show resolved
Hide resolved
|
||
<!-- vale Grafana.Spelling = NO --> | ||
|
||
[Brendan Gregg](https://www.brendangregg.com/flamegraphs.html), the creator of flame graphs, was inspired by the inability to view, read, and understand stack traces using the regular profilers to debug performance issues. |
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.
hah, this is so cool 😎
Co-authored-by: Aleksandar Petrov <[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.
Looks good to me! I added a few minor suggestions for new and existing content.
docs/sources/configure-server/storage/configure-disk-storage.md
Outdated
Show resolved
Hide resolved
docs/sources/configure-server/storage/configure-disk-storage.md
Outdated
Show resolved
Hide resolved
docs/sources/configure-server/storage/configure-object-storage-backend.md
Outdated
Show resolved
Hide resolved
|
||
Continuous profiling is a systematic method of collecting and analyzing performance data from production systems. | ||
[//]: # 'Shared content for the when to use continuous profiling.' | ||
[//]: # 'This content is located in /pyroscope/docs/sources/shared/intro/continuous-profiling.md' |
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.
Are these comments a common practice or generated? Otherwise, perhaps it can be omitted as the next line is descriptive enough.
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.
It's a best practice when using the include files to make sure people understand where content comes from. Even if it's obvious in the include line itself, not everyone reads it.
The comments also provide a visual clue for where content is shared in a file. The comments don't appear in the final published HTML (unlike the <!-- comments).
Co-authored-by: Aleksandar Petrov <[email protected]>
* Restructure Pyroscope documentation and share content * Fix typo * Fix heading match for profile types * Restructure for configure server * Apply suggestions from code review Co-authored-by: Taylor C <[email protected]> * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Aleksandar Petrov <[email protected]> * Apply suggestions from code review Co-authored-by: Aleksandar Petrov <[email protected]> --------- Co-authored-by: Taylor C <[email protected]> Co-authored-by: Aleksandar Petrov <[email protected]> (cherry picked from commit f9b116c)
Restructure Pyroscope documentation to share content to other profiles-related content.
These changes will go live with the next Pyroscope release and should not be backported.
Notes for reviewer
Part of https://github.com/grafana/pyroscope-squad/issues/223 (reference the mind map linked in the issue for the file changes)
You can check out the PR using
gh pr checkout 3798
in your local tempo repo.Changes to check:
Blocks these PRs: