Add a validation to make sure we don't have component collisions afte…

…r conversion (#4809)

Signed-off-by: erikbaranowski <[email protected]>

converter/internal/common/validate.go

@@ -3,8 +3,10 @@ package common
 import (
 	"fmt"
 	"reflect"
+	"strings"
 
 	"github.com/grafana/agent/converter/diag"
+	"github.com/grafana/agent/pkg/river/token/builder"
 )
 
 func UnsupportedNotDeepEquals(a any, b any, name string) diag.Diagnostics {
@@ -24,3 +26,28 @@ func UnsupportedNotEquals(a any, b any, name string) diag.Diagnostics {
 
 	return diags
 }
+
+// ValidateNodes will look at the final nodes and ensure that there are no
+// duplicate labels.
+func ValidateNodes(f *builder.File) diag.Diagnostics {
+	var diags diag.Diagnostics
+
+	nodes := f.Body().Nodes()
+	labels := make(map[string]string, len(nodes))
+	for _, node := range nodes {
+		switch n := node.(type) {
+		case *builder.Block:
+			label := strings.Join(n.Name, ".")
+			if n.Label != "" {
+				label += "." + n.Label
+			}
+			if _, ok := labels[label]; ok {
+				diags.Add(diag.SeverityLevelCritical, fmt.Sprintf("duplicate label after conversion %q. this is due to how valid flow labels are assembled and can be avoided by updating named properties in the source config.", label))
+			} else {
+				labels[label] = label
+			}
+		}
+	}
+
+	return diags
+}

converter/internal/prometheusconvert/prometheusconvert.go

@@ -38,6 +38,7 @@ func Convert(in []byte) ([]byte, diag.Diagnostics) {
 
 	f := builder.NewFile()
 	diags = AppendAll(f, promConfig)
+	diags.AddAll(common.ValidateNodes(f))
 
 	var buf bytes.Buffer
 	if _, err := f.WriteTo(&buf); err != nil {

converter/internal/promtailconvert/promtailconvert.go

@@ -62,6 +62,7 @@ func Convert(in []byte) ([]byte, diag.Diagnostics) {
 
 	f := builder.NewFile()
 	diags = AppendAll(f, &cfg.Config, "", diags)
+	diags.AddAll(common.ValidateNodes(f))
 
 	var buf bytes.Buffer
 	if _, err := f.WriteTo(&buf); err != nil {

converter/internal/staticconvert/staticconvert.go

@@ -39,6 +39,7 @@ func Convert(in []byte) ([]byte, diag.Diagnostics) {
 
 	f := builder.NewFile()
 	diags = AppendAll(f, staticConfig)
+	diags.AddAll(common.ValidateNodes(f))
 
 	var buf bytes.Buffer
 	if _, err := f.WriteTo(&buf); err != nil {
@@ -93,7 +94,8 @@ func appendStaticPrometheus(f *builder.File, staticConfig *config.Config) diag.D
 			return name
 		}
 
-		// There is an edge case unhandled here with label collisions.
+		// There is an edge case here with label collisions that will be caught
+		// by a validation [common.ValidateNodes].
 		// For example,
 		//   metrics config name = "agent_test"
 		//   scrape config job_name = "prometheus"
@@ -133,7 +135,8 @@ func appendStaticPromtail(f *builder.File, staticConfig *config.Config) diag.Dia
 			promtailConfig.LimitsConfig = promtailconvert.DefaultLimitsConfig()
 		}
 
-		// There is an edge case unhandled here with label collisions.
+		// There is an edge case here with label collisions that will be caught
+		// by a validation [common.ValidateNodes].
 		// For example,
 		//   logs config name = "agent_test"
 		//   scrape config job_name = "promtail"

converter/internal/staticconvert/testdata/dup_labels.diags

@@ -0,0 +1,3 @@
+(Warning) Please review your agent command line flags and ensure they are set in your Flow mode config file where necessary.
+(Critical) duplicate label after conversion "discovery.consul.metrics_name_job_test". this is due to how valid flow labels are assembled and can be avoided by updating named properties in the source config.
+(Critical) duplicate label after conversion "prometheus.scrape.metrics_name_job_test". this is due to how valid flow labels are assembled and can be avoided by updating named properties in the source config.

converter/internal/staticconvert/testdata/dup_labels.yaml

@@ -0,0 +1,23 @@
+metrics:
+  global:
+    remote_write:
+      - url: http://localhost:9009/api/prom/push
+  configs:
+    - name: name_job
+      host_filter: false
+      scrape_configs:
+        - job_name: 'test'
+          static_configs:
+            - targets: ['localhost:9098','localhost:9100']
+          consul_sd_configs:
+          - server: 'localhost:8500'
+            services: ['myapp1']
+    - name: name
+      host_filter: false
+      scrape_configs:
+        - job_name: 'job_test'
+          static_configs:
+            - targets: ['localhost:9099','localhost:9101']
+          consul_sd_configs:
+          - server: 'localhost:8501'
+            services: ['myapp2']

pkg/river/token/builder/builder.go

@@ -92,6 +92,10 @@ func (b *Body) SetValueOverrideHook(valueOverrideHook ValueOverrideHook) {
 	b.valueOverrideHook = valueOverrideHook
 }
 
+func (b *Body) Nodes() []tokenNode {
+	return b.nodes
+}
+
 // A tokenNode is a structural element which can be converted into a set of
 // Tokens.
 type tokenNode interface {