-
Notifications
You must be signed in to change notification settings - Fork 238
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
feat(pyroscope/scrape): add support for configuring CPU profile's duration scraped by pyroscope.scrape
#591
Changes from 3 commits
4aefd9c
4e08a85
7773517
e835d35
5304d84
777562f
fd9c97c
581b887
f3d89f6
0a35136
771fcbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,7 @@ Name | Type | Description | |
`params` | `map(list(string))` | A set of query parameters with which the target is scraped. | | no | ||
`scrape_interval` | `duration` | How frequently to scrape the targets of this scrape configuration. | `"15s"` | no | ||
`scrape_timeout` | `duration` | The timeout for scraping targets of this configuration. Must be larger than `scrape_interval`. | `"18s"` | no | ||
`profiling_duration`| `duration` | The duration for a delta profiling to be scraped. Must be larger than 1 second. | `"14s"` | no | ||
`scheme` | `string` | The URL scheme with which to fetch metrics from targets. | `"http"` | no | ||
`bearer_token_file` | `string` | File containing a bearer token to authenticate with. | | no | ||
`bearer_token` | `secret` | Bearer token to authenticate with. | | no | ||
|
@@ -395,8 +396,10 @@ When the `delta` argument is `false`, the [pprof][] HTTP query will be instantan | |
When the `delta` argument is `true`: | ||
* The [pprof][] HTTP query will run for a certain amount of time. | ||
* A `seconds` parameter is automatically added to the HTTP request. | ||
* The `seconds` used will be equal to `scrape_interval - 1`. | ||
For example, if `scrape_interval` is `"15s"`, `seconds` will be 14 seconds. | ||
* The default value for the `seconds` query parameter is `scrape_interval - 1`. | ||
If you set `profiling_duration`, then `seconds` is assigned the same value as `profiling_duration`. | ||
For example, if you set `scrape_interval` to `"15s"`, then `seconds` defaults to `14s`. | ||
If you set `profiling_duration` to `16s`, then `seconds` is set to `16s` regardless of the `scrape_interval` value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @simonswine @korniltsev does it make sense for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should always be less. If you try to profile a service which is already running a profile, you'll get an error
I think we should either add some validation, or default back to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that makes sense as well, yeah cc @hainenber There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added the validation in the HEAD. Looks A-OK to you guys? |
||
If the HTTP endpoint is `/debug/pprof/profile`, then the HTTP query will become `/debug/pprof/profile?seconds=14` | ||
|
||
## Exported fields | ||
|
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.
Sorry, one last thing. Could we name this with a little more descriptive name?
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 changed this in HEAD. PTAL