Skip to content

Commit

Permalink
feat(pyroscope/scrape): rename profiling_duration to more comprehen…
Browse files Browse the repository at this point in the history
…sible `delta_profiling_duration` + stricter validation

Signed-off-by: hainenber <[email protected]>
  • Loading branch information
hainenber committed May 1, 2024
1 parent 309ecdb commit 5be0053
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 88 deletions.
8 changes: 4 additions & 4 deletions docs/sources/reference/components/pyroscope.scrape.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +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
`delta_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
Expand Down Expand Up @@ -397,10 +397,10 @@ 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 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`.
However, the `profiling_duration` cannot be larger than `scrape_interval`.
If you set `delta_profiling_duration`, then `seconds` is assigned the same value as `delta_profiling_duration`.
However, the `delta_profiling_duration` cannot be larger than `scrape_interval`.
For example, if you set `scrape_interval` to `"15s"`, then `seconds` defaults to `14s`
If you set `profiling_duration` to `16s`, then `scrape_interval` must be set to at least `17s`.
If you set `delta_profiling_duration` to `16s`, then `scrape_interval` must be set to at least `17s`.
If the HTTP endpoint is `/debug/pprof/profile`, then the HTTP query will become `/debug/pprof/profile?seconds=14`

## Exported fields
Expand Down
22 changes: 11 additions & 11 deletions internal/component/pyroscope/scrape/scrape.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type Arguments struct {
// The URL scheme with which to fetch metrics from targets.
Scheme string `alloy:"scheme,attr,optional"`
// The duration for a profile to be scrapped.
ProfilingDuration time.Duration `alloy:"profiling_duration,attr,optional"`
DeltaProfilingDuration time.Duration `alloy:"delta_profiling_duration,attr,optional"`

// todo(ctovena): add support for limits.
// // An uncompressed response body larger than this many bytes will cause the
Expand Down Expand Up @@ -198,12 +198,12 @@ var DefaultArguments = NewDefaultArguments()
// NewDefaultArguments create the default settings for a scrape job.
func NewDefaultArguments() Arguments {
return Arguments{
Scheme: "http",
HTTPClientConfig: component_config.DefaultHTTPClientConfig,
ScrapeInterval: 15 * time.Second,
ScrapeTimeout: 10 * time.Second,
ProfilingConfig: DefaultProfilingConfig,
ProfilingDuration: defaultProfilingDuration,
Scheme: "http",
HTTPClientConfig: component_config.DefaultHTTPClientConfig,
ScrapeInterval: 15 * time.Second,
ScrapeTimeout: 10 * time.Second,
ProfilingConfig: DefaultProfilingConfig,
DeltaProfilingDuration: defaultProfilingDuration,
}
}

Expand All @@ -226,11 +226,11 @@ func (arg *Arguments) Validate() error {
return fmt.Errorf("scrape_interval must be at least 2 seconds when using delta profiling")
}
if target.Enabled && target.Delta {
if arg.ProfilingDuration.Seconds() <= 1 {
return fmt.Errorf("profiling_duration must be larger than 1 second when using delta profiling")
if arg.DeltaProfilingDuration.Seconds() <= 1 {
return fmt.Errorf("delta_profiling_duration must be larger than 1 second when using delta profiling")
}
if arg.ProfilingDuration.Seconds() >= arg.ScrapeInterval.Seconds() {
return fmt.Errorf("profiling_duration must be smaller than scrape_interval when using delta profiling")
if arg.DeltaProfilingDuration.Seconds() > arg.ScrapeInterval.Seconds()-1 {
return fmt.Errorf("delta_profiling_duration must be at least 1 second smaller than scrape_interval when using delta profiling")
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions internal/component/pyroscope/scrape/scrape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,25 +160,25 @@ func TestUnmarshalConfig(t *testing.T) {
`,
expectedErr: "scrape_interval must be at least 2 seconds when using delta profiling",
},
"invalid cpu profiling_duration": {
"invalid cpu delta_profiling_duration": {
in: `
targets = []
forward_to = null
scrape_timeout = "1s"
scrape_interval = "10s"
profiling_duration = "1s"
delta_profiling_duration = "1s"
`,
expectedErr: "profiling_duration must be larger than 1 second when using delta profiling",
expectedErr: "delta_profiling_duration must be larger than 1 second when using delta profiling",
},
"erroneous cpu profiling_duration": {
"erroneous cpu delta_profiling_duration": {
in: `
targets = []
forward_to = null
scrape_timeout = "1s"
scrape_interval = "10s"
profiling_duration = "12s"
delta_profiling_duration = "12s"
`,
expectedErr: "profiling_duration must be smaller than scrape_interval when using delta profiling",
expectedErr: "delta_profiling_duration must be at least 1 second smaller than scrape_interval when using delta profiling",
},
"allow short scrape_intervals without delta": {
in: `
Expand All @@ -205,7 +205,7 @@ func TestUnmarshalConfig(t *testing.T) {
forward_to = null
scrape_timeout = "5s"
scrape_interval = "3s"
profiling_duration = "2s"
delta_profiling_duration = "2s"
bearer_token = "token"
bearer_token_file = "/path/to/file.token"
`,
Expand Down
4 changes: 2 additions & 2 deletions internal/component/pyroscope/scrape/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,8 @@ func targetsFromGroup(group *targetgroup.Group, cfg Arguments, targetTypes map[s

if pcfg, found := targetTypes[profType]; found && pcfg.Delta {
seconds := (cfg.ScrapeInterval)/time.Second - 1
if cfg.ProfilingDuration != defaultProfilingDuration {
seconds = (cfg.ProfilingDuration) / time.Second
if cfg.DeltaProfilingDuration != defaultProfilingDuration {
seconds = (cfg.DeltaProfilingDuration) / time.Second
}
params.Add("seconds", strconv.Itoa(int(seconds)))
}
Expand Down
66 changes: 2 additions & 64 deletions internal/component/pyroscope/scrape/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,68 +238,6 @@ func Test_targetsFromGroup(t *testing.T) {
require.Empty(t, dropped)
}

func Test_targetsFromGroup_withSpecifiedProfilingDuration(t *testing.T) {
args := NewDefaultArguments()
args.ProfilingConfig.Block.Enabled = false
args.ProfilingConfig.Goroutine.Enabled = false
args.ProfilingConfig.Mutex.Enabled = false
args.ProfilingDuration = 20 * time.Second

active, dropped, err := targetsFromGroup(&targetgroup.Group{
Targets: []model.LabelSet{
{model.AddressLabel: "localhost:9090"},
},
Labels: model.LabelSet{
"foo": "bar",
},
}, args, args.ProfilingConfig.AllTargets())
expected := []*Target{
// unspecified
NewTarget(
labels.FromMap(map[string]string{
model.AddressLabel: "localhost:9090",
serviceNameLabel: "unspecified",
model.MetricNameLabel: pprofMemory,
ProfilePath: "/debug/pprof/allocs",
model.SchemeLabel: "http",
"foo": "bar",
"instance": "localhost:9090",
}),
labels.FromMap(map[string]string{
model.AddressLabel: "localhost:9090",
model.MetricNameLabel: pprofMemory,
ProfilePath: "/debug/pprof/allocs",
model.SchemeLabel: "http",
"foo": "bar",
}),
url.Values{}),
NewTarget(
labels.FromMap(map[string]string{
model.AddressLabel: "localhost:9090",
serviceNameLabel: "unspecified",
model.MetricNameLabel: pprofProcessCPU,
ProfilePath: "/debug/pprof/profile",
model.SchemeLabel: "http",
"foo": "bar",
"instance": "localhost:9090",
}),
labels.FromMap(map[string]string{
model.AddressLabel: "localhost:9090",
model.MetricNameLabel: pprofProcessCPU,
ProfilePath: "/debug/pprof/profile",
model.SchemeLabel: "http",
"foo": "bar",
}),
url.Values{"seconds": []string{"20"}}),
}

require.NoError(t, err)
sort.Sort(Targets(active))
sort.Sort(Targets(expected))
require.Equal(t, expected, active)
require.Empty(t, dropped)
}

// Test that the godeltaprof is not surfaced publicly
func Test_NewTarget_godeltaprof(t *testing.T) {
withGodeltaprof := NewTarget(
Expand Down Expand Up @@ -347,12 +285,12 @@ func Test_NewTarget_godeltaprof(t *testing.T) {
require.Equal(t, withGodeltaprof.publicLabels, withoutGodeltaprof.publicLabels)
}

func Test_targetsFromGroup_withSpecifiedProfilingDuration(t *testing.T) {
func Test_targetsFromGroup_withSpecifiedDeltaProfilingDuration(t *testing.T) {
args := NewDefaultArguments()
args.ProfilingConfig.Block.Enabled = false
args.ProfilingConfig.Goroutine.Enabled = false
args.ProfilingConfig.Mutex.Enabled = false
args.ProfilingDuration = 20 * time.Second
args.DeltaProfilingDuration = 20 * time.Second

active, dropped, err := targetsFromGroup(&targetgroup.Group{
Targets: []model.LabelSet{
Expand Down

0 comments on commit 5be0053

Please sign in to comment.