From 5be00535075e164cc7481b03db6d305b0a9752c6 Mon Sep 17 00:00:00 2001 From: hainenber Date: Wed, 1 May 2024 17:53:15 +0700 Subject: [PATCH] feat(pyroscope/scrape): rename `profiling_duration` to more comprehensible `delta_profiling_duration` + stricter validation Signed-off-by: hainenber --- .../reference/components/pyroscope.scrape.md | 8 +-- internal/component/pyroscope/scrape/scrape.go | 22 +++---- .../component/pyroscope/scrape/scrape_test.go | 14 ++-- internal/component/pyroscope/scrape/target.go | 4 +- .../component/pyroscope/scrape/target_test.go | 66 +------------------ 5 files changed, 26 insertions(+), 88 deletions(-) diff --git a/docs/sources/reference/components/pyroscope.scrape.md b/docs/sources/reference/components/pyroscope.scrape.md index de3493d9d9..422e79da1b 100644 --- a/docs/sources/reference/components/pyroscope.scrape.md +++ b/docs/sources/reference/components/pyroscope.scrape.md @@ -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 @@ -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 diff --git a/internal/component/pyroscope/scrape/scrape.go b/internal/component/pyroscope/scrape/scrape.go index 2032349949..8989c55d55 100644 --- a/internal/component/pyroscope/scrape/scrape.go +++ b/internal/component/pyroscope/scrape/scrape.go @@ -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 @@ -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, } } @@ -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") } } } diff --git a/internal/component/pyroscope/scrape/scrape_test.go b/internal/component/pyroscope/scrape/scrape_test.go index 8bc5755a4f..31d960da02 100644 --- a/internal/component/pyroscope/scrape/scrape_test.go +++ b/internal/component/pyroscope/scrape/scrape_test.go @@ -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: ` @@ -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" `, diff --git a/internal/component/pyroscope/scrape/target.go b/internal/component/pyroscope/scrape/target.go index b2335e1305..c90144e4c1 100644 --- a/internal/component/pyroscope/scrape/target.go +++ b/internal/component/pyroscope/scrape/target.go @@ -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))) } diff --git a/internal/component/pyroscope/scrape/target_test.go b/internal/component/pyroscope/scrape/target_test.go index 6f630bdbb4..f7e825ff3a 100644 --- a/internal/component/pyroscope/scrape/target_test.go +++ b/internal/component/pyroscope/scrape/target_test.go @@ -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( @@ -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{