From b900f01d5061f55477e0bdafdcab6f1fafea52f3 Mon Sep 17 00:00:00 2001 From: William Dumont Date: Thu, 12 Dec 2024 14:25:17 +0100 Subject: [PATCH 1/5] update static convert test to run the alloy controller on some conditions --- .../converter/internal/test_common/testing.go | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/internal/converter/internal/test_common/testing.go b/internal/converter/internal/test_common/testing.go index a315f0362c..30ce5cbfab 100644 --- a/internal/converter/internal/test_common/testing.go +++ b/internal/converter/internal/test_common/testing.go @@ -3,6 +3,7 @@ package test_common import ( "bufio" "bytes" + "context" "fmt" "io/fs" "os" @@ -12,6 +13,7 @@ import ( "strings" "testing" + "github.com/grafana/alloy/internal/component" "github.com/grafana/alloy/internal/converter/diag" "github.com/grafana/alloy/internal/featuregate" alloy_runtime "github.com/grafana/alloy/internal/runtime" @@ -20,6 +22,7 @@ import ( cluster_service "github.com/grafana/alloy/internal/service/cluster" http_service "github.com/grafana/alloy/internal/service/http" "github.com/grafana/alloy/internal/service/labelstore" + "github.com/grafana/alloy/internal/service/livedebugging" remotecfg_service "github.com/grafana/alloy/internal/service/remotecfg" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" @@ -214,14 +217,42 @@ func attemptLoadingAlloyConfig(t *testing.T, bb []byte) { clusterService, labelstore.New(nil, prometheus.DefaultRegisterer), remotecfgService, + livedebugging.New(), }, EnableCommunityComps: true, }) err = f.LoadSource(cfg, nil, "") + // This is a bit of an ugly workaround but the spanmetrics connector is starting a go routine in its start function. + // If the goroutine is not stopped, it will result in a panic. To stop it we need to run the Alloy controller to run the component and then stop it. + // We cannot run the Alloy controller for all tests because some components such as the statsd_exporter will panic after being stopped and some other components have wrong config. + runAlloyController := false + components, err := f.ListComponents("", component.InfoOptions{}) + require.NoError(t, err) + for _, component := range components { + if component.ComponentName == "otelcol.connector.spanmetrics" { + runAlloyController = true + break + } + } + + if runAlloyController { + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + f.Run(ctx) + close(done) + }() + defer func() { + cancel() + <-done + }() + } + // Many components will fail to build as e.g. the cert files are missing, so we ignore these errors. // This is not ideal, but we still validate for other potential issues. - if err != nil && strings.Contains(err.Error(), "Failed to build component") { + // Tests that require the Alloy controller to run should not have build error else the components won't run. + if !runAlloyController && err != nil && strings.Contains(err.Error(), "Failed to build component") { t.Log("ignoring error: " + err.Error()) return } From 7e9994409591bba64a74ab020571038b8a624f53 Mon Sep 17 00:00:00 2001 From: William Dumont Date: Thu, 12 Dec 2024 14:25:36 +0100 Subject: [PATCH 2/5] update azure and prometheus static converters --- .../internal/prometheusconvert/component/azure.go | 14 +++++++++++--- .../build/converter_remotewriteexporter.go | 11 +++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/converter/internal/prometheusconvert/component/azure.go b/internal/converter/internal/prometheusconvert/component/azure.go index 47505a5e09..9252933d64 100644 --- a/internal/converter/internal/prometheusconvert/component/azure.go +++ b/internal/converter/internal/prometheusconvert/component/azure.go @@ -25,12 +25,10 @@ func toDiscoveryAzure(sdConfig *prom_azure.SDConfig) *azure.Arguments { return nil } - return &azure.Arguments{ + args := &azure.Arguments{ Environment: sdConfig.Environment, Port: sdConfig.Port, SubscriptionID: sdConfig.SubscriptionID, - OAuth: toDiscoveryAzureOauth2(sdConfig.ClientID, sdConfig.TenantID, string(sdConfig.ClientSecret)), - ManagedIdentity: toManagedIdentity(sdConfig), RefreshInterval: time.Duration(sdConfig.RefreshInterval), ResourceGroup: sdConfig.ResourceGroup, ProxyConfig: common.ToProxyConfig(sdConfig.HTTPClientConfig.ProxyConfig), @@ -38,6 +36,16 @@ func toDiscoveryAzure(sdConfig *prom_azure.SDConfig) *azure.Arguments { EnableHTTP2: sdConfig.HTTPClientConfig.EnableHTTP2, TLSConfig: *common.ToTLSConfig(&sdConfig.HTTPClientConfig.TLSConfig), } + + // Only one auth method is allowed. + switch sdConfig.AuthenticationMethod { + case "ManagedIdentity": + args.ManagedIdentity = toManagedIdentity(sdConfig) + default: + args.OAuth = toDiscoveryAzureOauth2(sdConfig.ClientID, sdConfig.TenantID, string(sdConfig.ClientSecret)) + } + + return args } func ValidateDiscoveryAzure(sdConfig *prom_azure.SDConfig) diag.Diagnostics { diff --git a/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go b/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go index 23aa682ed3..0623cac283 100644 --- a/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go +++ b/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go @@ -91,13 +91,20 @@ func toremotewriteexporterConfig(cfg *remotewriteexporter.Config, forwardTo []st defaultArgs := &prometheus.Arguments{} defaultArgs.SetToDefault() - return &prometheus.Arguments{ + args := &prometheus.Arguments{ IncludeTargetInfo: defaultArgs.IncludeTargetInfo, IncludeScopeInfo: defaultArgs.IncludeScopeInfo, IncludeScopeLabels: defaultArgs.IncludeScopeLabels, - GCFrequency: cfg.StaleTime, + GCFrequency: defaultArgs.GCFrequency, ForwardTo: forwardTo, AddMetricSuffixes: defaultArgs.AddMetricSuffixes, ResourceToTelemetryConversion: defaultArgs.ResourceToTelemetryConversion, } + + // Override default only if > 0 because GCFrequency of 0 is not allowed + if cfg.StaleTime > 0 { + args.GCFrequency = cfg.StaleTime + } + + return args } From 1f9f8fa41be48fd4c4561d208d6a64b149048848 Mon Sep 17 00:00:00 2001 From: William Dumont Date: Thu, 12 Dec 2024 14:25:55 +0100 Subject: [PATCH 3/5] update tests --- .../internal/prometheusconvert/testdata/azure.alloy | 10 ---------- .../internal/prometheusconvert/testdata/azure.yaml | 1 + .../prometheusconvert/testdata/discovery.alloy | 8 -------- .../prometheusconvert/testdata/discovery_relabel.alloy | 4 ---- .../internal/promtailconvert/testdata/azure.alloy | 4 ---- .../internal/staticconvert/testdata/prom_scrape.alloy | 8 -------- .../internal/staticconvert/testdata/traces.alloy | 10 ++-------- 7 files changed, 3 insertions(+), 42 deletions(-) diff --git a/internal/converter/internal/prometheusconvert/testdata/azure.alloy b/internal/converter/internal/prometheusconvert/testdata/azure.alloy index 631f7a5d7a..6d8d7093fb 100644 --- a/internal/converter/internal/prometheusconvert/testdata/azure.alloy +++ b/internal/converter/internal/prometheusconvert/testdata/azure.alloy @@ -6,21 +6,11 @@ discovery.azure "prometheus1" { tenant_id = "tenant" client_secret = "secret" } - - managed_identity { - client_id = "client" - } } discovery.azure "prometheus2" { subscription_id = "subscription" - oauth { - client_id = "client" - tenant_id = "tenant" - client_secret = "secret" - } - managed_identity { client_id = "client" } diff --git a/internal/converter/internal/prometheusconvert/testdata/azure.yaml b/internal/converter/internal/prometheusconvert/testdata/azure.yaml index ca224eff7a..7f03fbaa25 100644 --- a/internal/converter/internal/prometheusconvert/testdata/azure.yaml +++ b/internal/converter/internal/prometheusconvert/testdata/azure.yaml @@ -15,6 +15,7 @@ scrape_configs: client_secret: "secret" proxy_url: "proxy" follow_redirects: false + authentication_method: "ManagedIdentity" remote_write: - name: "remote1" diff --git a/internal/converter/internal/prometheusconvert/testdata/discovery.alloy b/internal/converter/internal/prometheusconvert/testdata/discovery.alloy index 726eca7771..f3d2b4e7a1 100644 --- a/internal/converter/internal/prometheusconvert/testdata/discovery.alloy +++ b/internal/converter/internal/prometheusconvert/testdata/discovery.alloy @@ -6,10 +6,6 @@ discovery.azure "prometheus1" { tenant_id = "tenant1" client_secret = "secret1" } - - managed_identity { - client_id = "client1" - } } discovery.azure "prometheus1_2" { @@ -20,10 +16,6 @@ discovery.azure "prometheus1_2" { tenant_id = "tenant2" client_secret = "secret2" } - - managed_identity { - client_id = "client2" - } } discovery.relabel "prometheus1" { diff --git a/internal/converter/internal/prometheusconvert/testdata/discovery_relabel.alloy b/internal/converter/internal/prometheusconvert/testdata/discovery_relabel.alloy index 380c09804a..322caeafbc 100644 --- a/internal/converter/internal/prometheusconvert/testdata/discovery_relabel.alloy +++ b/internal/converter/internal/prometheusconvert/testdata/discovery_relabel.alloy @@ -6,10 +6,6 @@ discovery.azure "prometheus2" { tenant_id = "tenant" client_secret = "secret" } - - managed_identity { - client_id = "client" - } } discovery.relabel "prometheus1" { diff --git a/internal/converter/internal/promtailconvert/testdata/azure.alloy b/internal/converter/internal/promtailconvert/testdata/azure.alloy index 3a26563523..789f1738df 100644 --- a/internal/converter/internal/promtailconvert/testdata/azure.alloy +++ b/internal/converter/internal/promtailconvert/testdata/azure.alloy @@ -6,10 +6,6 @@ discovery.azure "fun" { tenant_id = "tenant" client_secret = "secret" } - - managed_identity { - client_id = "client" - } } local.file_match "fun" { diff --git a/internal/converter/internal/staticconvert/testdata/prom_scrape.alloy b/internal/converter/internal/staticconvert/testdata/prom_scrape.alloy index 625ef1b76a..9aedd09025 100644 --- a/internal/converter/internal/staticconvert/testdata/prom_scrape.alloy +++ b/internal/converter/internal/staticconvert/testdata/prom_scrape.alloy @@ -6,10 +6,6 @@ discovery.azure "metrics_agent_promobee" { tenant_id = "tenant" client_secret = "secret" } - - managed_identity { - client_id = "client" - } proxy_url = "proxy" } @@ -21,10 +17,6 @@ discovery.azure "metrics_agent_promobee_2" { tenant_id = "tenant" client_secret = "secret" } - - managed_identity { - client_id = "client" - } proxy_url = "proxy" } diff --git a/internal/converter/internal/staticconvert/testdata/traces.alloy b/internal/converter/internal/staticconvert/testdata/traces.alloy index 5bfde72dad..bc73822878 100644 --- a/internal/converter/internal/staticconvert/testdata/traces.alloy +++ b/internal/converter/internal/staticconvert/testdata/traces.alloy @@ -52,10 +52,6 @@ discovery.azure "_0_default_prometheus1" { tenant_id = "tenant1" client_secret = "secret1" } - - managed_identity { - client_id = "client1" - } } discovery.lightsail "_0_default_prometheus1" { @@ -118,8 +114,7 @@ prometheus.relabel "_0_default" { } otelcol.exporter.prometheus "_0_default" { - gc_frequency = "0s" - forward_to = [prometheus.relabel._0_default.receiver] + forward_to = [prometheus.relabel._0_default.receiver] } otelcol.exporter.loadbalancing "_0_default" { @@ -204,8 +199,7 @@ prometheus.relabel "_1_default" { } otelcol.exporter.prometheus "_1_default" { - gc_frequency = "0s" - forward_to = [prometheus.relabel._1_default.receiver] + forward_to = [prometheus.relabel._1_default.receiver] } otelcol.exporter.otlp "_1_0" { From 1e769d527fbbaa4ac91ecc1a1a217c17e4a94705 Mon Sep 17 00:00:00 2001 From: William Dumont Date: Thu, 12 Dec 2024 14:32:44 +0100 Subject: [PATCH 4/5] changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec8ba4d78d..09c4eda037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,8 @@ Main (unreleased) - Fixed an issue where the `otelcol.processor.interval` could not be used because the debug metrics were not set to default. (@wildum) +- Fix conversion of static config to Alloy for `discovery.azure` and `otelcol.exporter.prometheus`. (@wildum) + ### Other changes - Change the stability of the `livedebugging` feature from "experimental" to "generally available". (@wildum) From fb283efc94dd87f2f23d9ac1e8d83d5eedd2399d Mon Sep 17 00:00:00 2001 From: William Dumont Date: Fri, 13 Dec 2024 12:41:12 +0100 Subject: [PATCH 5/5] fix default yaml values for otelcol prometheus exporter converter --- .../internal/build/converter_remotewriteexporter.go | 11 ++--------- .../internal/staticconvert/testdata/traces.alloy | 6 ++++-- internal/static/traces/remotewriteexporter/factory.go | 5 ++++- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go b/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go index 0623cac283..23aa682ed3 100644 --- a/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go +++ b/internal/converter/internal/staticconvert/internal/build/converter_remotewriteexporter.go @@ -91,20 +91,13 @@ func toremotewriteexporterConfig(cfg *remotewriteexporter.Config, forwardTo []st defaultArgs := &prometheus.Arguments{} defaultArgs.SetToDefault() - args := &prometheus.Arguments{ + return &prometheus.Arguments{ IncludeTargetInfo: defaultArgs.IncludeTargetInfo, IncludeScopeInfo: defaultArgs.IncludeScopeInfo, IncludeScopeLabels: defaultArgs.IncludeScopeLabels, - GCFrequency: defaultArgs.GCFrequency, + GCFrequency: cfg.StaleTime, ForwardTo: forwardTo, AddMetricSuffixes: defaultArgs.AddMetricSuffixes, ResourceToTelemetryConversion: defaultArgs.ResourceToTelemetryConversion, } - - // Override default only if > 0 because GCFrequency of 0 is not allowed - if cfg.StaleTime > 0 { - args.GCFrequency = cfg.StaleTime - } - - return args } diff --git a/internal/converter/internal/staticconvert/testdata/traces.alloy b/internal/converter/internal/staticconvert/testdata/traces.alloy index bc73822878..b95bc9158d 100644 --- a/internal/converter/internal/staticconvert/testdata/traces.alloy +++ b/internal/converter/internal/staticconvert/testdata/traces.alloy @@ -114,7 +114,8 @@ prometheus.relabel "_0_default" { } otelcol.exporter.prometheus "_0_default" { - forward_to = [prometheus.relabel._0_default.receiver] + gc_frequency = "15m0s" + forward_to = [prometheus.relabel._0_default.receiver] } otelcol.exporter.loadbalancing "_0_default" { @@ -199,7 +200,8 @@ prometheus.relabel "_1_default" { } otelcol.exporter.prometheus "_1_default" { - forward_to = [prometheus.relabel._1_default.receiver] + gc_frequency = "15m0s" + forward_to = [prometheus.relabel._1_default.receiver] } otelcol.exporter.otlp "_1_0" { diff --git a/internal/static/traces/remotewriteexporter/factory.go b/internal/static/traces/remotewriteexporter/factory.go index 03935f960c..8af2a3393a 100644 --- a/internal/static/traces/remotewriteexporter/factory.go +++ b/internal/static/traces/remotewriteexporter/factory.go @@ -37,7 +37,10 @@ func NewFactory() exporter.Factory { } func createDefaultConfig() component.Config { - return &Config{} + return &Config{ + StaleTime: 15 * time.Minute, + LoopInterval: time.Second, + } } func createMetricsExporter(