[service] ensure endpoint is prefixed w/ scheme (#12258)

#### Description Users can enter the OTLP endpoint w/o a scheme in the prefix. This causes issues with the URL parsing code in the config package. Fixes #12254 --------- Signed-off-by: Alex Boten <[email protected]>
open-telemetry · Feb 3, 2025 · e69ca35 · e69ca35
1 parent a6aaf37
commit e69ca35
Show file tree

Hide file tree

Showing 10 changed files with 129 additions and 7 deletions.
diff --git a/.chloggen/codeboten_fix-12254.yaml b/.chloggen/codeboten_fix-12254.yaml
@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: service
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Preserve URL normalization logic that was present before.
+
+# One or more tracking issues or pull requests related to the change
+issues: [12254]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []
diff --git a/service/telemetry/internal/migration/normalize.go b/service/telemetry/internal/migration/normalize.go
@@ -0,0 +1,13 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package migration // import "go.opentelemetry.io/collector/service/telemetry/internal/migration"
+
+import "strings"
+
+func normalizeEndpoint(endpoint string) *string {
+	if !strings.HasPrefix(endpoint, "https://") && !strings.HasPrefix(endpoint, "http://") {
+		endpoint = "http://" + endpoint
+	}
+	return &endpoint
+}
diff --git a/service/telemetry/internal/migration/testdata/v0.2.0_logs.yaml b/service/telemetry/internal/migration/testdata/v0.2.0_logs.yaml
@@ -10,3 +10,10 @@ processors:
   - simple:
       exporter:
         console: {}
+  - simple:
+      exporter:
+        otlp:
+          protocol: http/protobuf
+          endpoint: http://127.0.0.1:4317
+          headers:
+            "key1": "value1"
diff --git a/service/telemetry/internal/migration/testdata/v0.2.0_traces.yaml b/service/telemetry/internal/migration/testdata/v0.2.0_traces.yaml
@@ -10,3 +10,10 @@ processors:
   - simple:
       exporter:
         console: {}
+  - simple:
+      exporter:
+        otlp:
+          protocol: http/protobuf
+          endpoint: http://127.0.0.1:4317
+          headers:
+            "key1": "value1"
diff --git a/service/telemetry/internal/migration/testdata/v0.3.0_logs.yaml b/service/telemetry/internal/migration/testdata/v0.3.0_logs.yaml
@@ -11,3 +11,11 @@ processors:
   - simple:
       exporter:
         console: {}
+  - simple:
+      exporter:
+        otlp:
+          protocol: http/protobuf
+          endpoint: http://127.0.0.1:4317
+          headers:
+          - name: "key1"
+            value: "value1"
diff --git a/service/telemetry/internal/migration/testdata/v0.3.0_traces.yaml b/service/telemetry/internal/migration/testdata/v0.3.0_traces.yaml
@@ -11,3 +11,11 @@ processors:
   - simple:
       exporter:
         console: {}
+  - simple:
+      exporter:
+        otlp:
+          protocol: http/protobuf
+          endpoint: http://127.0.0.1:4317
+          headers:
+          - name: "key1"
+            value: "value1"
diff --git a/service/telemetry/internal/migration/v0.2.0.go b/service/telemetry/internal/migration/v0.2.0.go
@@ -57,7 +57,7 @@ func otlpV02ToV03(in *configv02.OTLP) *config.OTLP {
 		ClientCertificate: in.ClientCertificate,
 		ClientKey:         in.ClientKey,
 		Compression:       in.Compression,
-		Endpoint:          &in.Endpoint,
+		Endpoint:          normalizeEndpoint(in.Endpoint),
 		Insecure:          in.Insecure,
 		Protocol:          &in.Protocol,
 		Timeout:           in.Timeout,
@@ -74,7 +74,7 @@ func otlpMetricV02ToV03(in *configv02.OTLPMetric) *config.OTLPMetric {
 		ClientCertificate: in.ClientCertificate,
 		ClientKey:         in.ClientKey,
 		Compression:       in.Compression,
-		Endpoint:          &in.Endpoint,
+		Endpoint:          normalizeEndpoint(in.Endpoint),
 		Insecure:          in.Insecure,
 		Protocol:          &in.Protocol,
 		Timeout:           in.Timeout,

diff --git a/service/telemetry/internal/migration/v0.2.0_test.go b/service/telemetry/internal/migration/v0.2.0_test.go
@@ -18,7 +18,11 @@ func TestUnmarshalLogsConfigV020(t *testing.T) {
 
 	cfg := LogsConfigV030{}
 	require.NoError(t, cm.Unmarshal(&cfg))
-	require.Len(t, cfg.Processors, 2)
+	require.Len(t, cfg.Processors, 3)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[0].Batch.Exporter.OTLP.Endpoint)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[2].Simple.Exporter.OTLP.Endpoint)
 }
 
 func TestUnmarshalTracesConfigV020(t *testing.T) {
@@ -27,7 +31,11 @@ func TestUnmarshalTracesConfigV020(t *testing.T) {
 
 	cfg := TracesConfigV030{}
 	require.NoError(t, cm.Unmarshal(&cfg))
-	require.Len(t, cfg.Processors, 2)
+	require.Len(t, cfg.Processors, 3)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[0].Batch.Exporter.OTLP.Endpoint)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[2].Simple.Exporter.OTLP.Endpoint)
 }
 
 func TestUnmarshalMetricsConfigV020(t *testing.T) {
@@ -37,4 +45,6 @@ func TestUnmarshalMetricsConfigV020(t *testing.T) {
 	cfg := MetricsConfigV030{}
 	require.NoError(t, cm.Unmarshal(&cfg))
 	require.Len(t, cfg.Readers, 2)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Readers[0].Periodic.Exporter.OTLP.Endpoint)
 }
