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
profiler: support using custom profiler labels in our UI #2282
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.
3b6917b
to
742c1b3
Compare
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. |
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. |
The backend changes are deployed, and I tested this again with Alex, using the updated |
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, but see comments. 🙇
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.
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.
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!