profiler: support using custom profiler labels in our UI#2282
Merged
Conversation
BenchmarksBenchmark execution time: 2024-01-02 17:24:12 Comparing candidate commit 78e3abf in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 41 metrics, 0 unstable metrics. |
Users can use profiler labels to add annotations to CPU and goroutine profiles. Those labels will be in the profiles we upload, but won't currently be available as attributes for filtering frames in flame graphs in our UI. Add an API for specifying pprof label keys which should show up in the UI. There is a limit today to avoid worst-case situations with massive numbers of unique label keys.
nsrip-dd
force-pushed
the
nick.ripley/profiler-custom-context
branch
from
October 20, 2023 09:47
3b6917b to
742c1b3
Compare
nsrip-dd
changed the title
nsrip-dd
marked this pull request as ready for review
October 20, 2023 10:02
nsrip-dd
marked this pull request as draft
October 31, 2023 17:19
Contributor
|
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Contributor
|
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
Contributor
Author
|
The backend changes are deployed, and I tested this again with Alex, using the updated |
nsrip-dd
marked this pull request as ready for review
December 21, 2023 19:22
felixge
self-requested a review
January 2, 2024 12:11
felixge
approved these changes
Jan 2, 2024
To clarify that these are *just* keys, not key/value pairs like for the pprof.Labels API.
Just record that they keys are used. We can keep them in the startup logs since those belong to the user, but I think we shouldn't send more specific user data than we need for the telemetry since that doesn't go to the user's org.
Alex has suggested to start with a low limit and track usage to see if it's worth increasing. Modify the telemetry to record the number of keys so we can see typical usage.
felixge
approved these changes
Jan 3, 2024
felixge
left a comment
felixge
left a comment
Member
There was a problem hiding this comment.
Thanks for the updates. LGTM 🙇 – Ship it!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Add an API for specifying profiler label keys which should show up in the UI.
There is a limit today to avoid worst-case situations with massive numbers of
labels.
This PR has a unit test, and I also tested it on our staging environment (with
help from Alex)
Motivation
Users can use profiler labels to add annotations to CPU and goroutine profiles.
Those labels will be in the profiles we upload, but won't currently be
available as attributes for filtering flame graphs in our UI. We have support
for that now in our backend & UI, so we just need a way to make it
available to users.
(for internal use: see PROF-8379)
Reviewer's Checklist
For Datadog employees:
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!