Generate spanmetrics after load balancing. (#5889)

Co-authored-by: Paschalis Tsilias <[email protected]>
grafana · Dec 8, 2023 · d7fbffa · d7fbffa
1 parent 7424b92
commit d7fbffa
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,10 @@ Main (unreleased)
 
 - Update `pyroscope.ebpf` to produce more optimal pprof profiles for python processes https://github.com/grafana/pyroscope/pull/2788 (@korniltsev)
 
+- In Static mode's `traces` subsystem, `spanmetrics` used to be generated prior to load balancing.
+  This could lead to inaccurate metrics. This issue only affects Agents using both `spanmetrics` and 
+  `load_balancing`, when running in a load balanced cluster with more than one Agent instance. (@ptodev)
+
 - Fixes `loki.source.docker` a behavior that synced an incomplete list of targets to the tailer manager. (@FerdinandvHagen)
 
 v0.38.1 (2023-11-30)

diff --git a/pkg/traces/config.go b/pkg/traces/config.go
@@ -581,7 +581,7 @@ func (c *InstanceConfig) extensions() (map[string]interface{}, error) {
 
 func resolver(config map[string]interface{}) (map[string]interface{}, error) {
 	if len(config) == 0 {
-		return nil, fmt.Errorf("must configure one resolver (dns or static)")
+		return nil, fmt.Errorf("must configure one resolver (dns, static, or kubernetes)")
 	}
 	resolverCfg := make(map[string]interface{})
 	for typ, cfg := range config {
@@ -950,7 +950,9 @@ func tracingFactories() (otelcol.Factories, error) {
 // sets: before and after load balancing
 func orderProcessors(processors []string, splitPipelines bool) [][]string {
 	order := map[string]int{
-		"attributes":        0,
+		"attributes": 0,
+		// Spanmetrics should be before tail_sampling so that
+		// metrics are generated using as many spans as possible.
 		"spanmetrics":       1,
 		"service_graphs":    2,
 		"tail_sampling":     3,
@@ -978,6 +980,7 @@ func orderProcessors(processors []string, splitPipelines bool) [][]string {
 		if processor == "batch" ||
 			processor == "tail_sampling" ||
 			processor == "automatic_logging" ||
+			processor == "spanmetrics" ||
 			processor == "service_graphs" {
 
 			foundAt = i

diff --git a/pkg/traces/config_test.go b/pkg/traces/config_test.go
@@ -1664,9 +1664,9 @@ service_graphs:
 			expectedProcessors: map[component.ID][]component.ID{
 				component.NewIDWithName("traces", "0"): {
 					component.NewID("attributes"),
-					component.NewID("spanmetrics"),
 				},
 				component.NewIDWithName("traces", "1"): {
+					component.NewID("spanmetrics"),
 					component.NewID("service_graphs"),
 					component.NewID("tail_sampling"),
 					component.NewID("automatic_logging"),
@@ -1715,9 +1715,9 @@ load_balancing:
 			expectedProcessors: map[component.ID][]component.ID{
 				component.NewIDWithName("traces", "0"): {
 					component.NewID("attributes"),
-					component.NewID("spanmetrics"),
 				},
 				component.NewIDWithName("traces", "1"): {
+					component.NewID("spanmetrics"),
 					component.NewID("automatic_logging"),
 					component.NewID("batch"),
 				},
@@ -1819,9 +1819,9 @@ func TestOrderProcessors(t *testing.T) {
 			expected: [][]string{
 				{
 					"attributes",
-					"spanmetrics",
 				},
 				{
+					"spanmetrics",
 					"tail_sampling",
 					"automatic_logging",
 					"batch",
@@ -1853,9 +1853,10 @@ func TestOrderProcessors(t *testing.T) {
 			expected: [][]string{
 				{
 					"attributes",
+				},
+				{
 					"spanmetrics",
 				},
-				{},
 			},
 		},
 	}

diff --git a/pkg/traces/instance.go b/pkg/traces/instance.go
@@ -112,8 +112,13 @@ func (i *Instance) buildAndStartPipeline(ctx context.Context, cfg InstanceConfig
 	}
 
 	if cfg.LoadBalancing == nil && (cfg.TailSampling != nil || cfg.ServiceGraphs != nil) {
-		i.logger.Warn("Configuring tail_sampling and/or service_graphs without load_balance." +
-			"Load balancing is required for those features to properly work in multi agent deployments")
+		i.logger.Warn("Configuring tail_sampling and/or service_graphs without load_balancing." +
+			"Load balancing via trace ID is required for those features to work properly in multi agent deployments")
+	}
+
+	if cfg.LoadBalancing == nil && cfg.SpanMetrics != nil {
+		i.logger.Warn("Configuring spanmetrics without load_balancing." +
+			"Load balancing via service name is required for spanmetrics to work properly in multi agent deployments")
 	}
 
 	if cfg.AutomaticLogging != nil && cfg.AutomaticLogging.Backend != automaticloggingprocessor.BackendStdout {