otelcol.processor.discovery should not modify its targets attribute (#5170)

Fixed a bug where `otelcol.processor.discovery` could modify the `targets` passed by an upstream component.

---------

Co-authored-by: Paschalis Tsilias <[email protected]>

Author: ptodev

CHANGELOG.md

@@ -73,6 +73,10 @@ Main (unreleased)
   `msg`, followed by non-common fields. Previously, the position of `msg` was
   not consistent. (@rfratto)
 
+### Bugfixes
+
+- Fixed a bug where `otelcol.processor.discovery` could modify the `targets` passed by an upstream component. (@ptodev)
+
 v0.36.1 (2023-09-06)
 --------------------
 

component/otelcol/processor/discovery/discovery.go

@@ -77,7 +77,6 @@ func (args *Arguments) Validate() error {
 
 // Component is the otelcol.exporter.discovery component.
 type Component struct {
-	cfg      Arguments
 	consumer *promsdconsumer.Consumer
 	logger   log.Logger
 }
@@ -134,18 +133,17 @@ func (c *Component) Run(ctx context.Context) error {
 // Update implements Component.
 func (c *Component) Update(newConfig component.Arguments) error {
 	cfg := newConfig.(Arguments)
-	c.cfg = cfg
 
 	hostLabels := make(map[string]discovery.Target)
 
-	for _, labels := range c.cfg.Targets {
+	for _, labels := range cfg.Targets {
 		host, err := promsdconsumer.GetHostFromLabels(labels)
 		if err != nil {
 			level.Warn(c.logger).Log("msg", "ignoring target, unable to find address", "err", err)
 			continue
 		}
-		promsdconsumer.CleanupLabels(labels)
-		hostLabels[host] = labels
+
+		hostLabels[host] = promsdconsumer.NewTargetsWithNonInternalLabels(labels)
 	}
 
 	err := c.consumer.UpdateOptions(promsdconsumer.Options{

docs/sources/flow/reference/components/otelcol.processor.discovery.md

@@ -104,7 +104,7 @@ information.
 ## Examples
 
 ### Basic usage
-```
+```river
 discovery.http "dynamic_targets" {
     url              = "https://example.com/scrape_targets"
     refresh_interval = "15s"
@@ -123,7 +123,7 @@ otelcol.processor.discovery "default" {
 
 Outputs from more than one discovery process can be combined via the `concat` function.
 
-```
+```river
 discovery.http "dynamic_targets" {
     url              = "https://example.com/scrape_targets"
     refresh_interval = "15s"
@@ -149,7 +149,7 @@ It is not necessary to use a discovery component. In the example below, a `test_
 attribute will be added to a span if its IP address is "1.2.2.2". The `__internal_label__` will
 be not be added to the span, because it begins with a double underscore (`__`).
 
-```
+```river
 otelcol.processor.discovery "default" {
     targets = [{
         "__address__"        = "1.2.2.2", 

docs/sources/flow/reference/components/otelcol.processor.span.md

@@ -234,7 +234,7 @@ This example creates a new span name from the values of attributes `db.svc`,
 `operation`, and `id`, in that order, separated by the value `::`. 
 All attribute keys need to be specified in the span for the processor to rename it.
 
-```
+```river
 otelcol.processor.span "default" {
   name {
     separator        = "::"
@@ -247,20 +247,20 @@ otelcol.processor.span "default" {
 }
 ```
 
-For a span with the following attributes key/value pairs, the example
+For a span with the following attributes key/value pairs, the above
 Flow configuration will change the span name to `"location::get::1234"`:
-```
+```json
 { 
   "db.svc": "location", 
   "operation": "get", 
   "id": "1234"
 }
 ```
 
-For a span with the following attributes key/value pairs, the example 
+For a span with the following attributes key/value pairs, the above 
 Flow configuration will not change the span name. 
 This is because the attribute key `operation` isn't set:
-```
+```json
 { 
   "db.svc": "location", 
   "id": "1234"
@@ -269,7 +269,7 @@ This is because the attribute key `operation` isn't set:
 
 ### Creating a new span name from attribute values (no separator)
 
-```
+```river
 otelcol.processor.span "default" {
   name {
     from_attributes = ["db.svc", "operation", "id"]
@@ -281,9 +281,9 @@ otelcol.processor.span "default" {
 }
 ```
 
-For a span with the following attributes key/value pairs, the example
+For a span with the following attributes key/value pairs, the above
 Flow configuration will change the span name to `"locationget1234"`:
-```
+```json
 { 
   "db.svc": "location", 
   "operation": "get", 
@@ -298,7 +298,7 @@ Example input and output using the Flow configuration below:
 2. The span name will be changed to `/api/v1/document/{documentId}/update`
 3. A new attribute `"documentId"="12345678"` will be added to the span.
 
-```
+```river
 otelcol.processor.span "default" {
   name {
     to_attributes {
@@ -321,7 +321,7 @@ if the span has the following properties:
 - The span name contains `/` anywhere in the string.
 - The span name is not `donot/change`.
 
-```
+```river
 otelcol.processor.span "default" {
   include {
     match_type = "regexp"
@@ -348,7 +348,7 @@ otelcol.processor.span "default" {
 
 This example changes the status of a span to "Error" and sets an error description.
 
-```
+```river
 otelcol.processor.span "default" {
   status {
     code        = "Error"
@@ -366,7 +366,7 @@ otelcol.processor.span "default" {
 This example sets the status to success only when attribute `http.status_code` 
 is equal to `400`.
 
-```
+```river
 otelcol.processor.span "default" {
   include {
     match_type = "strict"

pkg/traces/promsdprocessor/consumer/consumer.go

@@ -247,10 +247,12 @@ func GetHostFromLabels(labels discovery.Target) (string, error) {
 	return host, nil
 }
 
-func CleanupLabels(labels discovery.Target) {
-	for k := range labels {
-		if strings.HasPrefix(k, "__") {
-			delete(labels, k)
+func NewTargetsWithNonInternalLabels(labels discovery.Target) discovery.Target {
+	res := make(discovery.Target)
+	for k, v := range labels {
+		if !strings.HasPrefix(k, "__") {
+			res[k] = v
 		}
 	}
+	return res
 }

pkg/traces/promsdprocessor/prom_sd_processor.go

@@ -172,9 +172,7 @@ func (p *promServiceDiscoProcessor) syncTargets(jobName string, group *targetgro
 			continue
 		}
 
-		promsdconsumer.CleanupLabels(labels)
-
 		level.Debug(p.logger).Log("msg", "adding host to hostLabels", "host", host)
-		hostLabels[host] = labels
+		hostLabels[host] = promsdconsumer.NewTargetsWithNonInternalLabels(labels)
 	}
 }