From 2686177fa4616d59cf573ba4e36e1ea24eab103f Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 27 Aug 2024 15:07:54 +0200 Subject: [PATCH 01/10] [receiver/prometheus]: also consider scrape config files when setting up receiver Signed-off-by: Florian Bacher --- .../prometheus-receiver-scrape-config.yaml | 27 ++++++++ .../prometheusreceiver/metrics_receiver.go | 7 +- ...trics_receiver_scrape_config_files_test.go | 69 +++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 .chloggen/prometheus-receiver-scrape-config.yaml create mode 100644 receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go diff --git a/.chloggen/prometheus-receiver-scrape-config.yaml b/.chloggen/prometheus-receiver-scrape-config.yaml new file mode 100644 index 000000000000..b2b5d3db7c4c --- /dev/null +++ b/.chloggen/prometheus-receiver-scrape-config.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: prometheusreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix the retrieval of scrape configurations by also considering scrape config files + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34786] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/receiver/prometheusreceiver/metrics_receiver.go b/receiver/prometheusreceiver/metrics_receiver.go index 8868cb71534f..09e53f5e8b12 100644 --- a/receiver/prometheusreceiver/metrics_receiver.go +++ b/receiver/prometheusreceiver/metrics_receiver.go @@ -363,7 +363,12 @@ func (r *pReceiver) applyCfg(cfg *PromConfig) error { } discoveryCfg := make(map[string]discovery.Configs) - for _, scrapeConfig := range cfg.ScrapeConfigs { + + scrapeConfigs, err := (*config.Config)(cfg).GetScrapeConfigs() + if err != nil { + return fmt.Errorf("could not get scrape configs: %w", err) + } + for _, scrapeConfig := range scrapeConfigs { discoveryCfg[scrapeConfig.JobName] = scrapeConfig.ServiceDiscoveryConfigs r.settings.Logger.Info("Scrape job added", zap.String("jobName", scrapeConfig.JobName)) } diff --git a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go new file mode 100644 index 000000000000..3db01bdf005c --- /dev/null +++ b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go @@ -0,0 +1,69 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package prometheusreceiver + +import ( + "fmt" + "github.com/stretchr/testify/assert" + + "github.com/prometheus/prometheus/config" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/pdata/pmetric" + semconv "go.opentelemetry.io/collector/semconv/v1.27.0" + "gopkg.in/yaml.v2" + "os" + "testing" +) + +var scrapeFileTargetPage = ` +# A simple counter +# TYPE foo counter +foo1 1 +` + +// TestScrapeConfigFiles validates that scrape configs provided via scrape_config_files are +// also considered and added to the applied configuration + +func TestScrapeConfigFiles(t *testing.T) { + setMetricsTimestamp() + targets := []*testData{ + { + name: "target1", + pages: []mockPrometheusResponse{ + {code: 200, data: scrapeFileTargetPage}, + }, + validateFunc: verifyScrapeConfigFiles, + }, + } + + testComponent(t, targets, func(cfg *Config) { + // take the generated scrape config and move it into a file instead + marshalledScrapeConfigs, err := yaml.Marshal(cfg.PrometheusConfig.ScrapeConfigs) + require.Nil(t, err) + tmpDir := t.TempDir() + cfgFileName := fmt.Sprintf("%s/test-scrape-config.yaml", tmpDir) + scrapeConfigFileContent := fmt.Sprintf("scrape_configs:\n%s", string(marshalledScrapeConfigs)) + err = os.WriteFile(cfgFileName, []byte(scrapeConfigFileContent), 0400) + require.Nil(t, err) + cfg.PrometheusConfig.ScrapeConfigs = []*config.ScrapeConfig{} + cfg.PrometheusConfig.ScrapeConfigFiles = []string{cfgFileName} + }) +} + +func verifyScrapeConfigFiles(t *testing.T, td *testData, result []pmetric.ResourceMetrics) { + require.Len(t, result, 1) + serviceName, ok := result[0].Resource().Attributes().Get(semconv.AttributeServiceName) + assert.True(t, ok) + assert.Equal(t, "target1", serviceName.Str()) + assert.Equal(t, 6, result[0].ScopeMetrics().At(0).Metrics().Len()) + metricFound := false + + for i := 0; i < result[0].ScopeMetrics().At(0).Metrics().Len(); i++ { + if result[0].ScopeMetrics().At(0).Metrics().At(i).Name() == "foo1" { + metricFound = true + break + } + } + assert.True(t, metricFound) +} From 977e86e1b57fdba4097bf92f78727d064ab2b60e Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 28 Aug 2024 07:46:49 +0200 Subject: [PATCH 02/10] replace all occurrences of cfg.ScrapeConfigs with cfg.GetScrapeConfigs Signed-off-by: Florian Bacher --- receiver/prometheusreceiver/config.go | 24 +++++++++++++++++-- receiver/prometheusreceiver/config_test.go | 13 ++++++++++ .../prometheusreceiver/metrics_receiver.go | 11 +++++---- .../testdata/config_scrape_config_files.yaml | 8 +++++++ .../testdata/scrape-config.yaml | 3 +++ 5 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 receiver/prometheusreceiver/testdata/config_scrape_config_files.yaml create mode 100644 receiver/prometheusreceiver/testdata/scrape-config.yaml diff --git a/receiver/prometheusreceiver/config.go b/receiver/prometheusreceiver/config.go index c4269efde038..322a139211d7 100644 --- a/receiver/prometheusreceiver/config.go +++ b/receiver/prometheusreceiver/config.go @@ -42,12 +42,27 @@ type Config struct { // Validate checks the receiver configuration is valid. func (cfg *Config) Validate() error { - if (cfg.PrometheusConfig == nil || len(cfg.PrometheusConfig.ScrapeConfigs) == 0) && cfg.TargetAllocator == nil { + if !containsScrapeConfig(cfg) && cfg.TargetAllocator == nil { return errors.New("no Prometheus scrape_configs or target_allocator set") } return nil } +func containsScrapeConfig(cfg *Config) bool { + if cfg.PrometheusConfig == nil { + return false + } + scrapeConfigs, err := (*promconfig.Config)(cfg.PrometheusConfig).GetScrapeConfigs() + if err != nil { + return false + } + + if len(scrapeConfigs) == 0 { + return false + } + return true +} + type TargetAllocator struct { confighttp.ClientConfig `mapstructure:",squash"` Interval time.Duration `mapstructure:"interval"` @@ -110,7 +125,12 @@ func (cfg *PromConfig) Validate() error { return fmt.Errorf("unsupported features:\n\t%s", strings.Join(unsupportedFeatures, "\n\t")) } - for _, sc := range cfg.ScrapeConfigs { + scrapeConfigs, err := (*promconfig.Config)(cfg).GetScrapeConfigs() + if err != nil { + return err + } + + for _, sc := range scrapeConfigs { if err := validateHTTPClientConfig(&sc.HTTPClientConfig); err != nil { return err } diff --git a/receiver/prometheusreceiver/config_test.go b/receiver/prometheusreceiver/config_test.go index 6ba48222ce92..a1028ab51370 100644 --- a/receiver/prometheusreceiver/config_test.go +++ b/receiver/prometheusreceiver/config_test.go @@ -106,6 +106,19 @@ func TestLoadTargetAllocatorConfig(t *testing.T) { assert.Equal(t, promModel.Duration(5*time.Second), r2.PrometheusConfig.ScrapeConfigs[0].ScrapeInterval) } +func TestValidateConfigWithScrapeConfigFiles(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_scrape_config_files.yaml")) + require.NoError(t, err) + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + + sub, err := cm.Sub(component.NewIDWithName(metadata.Type, "").String()) + require.NoError(t, err) + require.NoError(t, sub.Unmarshal(cfg)) + + require.NoError(t, component.ValidateConfig(cfg)) +} + func TestLoadConfigFailsOnUnknownSection(t *testing.T) { cm, err := confmaptest.LoadConf(filepath.Join("testdata", "invalid-config-section.yaml")) require.NoError(t, err) diff --git a/receiver/prometheusreceiver/metrics_receiver.go b/receiver/prometheusreceiver/metrics_receiver.go index 09e53f5e8b12..2bf930a5cf2f 100644 --- a/receiver/prometheusreceiver/metrics_receiver.go +++ b/receiver/prometheusreceiver/metrics_receiver.go @@ -351,9 +351,14 @@ func (r *pReceiver) getScrapeConfigsResponse(baseURL string) (map[string]*config } func (r *pReceiver) applyCfg(cfg *PromConfig) error { + scrapeConfigs, err := (*config.Config)(cfg).GetScrapeConfigs() + if err != nil { + return fmt.Errorf("could not get scrape configs: %w", err) + } + if !enableNativeHistogramsGate.IsEnabled() { // Enforce scraping classic histograms to avoid dropping them. - for _, scrapeConfig := range cfg.ScrapeConfigs { + for _, scrapeConfig := range scrapeConfigs { scrapeConfig.ScrapeClassicHistograms = true } } @@ -364,10 +369,6 @@ func (r *pReceiver) applyCfg(cfg *PromConfig) error { discoveryCfg := make(map[string]discovery.Configs) - scrapeConfigs, err := (*config.Config)(cfg).GetScrapeConfigs() - if err != nil { - return fmt.Errorf("could not get scrape configs: %w", err) - } for _, scrapeConfig := range scrapeConfigs { discoveryCfg[scrapeConfig.JobName] = scrapeConfig.ServiceDiscoveryConfigs r.settings.Logger.Info("Scrape job added", zap.String("jobName", scrapeConfig.JobName)) diff --git a/receiver/prometheusreceiver/testdata/config_scrape_config_files.yaml b/receiver/prometheusreceiver/testdata/config_scrape_config_files.yaml new file mode 100644 index 000000000000..271a28933462 --- /dev/null +++ b/receiver/prometheusreceiver/testdata/config_scrape_config_files.yaml @@ -0,0 +1,8 @@ +prometheus: + trim_metric_suffixes: true + use_start_time_metric: true + start_time_metric_regex: '^(.+_)*process_start_time_seconds$' + report_extra_scrape_metrics: true + config: + scrape_config_files: + - ./testdata/scrape-config.yaml diff --git a/receiver/prometheusreceiver/testdata/scrape-config.yaml b/receiver/prometheusreceiver/testdata/scrape-config.yaml new file mode 100644 index 000000000000..712a1edd3b52 --- /dev/null +++ b/receiver/prometheusreceiver/testdata/scrape-config.yaml @@ -0,0 +1,3 @@ +scrape_configs: + - job_name: 'demo' + scrape_interval: 5s \ No newline at end of file From d3fc9d61ce9a7d4739f19d2aeea61906e1b1dd54 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 28 Aug 2024 07:55:29 +0200 Subject: [PATCH 03/10] fix changelog entry Signed-off-by: Florian Bacher --- .chloggen/prometheus-receiver-scrape-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/prometheus-receiver-scrape-config.yaml b/.chloggen/prometheus-receiver-scrape-config.yaml index b2b5d3db7c4c..d7440600fb6e 100644 --- a/.chloggen/prometheus-receiver-scrape-config.yaml +++ b/.chloggen/prometheus-receiver-scrape-config.yaml @@ -1,7 +1,7 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: +change_type: bug_fix # The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) component: prometheusreceiver From 65f9b7535ee0e03e3ac2849a6be83cf2e84b1df7 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 28 Aug 2024 08:08:09 +0200 Subject: [PATCH 04/10] fix linting Signed-off-by: Florian Bacher --- .../metrics_receiver_scrape_config_files_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go index 3db01bdf005c..45ae793fb18f 100644 --- a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go +++ b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go @@ -51,7 +51,7 @@ func TestScrapeConfigFiles(t *testing.T) { }) } -func verifyScrapeConfigFiles(t *testing.T, td *testData, result []pmetric.ResourceMetrics) { +func verifyScrapeConfigFiles(t *testing.T, _ *testData, result []pmetric.ResourceMetrics) { require.Len(t, result, 1) serviceName, ok := result[0].Resource().Attributes().Get(semconv.AttributeServiceName) assert.True(t, ok) From 6ed882653929f441d9acdbdaff799974e8dd1cb8 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 28 Aug 2024 08:40:55 +0200 Subject: [PATCH 05/10] fix linting Signed-off-by: Florian Bacher --- .../metrics_receiver_scrape_config_files_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go index 45ae793fb18f..64f23dea4a3b 100644 --- a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go +++ b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go @@ -5,15 +5,15 @@ package prometheusreceiver import ( "fmt" - "github.com/stretchr/testify/assert" + "os" + "testing" "github.com/prometheus/prometheus/config" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pmetric" semconv "go.opentelemetry.io/collector/semconv/v1.27.0" "gopkg.in/yaml.v2" - "os" - "testing" ) var scrapeFileTargetPage = ` From 76e079c465cd700a2ce6eed5bae60d4099610812 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 28 Aug 2024 15:42:42 +0200 Subject: [PATCH 06/10] adapt to changes from main branch Signed-off-by: Florian Bacher --- receiver/prometheusreceiver/targetallocator/manager.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/receiver/prometheusreceiver/targetallocator/manager.go b/receiver/prometheusreceiver/targetallocator/manager.go index 67d29fb297d5..06b3f2802d6d 100644 --- a/receiver/prometheusreceiver/targetallocator/manager.go +++ b/receiver/prometheusreceiver/targetallocator/manager.go @@ -158,6 +158,10 @@ func (m *Manager) sync(compareHash uint64, httpClient *http.Client) (uint64, err } func (m *Manager) applyCfg() error { + scrapeConfigs, err := m.promCfg.GetScrapeConfigs() + if err != nil { + return fmt.Errorf("could not get scrape configs: %w", err) + } if !m.enableNativeHistograms { // Enforce scraping classic histograms to avoid dropping them. for _, scrapeConfig := range m.promCfg.ScrapeConfigs { @@ -170,7 +174,7 @@ func (m *Manager) applyCfg() error { } discoveryCfg := make(map[string]discovery.Configs) - for _, scrapeConfig := range m.promCfg.ScrapeConfigs { + for _, scrapeConfig := range scrapeConfigs { discoveryCfg[scrapeConfig.JobName] = scrapeConfig.ServiceDiscoveryConfigs m.settings.Logger.Info("Scrape job added", zap.String("jobName", scrapeConfig.JobName)) } From d8416c58bd3a193b1d2f35ff6e11d70458d697f5 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 29 Aug 2024 07:35:30 +0200 Subject: [PATCH 07/10] trigger CI checks Signed-off-by: Florian Bacher From 41c271affcec8916551ec31695cb79cb49339191 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 29 Aug 2024 08:46:20 +0200 Subject: [PATCH 08/10] trigger CI checks Signed-off-by: Florian Bacher From 44afa74561dc7f44f120204f98709d2abdb13f7f Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 5 Sep 2024 07:11:34 +0200 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Antoine Toulme --- receiver/prometheusreceiver/config.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/receiver/prometheusreceiver/config.go b/receiver/prometheusreceiver/config.go index be6547f8d6c3..672f0e437b2c 100644 --- a/receiver/prometheusreceiver/config.go +++ b/receiver/prometheusreceiver/config.go @@ -55,10 +55,7 @@ func containsScrapeConfig(cfg *Config) bool { return false } - if len(scrapeConfigs) == 0 { - return false - } - return true + return len(scrapeConfigs) > 0 } // PromConfig is a redeclaration of promconfig.Config because we need custom unmarshaling From 927cf8768909b0d73979f33558555427509fd225 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Thu, 5 Sep 2024 12:40:25 +0200 Subject: [PATCH 10/10] fix linting Signed-off-by: Florian Bacher --- .../metrics_receiver_scrape_config_files_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go index 64f23dea4a3b..67fa4e4105b0 100644 --- a/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go +++ b/receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go @@ -40,12 +40,12 @@ func TestScrapeConfigFiles(t *testing.T) { testComponent(t, targets, func(cfg *Config) { // take the generated scrape config and move it into a file instead marshalledScrapeConfigs, err := yaml.Marshal(cfg.PrometheusConfig.ScrapeConfigs) - require.Nil(t, err) + require.NoError(t, err) tmpDir := t.TempDir() cfgFileName := fmt.Sprintf("%s/test-scrape-config.yaml", tmpDir) scrapeConfigFileContent := fmt.Sprintf("scrape_configs:\n%s", string(marshalledScrapeConfigs)) err = os.WriteFile(cfgFileName, []byte(scrapeConfigFileContent), 0400) - require.Nil(t, err) + require.NoError(t, err) cfg.PrometheusConfig.ScrapeConfigs = []*config.ScrapeConfig{} cfg.PrometheusConfig.ScrapeConfigFiles = []string{cfgFileName} })