From f3d89f6c6cb3ca80c67da1b4fd0c1947a144e08e 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 | 98 +++++++++---------- 5 files changed, 73 insertions(+), 73 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 93f256356f..ff994fcb4a 100644 --- a/internal/component/pyroscope/scrape/scrape.go +++ b/internal/component/pyroscope/scrape/scrape.go @@ -63,7 +63,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 @@ -196,12 +196,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, } } @@ -224,11 +224,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 10a5c9c9dd..4d8ae93990 100644 --- a/internal/component/pyroscope/scrape/scrape_test.go +++ b/internal/component/pyroscope/scrape/scrape_test.go @@ -151,25 +151,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: ` @@ -196,7 +196,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 a05bbfb0b0..c7dd581e9e 100644 --- a/internal/component/pyroscope/scrape/target.go +++ b/internal/component/pyroscope/scrape/target.go @@ -407,8 +407,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 d7793a6156..f7e825ff3a 100644 --- a/internal/component/pyroscope/scrape/target_test.go +++ b/internal/component/pyroscope/scrape/target_test.go @@ -238,12 +238,59 @@ func Test_targetsFromGroup(t *testing.T) { require.Empty(t, dropped) } -func Test_targetsFromGroup_withSpecifiedProfilingDuration(t *testing.T) { +// Test that the godeltaprof is not surfaced publicly +func Test_NewTarget_godeltaprof(t *testing.T) { + withGodeltaprof := NewTarget( + labels.FromMap(map[string]string{ + model.AddressLabel: "localhost:9094", + serviceNameLabel: "docker-container", + model.MetricNameLabel: pprofGoDeltaProfMemory, + ProfilePath: "/debug/pprof/delta_heap", + model.SchemeLabel: "http", + "foo": "bar", + "instance": "localhost:9094", + }), + labels.FromMap(map[string]string{ + model.AddressLabel: "localhost:9094", + "__meta_docker_container_name": "docker-container", + model.MetricNameLabel: pprofGoDeltaProfMemory, + ProfilePath: "/debug/pprof/delta_heap", + model.SchemeLabel: "http", + "foo": "bar", + }), + url.Values{}, + ) + withoutGodeltaprof := NewTarget( + labels.FromMap(map[string]string{ + model.AddressLabel: "localhost:9094", + serviceNameLabel: "docker-container", + model.MetricNameLabel: pprofMemory, + ProfilePath: "/debug/pprof/heap", + model.SchemeLabel: "http", + "foo": "bar", + "instance": "localhost:9094", + }), + labels.FromMap(map[string]string{ + model.AddressLabel: "localhost:9094", + "__meta_docker_container_name": "docker-container", + model.MetricNameLabel: pprofMemory, + ProfilePath: "/debug/pprof/heap", + model.SchemeLabel: "http", + "foo": "bar", + }), + url.Values{}, + ) + + require.NotEqual(t, withGodeltaprof.allLabels, withoutGodeltaprof.allLabels) + require.Equal(t, withGodeltaprof.publicLabels, withoutGodeltaprof.publicLabels) +} + +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{ @@ -299,50 +346,3 @@ func Test_targetsFromGroup_withSpecifiedProfilingDuration(t *testing.T) { 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( - labels.FromMap(map[string]string{ - model.AddressLabel: "localhost:9094", - serviceNameLabel: "docker-container", - model.MetricNameLabel: pprofGoDeltaProfMemory, - ProfilePath: "/debug/pprof/delta_heap", - model.SchemeLabel: "http", - "foo": "bar", - "instance": "localhost:9094", - }), - labels.FromMap(map[string]string{ - model.AddressLabel: "localhost:9094", - "__meta_docker_container_name": "docker-container", - model.MetricNameLabel: pprofGoDeltaProfMemory, - ProfilePath: "/debug/pprof/delta_heap", - model.SchemeLabel: "http", - "foo": "bar", - }), - url.Values{}, - ) - withoutGodeltaprof := NewTarget( - labels.FromMap(map[string]string{ - model.AddressLabel: "localhost:9094", - serviceNameLabel: "docker-container", - model.MetricNameLabel: pprofMemory, - ProfilePath: "/debug/pprof/heap", - model.SchemeLabel: "http", - "foo": "bar", - "instance": "localhost:9094", - }), - labels.FromMap(map[string]string{ - model.AddressLabel: "localhost:9094", - "__meta_docker_container_name": "docker-container", - model.MetricNameLabel: pprofMemory, - ProfilePath: "/debug/pprof/heap", - model.SchemeLabel: "http", - "foo": "bar", - }), - url.Values{}, - ) - - require.NotEqual(t, withGodeltaprof.allLabels, withoutGodeltaprof.allLabels) - require.Equal(t, withGodeltaprof.publicLabels, withoutGodeltaprof.publicLabels) -}