Override insecure when endpoint URL is set (#5944)

When an endpoint is set in both Environment variable with "http" and passed in WithEndpointURL with "https", Insecure is set to true while the endpoint is used from WithEndpointURL. Example - OTEL_EXPORTER_OTLP_ENDPOINT is set to "http://env.endpoint/prefix" - WithEndpointURL is passed "https://someendpoint/somepath" The real endpoint used is "http://someendpoint/somepath", which is actually neither of both. --------- Co-authored-by: Robert Pająk <pellared@hotmail.com>
open-telemetry · Nov 7, 2024 · b62a3fd · b62a3fd
1 parent 937813d
commit b62a3fd
Show file tree

Hide file tree

Showing 16 changed files with 199 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Global MeterProvider registration unwraps global instrument Observers, the undocumented Unwrap() methods are now private. (#5881)
 - Fix `go.opentelemetry.io/otel/exporters/prometheus` trying to add exemplars to Gauge metrics, which is unsupported. (#5912)
+- Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`. (#5944)
+- Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5944)
+- Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5944)
+- Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5944)
 
 ### Changed
 

diff --git a/exporters/otlp/otlplog/otlploggrpc/config_test.go b/exporters/otlp/otlplog/otlploggrpc/config_test.go
@@ -169,6 +169,21 @@ func TestNewConfig(t *testing.T) {
 				retryCfg: newSetting(defaultRetryCfg),
 			},
 		},
+		{
+			name: "WithEndpointURL secure when Environment Endpoint is set insecure",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT": "http://env.endpoint:8080/prefix",
+			},
+			options: []Option{
+				WithEndpointURL("https://test:8080/path"),
+			},
+			want: config{
+				endpoint: newSetting("test:8080"),
+				insecure: newSetting(false),
+				timeout:  newSetting(defaultTimeout),
+				retryCfg: newSetting(defaultRetryCfg),
+			},
+		},
 		{
 			name: "LogEnvironmentVariables",
 			envars: map[string]string{
@@ -235,6 +250,21 @@ func TestNewConfig(t *testing.T) {
 				retryCfg: newSetting(defaultRetryCfg),
 			},
 		},
+		{
+			name: "WithEndpointURL secure when Environment insecure is set false",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_INSECURE": "true",
+			},
+			options: []Option{
+				WithEndpointURL("https://test:8080/path"),
+			},
+			want: config{
+				endpoint: newSetting("test:8080"),
+				insecure: newSetting(false),
+				timeout:  newSetting(defaultTimeout),
+				retryCfg: newSetting(defaultRetryCfg),
+			},
+		},
 		{
 			name: "EnvironmentVariablesPrecedence",
 			envars: map[string]string{

diff --git a/exporters/otlp/otlplog/otlploghttp/config.go b/exporters/otlp/otlplog/otlploghttp/config.go
@@ -183,11 +183,7 @@ func WithEndpointURL(rawURL string) Option {
 	return fnOpt(func(c config) config {
 		c.endpoint = newSetting(u.Host)
 		c.path = newSetting(u.Path)
-		if u.Scheme != "https" {
-			c.insecure = newSetting(true)
-		} else {
-			c.insecure = newSetting(false)
-		}
+		c.insecure = newSetting(u.Scheme != "https")
 		return c
 	})
 }

diff --git a/exporters/otlp/otlplog/otlploghttp/config_test.go b/exporters/otlp/otlplog/otlploghttp/config_test.go
@@ -167,6 +167,38 @@ func TestNewConfig(t *testing.T) {
 				retryCfg: newSetting(defaultRetryCfg),
 			},
 		},
+		{
+			name: "WithEndpointURL secure when Environment Endpoint is set insecure",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT": "http://env.endpoint:8080/prefix",
+			},
+			options: []Option{
+				WithEndpointURL("https://test:8080/path"),
+			},
+			want: config{
+				endpoint: newSetting("test:8080"),
+				path:     newSetting("/path"),
+				insecure: newSetting(false),
+				timeout:  newSetting(defaultTimeout),
+				retryCfg: newSetting(defaultRetryCfg),
+			},
+		},
+		{
+			name: "WithEndpointURL secure when Environment insecure is set false",
+			envars: map[string]string{
+				"OTEL_EXPORTER_OTLP_LOGS_INSECURE": "true",
+			},
+			options: []Option{
+				WithEndpointURL("https://test:8080/path"),
+			},
+			want: config{
+				endpoint: newSetting("test:8080"),
+				path:     newSetting("/path"),
+				insecure: newSetting(false),
+				timeout:  newSetting(defaultTimeout),
+				retryCfg: newSetting(defaultRetryCfg),
+			},
+		},
 		{
 			name: "LogEnvironmentVariables",
 			envars: map[string]string{

diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go
@@ -287,9 +287,7 @@ func WithEndpointURL(v string) GenericOption {
 
 		cfg.Metrics.Endpoint = u.Host
 		cfg.Metrics.URLPath = u.Path
-		if u.Scheme != "https" {
-			cfg.Metrics.Insecure = true
-		}
+		cfg.Metrics.Insecure = u.Scheme != "https"
 
 		return cfg
 	})

diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go
@@ -149,6 +149,34 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
 			},
 		},
+		{
+			name: "Test With WithEndpointURL secure when Environment Endpoint is set insecure",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env.endpoint/prefix",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
+				assert.Equal(t, "/somepath", c.Metrics.URLPath)
+				assert.False(t, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test With WithEndpointURL secure when Environment insecure is set true",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_INSECURE": "true",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
+				assert.Equal(t, "/somepath", c.Metrics.URLPath)
+				assert.False(t, c.Metrics.Insecure)
+			},
+		},
 		{
 			name: "Test Environment Endpoint",
 			env: map[string]string{

diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go
@@ -287,9 +287,7 @@ func WithEndpointURL(v string) GenericOption {
 
 		cfg.Metrics.Endpoint = u.Host
 		cfg.Metrics.URLPath = u.Path
-		if u.Scheme != "https" {
-			cfg.Metrics.Insecure = true
-		}
+		cfg.Metrics.Insecure = u.Scheme != "https"
 
 		return cfg
 	})

diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go
@@ -149,6 +149,34 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
 			},
 		},
+		{
+			name: "Test With WithEndpointURL secure when Environment Endpoint is set insecure",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env.endpoint/prefix",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
+				assert.Equal(t, "/somepath", c.Metrics.URLPath)
+				assert.False(t, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test With WithEndpointURL secure when Environment insecure is set true",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_INSECURE": "true",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
+				assert.Equal(t, "/somepath", c.Metrics.URLPath)
+				assert.False(t, c.Metrics.Insecure)
+			},
+		},
 		{
 			name: "Test Environment Endpoint",
 			env: map[string]string{

diff --git a/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options.go b/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options.go
@@ -278,9 +278,7 @@ func WithEndpointURL(v string) GenericOption {
 
 		cfg.Traces.Endpoint = u.Host
 		cfg.Traces.URLPath = u.Path
-		if u.Scheme != "https" {
-			cfg.Traces.Insecure = true
-		}
+		cfg.Traces.Insecure = u.Scheme != "https"
 
 		return cfg
 	})

diff --git a/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options_test.go
@@ -147,6 +147,20 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "someendpoint", c.Traces.Endpoint)
 			},
 		},
+		{
+			name: "Test With WithEndpointURL secure when Environment Endpoint is set insecure",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env.endpoint/prefix",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Traces.Endpoint)
+				assert.Equal(t, "/somepath", c.Traces.URLPath)
+				assert.False(t, c.Traces.Insecure)
+			},
+		},
 		{
 			name: "Test Environment Endpoint",
 			env: map[string]string{

diff --git a/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options.go b/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options.go
@@ -278,9 +278,7 @@ func WithEndpointURL(v string) GenericOption {
 
 		cfg.Traces.Endpoint = u.Host
 		cfg.Traces.URLPath = u.Path
-		if u.Scheme != "https" {
-			cfg.Traces.Insecure = true
-		}
+		cfg.Traces.Insecure = u.Scheme != "https"
 
 		return cfg
 	})

diff --git a/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options_test.go
@@ -147,6 +147,20 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "someendpoint", c.Traces.Endpoint)
 			},
 		},
+		{
+			name: "Test With WithEndpointURL secure when Environment Endpoint is set insecure",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env.endpoint/prefix",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Traces.Endpoint)
+				assert.Equal(t, "/somepath", c.Traces.URLPath)
+				assert.False(t, c.Traces.Insecure)
+			},
+		},
 		{
 			name: "Test Environment Endpoint",
 			env: map[string]string{

diff --git a/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl
@@ -287,9 +287,7 @@ func WithEndpointURL(v string) GenericOption {
 
 		cfg.Metrics.Endpoint = u.Host
 		cfg.Metrics.URLPath = u.Path
-		if u.Scheme != "https" {
-			cfg.Metrics.Insecure = true
-		}
+		cfg.Metrics.Insecure = u.Scheme != "https"
 
 		return cfg
 	})

diff --git a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl
@@ -149,6 +149,34 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
 			},
 		},
+		{
+			name: "Test With WithEndpointURL secure when Environment Endpoint is set insecure",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env.endpoint/prefix",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
+				assert.Equal(t, "/somepath", c.Metrics.URLPath)
+				assert.False(t, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test With WithEndpointURL secure when Environment insecure is set true",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_INSECURE": "true",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Metrics.Endpoint)
+				assert.Equal(t, "/somepath", c.Metrics.URLPath)
+				assert.False(t, c.Metrics.Insecure)
+			},
+		},
 		{
 			name: "Test Environment Endpoint",
 			env: map[string]string{

diff --git a/internal/shared/otlp/otlptrace/otlpconfig/options.go.tmpl b/internal/shared/otlp/otlptrace/otlpconfig/options.go.tmpl
@@ -278,9 +278,7 @@ func WithEndpointURL(v string) GenericOption {
 
 		cfg.Traces.Endpoint = u.Host
 		cfg.Traces.URLPath = u.Path
-		if u.Scheme != "https" {
-			cfg.Traces.Insecure = true
-		}
+		cfg.Traces.Insecure = u.Scheme != "https"
 
 		return cfg
 	})

diff --git a/internal/shared/otlp/otlptrace/otlpconfig/options_test.go.tmpl b/internal/shared/otlp/otlptrace/otlpconfig/options_test.go.tmpl
@@ -147,6 +147,20 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "someendpoint", c.Traces.Endpoint)
 			},
 		},
+		{
+			name: "Test With WithEndpointURL secure when Environment Endpoint is set insecure",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env.endpoint/prefix",
+			},
+			opts: []GenericOption{
+				WithEndpointURL("https://someendpoint/somepath"),
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "someendpoint", c.Traces.Endpoint)
+				assert.Equal(t, "/somepath", c.Traces.URLPath)
+				assert.False(t, c.Traces.Insecure)
+			},
+		},
 		{
 			name: "Test Environment Endpoint",
 			env: map[string]string{