Cherry pick commits for the 35.1 patch release (#4581)

* Fix incorrect display of trace IDs in the automatic_logging processor (#4578) * Fix incorrect spanId and traceId string conversion * Fix upgrade guide (#4579)
grafana · Jul 25, 2023 · 168f618 · 168f618
1 parent 6753136
commit 168f618
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 84 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,15 @@ This document contains a historical list of changes between releases. Only
 changes that impact end-user behavior are listed; changes to documentation or
 internal API changes are not present.
 
+v0.35.1 (2023-07-25)
+--------------------
+
+### Bugfixes
+
+- Fix incorrect display of trace IDs in the automatic_logging processor of static mode's traces subsystem.
+  Users of the static mode's service graph processor are also advised to upgrade, 
+  although the bug should theoretically not affect them. (@ptodev)
+
 v0.35.0 (2023-07-18)
 --------------------
 

diff --git a/docs/sources/flow/upgrade-guide.md b/docs/sources/flow/upgrade-guide.md
@@ -19,50 +19,6 @@ Grafana Agent Flow.
 > [upgrade-guide-static]: {{< relref "../static/upgrade-guide.md" >}}
 > [upgrade-guide-operator]: {{< relref "../operator/upgrade-guide.md" >}}
 
-## Main (unreleased)
-
-### Breaking change: The algorithm for the "hash" action of `otelcol.processor.attributes` has changed
-The hash produced when using `action = "hash"` in the `otelcol.processor.attributes` flow component is now using the more secure SHA-256 algorithm.
-The change was made in PR [#22831](https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/22831) of opentelemetry-collector-contrib. 
-
-### Breaking change: `otelcol.exporter.loki` now includes instrumentation scope in its output
-
-Additional `instrumentation_scope` information will be added to the OTLP log signal, like this:
-```
-{
-    "body": "Example log",
-    "traceid": "01020304000000000000000000000000",
-    "spanid": "0506070800000000",
-    "severity": "error",
-    "attributes": {
-        "attr1": "1",
-        "attr2": "2"
-    },
-    "resources": {
-        "host.name": "something"
-    },
-    "instrumentation_scope": {
-        "name": "example-logger-name",
-        "version": "v1"
-    }
-}
-```
-
-### Breaking change: `otelcol.extension.jaeger_remote_sampling` removes the `/` HTTP endpoint
-
-The `/` HTTP endpoint was the same as the `/sampling` endpoint. The `/sampling` endpoint is still functional.
-The change was made in PR [#18070](https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/18070) of opentelemetry-collector-contrib. 
-
-### Breaking change: The `remote_sampling` block has been removed from `otelcol.receiver.jaeger`
-
-The `remote_sampling` block in `otelcol.receiver.jaeger` has been an undocumented no-op configuration for some time, and has now been removed. 
-Customers are advised to use `otelcol.extension.jaeger_remote_sampling` instead.
-
-### Deprecation: `otelcol.exporter.jaeger` has been deprecated and will be removed in Agent v0.38.0.
-
-This is because Jaeger supports OTLP directly and OpenTelemetry Collector is also removing its 
-[Jaeger receiver](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/jaegerexporter).
-
 ## v0.35
 
 ### Breaking change: `auth` and `version` attributes from `walk_params` block of `prometheus.exporter.snmp` have been removed
@@ -226,6 +182,48 @@ prometehus.scrape "example" {
 }
 ```
 
+### Breaking change: The algorithm for the "hash" action of `otelcol.processor.attributes` has changed
+The hash produced when using `action = "hash"` in the `otelcol.processor.attributes` flow component is now using the more secure SHA-256 algorithm.
+The change was made in PR [#22831](https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/22831) of opentelemetry-collector-contrib. 
+
+### Breaking change: `otelcol.exporter.loki` now includes instrumentation scope in its output
+
+Additional `instrumentation_scope` information will be added to the OTLP log signal, like this:
+```
+{
+    "body": "Example log",
+    "traceid": "01020304000000000000000000000000",
+    "spanid": "0506070800000000",
+    "severity": "error",
+    "attributes": {
+        "attr1": "1",
+        "attr2": "2"
+    },
+    "resources": {
+        "host.name": "something"
+    },
+    "instrumentation_scope": {
+        "name": "example-logger-name",
+        "version": "v1"
+    }
+}
+```
+
+### Breaking change: `otelcol.extension.jaeger_remote_sampling` removes the `/` HTTP endpoint
+
+The `/` HTTP endpoint was the same as the `/sampling` endpoint. The `/sampling` endpoint is still functional.
+The change was made in PR [#18070](https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/18070) of opentelemetry-collector-contrib. 
+
+### Breaking change: The `remote_sampling` block has been removed from `otelcol.receiver.jaeger`
+
+The `remote_sampling` block in `otelcol.receiver.jaeger` has been an undocumented no-op configuration for some time, and has now been removed. 
+Customers are advised to use `otelcol.extension.jaeger_remote_sampling` instead.
+
+### Deprecation: `otelcol.exporter.jaeger` has been deprecated and will be removed in Agent v0.38.0.
+
+This is because Jaeger supports OTLP directly and OpenTelemetry Collector is also removing its 
+[Jaeger receiver](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter/jaegerexporter).
+
 ## v0.34
 
 ### Breaking change: `phlare.scrape` and `phlare.write` have been renamed to `pyroscope.scrape` and `pyroscope.scrape`

diff --git a/docs/sources/static/upgrade-guide.md b/docs/sources/static/upgrade-guide.md
@@ -20,37 +20,6 @@ static mode.
 > [upgrade-guide-operator]: {{< relref "../operator/upgrade-guide.md" >}}
 > [upgrade-guide-flow]: {{< relref "../flow/upgrade-guide.md" >}}
 
-## Main (unreleased)
-
-### Breaking change: Removed and renamed tracing metrics
-
-In the traces subsystem for Static mode some metrics are removed and others are renamed.
-The reason for the removal is a bug which caused the metrics to be incorrect if more than one instance of a traces configuration is specified.
-
-Removed metrics:
-- "blackbox_exporter_config_last_reload_success_timestamp_seconds" (gauge)
-- "blackbox_exporter_config_last_reload_successful" (gauge)
-- "blackbox_module_unknown_total" (counter)
-- "traces_processor_tail_sampling_count_traces_sampled" (counter)
-- "traces_processor_tail_sampling_new_trace_id_received" (counter)
-- "traces_processor_tail_sampling_sampling_decision_latency" (histogram)
-- "traces_processor_tail_sampling_sampling_decision_timer_latency" (histogram)
-- "traces_processor_tail_sampling_sampling_policy_evaluation_error" (counter)
-- "traces_processor_tail_sampling_sampling_trace_dropped_too_early" (counter)
-- "traces_processor_tail_sampling_sampling_traces_on_memory" (gauge)
-- "traces_receiver_accepted_spans" (counter)
-- "traces_receiver_refused_spans" (counter)
-- "traces_exporter_enqueue_failed_log_records" (counter)
-- "traces_exporter_enqueue_failed_metric_points" (counter)
-- "traces_exporter_enqueue_failed_spans" (counter)
-- "traces_exporter_queue_capacity" (gauge)
-- "traces_exporter_queue_size" (gauge)
-
-Renamed metrics:
-- "traces_receiver_refused_spans" is renamed to "traces_receiver_refused_spans_total"
-- "traces_receiver_accepted_spans" is renamed to "traces_receiver_refused_spans_total"
-- "traces_exporter_sent_metric_points" is renamed to "traces_exporter_sent_metric_points_total"
-
 ## v0.35
 
 ### Breaking change: `auth` and `version` attributes from `walk_params` block of SNMP integration have been removed
@@ -97,6 +66,35 @@ See [Module and Auth Split Migration](https://github.com/prometheus/snmp_exporte
 The experimental feature Dynamic Configuration has been removed. The use case of dynamic configuration will be replaced
 with [Modules](../../concepts/modules/) in Grafana Agent Flow.
 
+### Breaking change: Removed and renamed tracing metrics
+
+In the traces subsystem for Static mode some metrics are removed and others are renamed.
+The reason for the removal is a bug which caused the metrics to be incorrect if more than one instance of a traces configuration is specified.
+
+Removed metrics:
+- "blackbox_exporter_config_last_reload_success_timestamp_seconds" (gauge)
+- "blackbox_exporter_config_last_reload_successful" (gauge)
+- "blackbox_module_unknown_total" (counter)
+- "traces_processor_tail_sampling_count_traces_sampled" (counter)
+- "traces_processor_tail_sampling_new_trace_id_received" (counter)
+- "traces_processor_tail_sampling_sampling_decision_latency" (histogram)
+- "traces_processor_tail_sampling_sampling_decision_timer_latency" (histogram)
+- "traces_processor_tail_sampling_sampling_policy_evaluation_error" (counter)
+- "traces_processor_tail_sampling_sampling_trace_dropped_too_early" (counter)
+- "traces_processor_tail_sampling_sampling_traces_on_memory" (gauge)
+- "traces_receiver_accepted_spans" (counter)
+- "traces_receiver_refused_spans" (counter)
+- "traces_exporter_enqueue_failed_log_records" (counter)
+- "traces_exporter_enqueue_failed_metric_points" (counter)
+- "traces_exporter_enqueue_failed_spans" (counter)
+- "traces_exporter_queue_capacity" (gauge)
+- "traces_exporter_queue_size" (gauge)
+
+Renamed metrics:
+- "traces_receiver_refused_spans" is renamed to "traces_receiver_refused_spans_total"
+- "traces_receiver_accepted_spans" is renamed to "traces_receiver_refused_spans_total"
+- "traces_exporter_sent_metric_points" is renamed to "traces_exporter_sent_metric_points_total"
+
 ## v0.33
 
 ### Symbolic links in Docker containers removed

diff --git a/pkg/integrations/v2/app_agent_receiver/payload.go b/pkg/integrations/v2/app_agent_receiver/payload.go
@@ -1,7 +1,6 @@
 package app_agent_receiver
 
 import (
-	"encoding/hex"
 	"fmt"
 	"sort"
 	"strconv"
@@ -154,11 +153,11 @@ func SpanToKeyVal(s ptrace.Span) *KeyVal {
 		KeyValAdd(kv, "end_timestamp", s.StartTimestamp().AsTime().String())
 	}
 	KeyValAdd(kv, "kind", "span")
-	KeyValAdd(kv, "traceID", hex.EncodeToString([]byte(s.TraceID().String())))
-	KeyValAdd(kv, "spanID", hex.EncodeToString([]byte(s.SpanID().String())))
+	KeyValAdd(kv, "traceID", s.TraceID().String())
+	KeyValAdd(kv, "spanID", s.SpanID().String())
 	KeyValAdd(kv, "span_kind", s.Kind().String())
 	KeyValAdd(kv, "name", s.Name())
-	KeyValAdd(kv, "parent_spanID", hex.EncodeToString([]byte(s.ParentSpanID().String())))
+	KeyValAdd(kv, "parent_spanID", s.ParentSpanID().String())
 	s.Attributes().Range(func(k string, v pcommon.Value) bool {
 		KeyValAdd(kv, "attr_"+k, fmt.Sprintf("%v", v))
 		return true

diff --git a/pkg/traces/automaticloggingprocessor/automaticloggingprocessor.go b/pkg/traces/automaticloggingprocessor/automaticloggingprocessor.go
@@ -2,7 +2,6 @@ package automaticloggingprocessor
 
 import (
 	"context"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"strconv"
@@ -124,7 +123,7 @@ func (p *automaticLoggingProcessor) ConsumeTraces(ctx context.Context, td ptrace
 			lastTraceID := ""
 			for k := 0; k < spanLen; k++ {
 				span := ss.Spans().At(k)
-				traceID := hex.EncodeToString([]byte(span.TraceID().String()))
+				traceID := span.TraceID().String()
 
 				if p.cfg.Spans {
 					keyValues := append(p.spanKeyVals(span), p.processKeyVals(rs.Resource(), svc)...)

diff --git a/pkg/traces/servicegraphprocessor/processor.go b/pkg/traces/servicegraphprocessor/processor.go
@@ -2,7 +2,6 @@ package servicegraphprocessor
 
 import (
 	"context"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"time"
@@ -316,7 +315,7 @@ func (p *processor) consume(trace ptrace.Traces) error {
 
 				switch span.Kind() {
 				case ptrace.SpanKindClient:
-					k := key(hex.EncodeToString([]byte(span.TraceID().String())), hex.EncodeToString([]byte(span.SpanID().String())))
+					k := key(span.TraceID().String(), span.SpanID().String())
 
 					edge, err := p.store.upsertEdge(k, func(e *edge) {
 						e.clientService = svc.Str()
@@ -339,7 +338,7 @@ func (p *processor) consume(trace ptrace.Traces) error {
 					}
 
 				case ptrace.SpanKindServer:
-					k := key(hex.EncodeToString([]byte(span.TraceID().String())), hex.EncodeToString([]byte(span.ParentSpanID().String())))
+					k := key(span.TraceID().String(), span.ParentSpanID().String())
 
 					edge, err := p.store.upsertEdge(k, func(e *edge) {
 						e.serverService = svc.Str()