diff --git a/service/telemetry/internal/migration/v0.3.0.go b/service/telemetry/internal/migration/v0.3.0.go
@@ -39,6 +39,19 @@ func (c *TracesConfigV030) Unmarshal(conf *confmap.Conf) error {
 		// TODO: add a warning log to tell users to migrate their config
 		return tracesConfigV02ToV03(v2TracesConfig, c)
 	}
+	// ensure endpoint normalization occurs
+	for _, r := range c.Processors {
+		if r.Batch != nil {
+			if r.Batch.Exporter.OTLP != nil {
+				r.Batch.Exporter.OTLP.Endpoint = normalizeEndpoint(*r.Batch.Exporter.OTLP.Endpoint)
+			}
+		}
+		if r.Simple != nil {
+			if r.Simple.Exporter.OTLP != nil && r.Simple.Exporter.OTLP.Endpoint != nil {
+				r.Simple.Exporter.OTLP.Endpoint = normalizeEndpoint(*r.Simple.Exporter.OTLP.Endpoint)
+			}
+		}
+	}
 	return nil
 }
 
@@ -70,6 +83,14 @@ func (c *MetricsConfigV030) Unmarshal(conf *confmap.Conf) error {
 		// TODO: add a warning log to tell users to migrate their config
 		return metricsConfigV02ToV03(v02, c)
 	}
+	// ensure endpoint normalization occurs
+	for _, r := range c.Readers {
+		if r.Periodic != nil {
+			if r.Periodic.Exporter.OTLP != nil && r.Periodic.Exporter.OTLP.Endpoint != nil {
+				r.Periodic.Exporter.OTLP.Endpoint = normalizeEndpoint(*r.Periodic.Exporter.OTLP.Endpoint)
+			}
+		}
+	}
 	return nil
 }
 
@@ -169,6 +190,18 @@ func (c *LogsConfigV030) Unmarshal(conf *confmap.Conf) error {
 		// TODO: add a warning log to tell users to migrate their config
 		return logsConfigV02ToV03(v02, c)
 	}
-	//
+	// ensure endpoint normalization occurs
+	for _, r := range c.Processors {
+		if r.Batch != nil {
+			if r.Batch.Exporter.OTLP != nil {
+				r.Batch.Exporter.OTLP.Endpoint = normalizeEndpoint(*r.Batch.Exporter.OTLP.Endpoint)
+			}
+		}
+		if r.Simple != nil {
+			if r.Simple.Exporter.OTLP != nil && r.Simple.Exporter.OTLP.Endpoint != nil {
+				r.Simple.Exporter.OTLP.Endpoint = normalizeEndpoint(*r.Simple.Exporter.OTLP.Endpoint)
+			}
+		}
+	}
 	return nil
 }
diff --git a/service/telemetry/internal/migration/v0.3.0_test.go b/service/telemetry/internal/migration/v0.3.0_test.go
@@ -18,7 +18,11 @@ func TestUnmarshalLogsConfigV030(t *testing.T) {
 
 	cfg := LogsConfigV030{}
 	require.NoError(t, cm.Unmarshal(&cfg))
-	require.Len(t, cfg.Processors, 2)
+	require.Len(t, cfg.Processors, 3)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[0].Batch.Exporter.OTLP.Endpoint)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[2].Simple.Exporter.OTLP.Endpoint)
 }
 
 func TestUnmarshalTracesConfigV030(t *testing.T) {
@@ -27,7 +31,11 @@ func TestUnmarshalTracesConfigV030(t *testing.T) {
 
 	cfg := TracesConfigV030{}
 	require.NoError(t, cm.Unmarshal(&cfg))
-	require.Len(t, cfg.Processors, 2)
+	require.Len(t, cfg.Processors, 3)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[0].Batch.Exporter.OTLP.Endpoint)
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Processors[2].Simple.Exporter.OTLP.Endpoint)
 }
 
 func TestUnmarshalMetricsConfigV030(t *testing.T) {
@@ -37,4 +45,7 @@ func TestUnmarshalMetricsConfigV030(t *testing.T) {
 	cfg := MetricsConfigV030{}
 	require.NoError(t, cm.Unmarshal(&cfg))
 	require.Len(t, cfg.Readers, 2)
+
+	// check the endpoint is prefixed w/ http
+	require.Equal(t, "http://127.0.0.1:4317", *cfg.Readers[0].Periodic.Exporter.OTLP.Endpoint)
 }