Add support for scheme in OTEL_EXPORTER_OTLP_ENDPOINT (#1886)

* Add support for scheme in OTEL_EXPORTER_OTLP_ENDPOINT according to the spec * Changes after review from pellared * Changes after review from pellared - 2 * Changes after review from pellared - 3 Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
open-telemetry · May 12, 2021 · 21c1641 · 21c1641
1 parent 5cb6263
commit 21c1641
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `Status` type was added to the `go.opentelemetry.io/otel/sdk/trace` package to represent the status of a span. (#1874)
 - The `SpanStub` type and its associated functions were added to the `go.opentelemetry.io/otel/sdk/trace/tracetest` package.
   This type can be used as a testing replacement for the `SpanSnapshot` that was removed from the `go.opentelemetry.io/otel/sdk/trace` package. (#1873)
+- Adds support for scheme in `OTEL_EXPORTER_OTLP_ENDPOINT` according to the spec. (#1886)
 
 ### Changed
 

diff --git a/exporters/otlp/internal/otlpconfig/envconfig.go b/exporters/otlp/internal/otlpconfig/envconfig.go
@@ -20,6 +20,7 @@ import (
 	"io/ioutil"
 	"net/url"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -29,6 +30,8 @@ import (
 	"go.opentelemetry.io/otel"
 )
 
+var httpSchemeRegexp = regexp.MustCompile(`(?i)^http://|https://`)
+
 func ApplyGRPCEnvConfigs(cfg *Config) {
 	e := EnvOptionsReader{
 		GetEnv:   os.Getenv,
@@ -71,13 +74,31 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption {
 
 	// Endpoint
 	if v, ok := e.getEnvValue("ENDPOINT"); ok {
-		opts = append(opts, WithEndpoint(v))
+		if isInsecureEndpoint(v) {
+			opts = append(opts, WithInsecure())
+		} else {
+			opts = append(opts, WithSecure())
+		}
+
+		opts = append(opts, WithEndpoint(trimSchema(v)))
 	}
 	if v, ok := e.getEnvValue("TRACES_ENDPOINT"); ok {
-		opts = append(opts, WithTracesEndpoint(v))
+		if isInsecureEndpoint(v) {
+			opts = append(opts, WithInsecureTraces())
+		} else {
+			opts = append(opts, WithSecureTraces())
+		}
+
+		opts = append(opts, WithTracesEndpoint(trimSchema(v)))
 	}
 	if v, ok := e.getEnvValue("METRICS_ENDPOINT"); ok {
-		opts = append(opts, WithMetricsEndpoint(v))
+		if isInsecureEndpoint(v) {
+			opts = append(opts, WithInsecureMetrics())
+		} else {
+			opts = append(opts, WithSecureMetrics())
+		}
+
+		opts = append(opts, WithMetricsEndpoint(trimSchema(v)))
 	}
 
 	// Certificate File
@@ -145,6 +166,14 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption {
 	return opts
 }
 
+func isInsecureEndpoint(endpoint string) bool {
+	return strings.HasPrefix(strings.ToLower(endpoint), "http://")
+}
+
+func trimSchema(endpoint string) string {
+	return httpSchemeRegexp.ReplaceAllString(endpoint, "")
+}
+
 // getEnvValue gets an OTLP environment variable value of the specified key using the GetEnv function.
 // This function already prepends the OTLP prefix to all key lookup.
 func (e *EnvOptionsReader) getEnvValue(key string) (string, bool) {

diff --git a/exporters/otlp/internal/otlpconfig/options.go b/exporters/otlp/internal/otlpconfig/options.go
@@ -303,18 +303,37 @@ func WithInsecure() GenericOption {
 	})
 }
 
+func WithSecure() GenericOption {
+	return newGenericOption(func(cfg *Config) {
+		cfg.Traces.Insecure = false
+		cfg.Metrics.Insecure = false
+	})
+}
+
 func WithInsecureTraces() GenericOption {
 	return newGenericOption(func(cfg *Config) {
 		cfg.Traces.Insecure = true
 	})
 }
 
+func WithSecureTraces() GenericOption {
+	return newGenericOption(func(cfg *Config) {
+		cfg.Traces.Insecure = false
+	})
+}
+
 func WithInsecureMetrics() GenericOption {
 	return newGenericOption(func(cfg *Config) {
 		cfg.Metrics.Insecure = true
 	})
 }
 
+func WithSecureMetrics() GenericOption {
+	return newGenericOption(func(cfg *Config) {
+		cfg.Metrics.Insecure = false
+	})
+}
+
 func WithHeaders(headers map[string]string) GenericOption {
 	return newGenericOption(func(cfg *Config) {
 		cfg.Traces.Headers = headers

diff --git a/exporters/otlp/internal/otlpconfig/options_test.go b/exporters/otlp/internal/otlpconfig/options_test.go
@@ -124,6 +124,84 @@ func TestConfigs(t *testing.T) {
 				assert.Equal(t, "env_endpoint", c.Metrics.Endpoint)
 			},
 		},
+		{
+			name: "Test Environment Endpoint with HTTP scheme",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "http://env_endpoint",
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "env_endpoint", c.Traces.Endpoint)
+				assert.Equal(t, "env_endpoint", c.Metrics.Endpoint)
+				assert.Equal(t, true, c.Traces.Insecure)
+				assert.Equal(t, true, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test Environment Endpoint with HTTP scheme and leading & trailingspaces",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "      http://env_endpoint    ",
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "env_endpoint", c.Traces.Endpoint)
+				assert.Equal(t, "env_endpoint", c.Metrics.Endpoint)
+				assert.Equal(t, true, c.Traces.Insecure)
+				assert.Equal(t, true, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test Environment Endpoint with HTTPS scheme",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT": "https://env_endpoint",
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "env_endpoint", c.Traces.Endpoint)
+				assert.Equal(t, "env_endpoint", c.Metrics.Endpoint)
+				assert.Equal(t, false, c.Traces.Insecure)
+				assert.Equal(t, false, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test Environment Signal Specific Endpoint",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT":         "http://overrode_by_signal_specific",
+				"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT":  "http://env_traces_endpoint",
+				"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "https://env_metrics_endpoint",
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "env_traces_endpoint", c.Traces.Endpoint)
+				assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint)
+				assert.Equal(t, true, c.Traces.Insecure)
+				assert.Equal(t, false, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test Environment Signal Specific Endpoint #2",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT":         "http://overrode_by_signal_specific",
+				"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT":  "http://env_traces_endpoint",
+				"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_metrics_endpoint",
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "env_traces_endpoint", c.Traces.Endpoint)
+				assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint)
+				assert.Equal(t, true, c.Traces.Insecure)
+				assert.Equal(t, false, c.Metrics.Insecure)
+			},
+		},
+		{
+			name: "Test Environment Signal Specific Endpoint with uppercase scheme",
+			env: map[string]string{
+				"OTEL_EXPORTER_OTLP_ENDPOINT":         "HTTP://overrode_by_signal_specific",
+				"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT":  "HtTp://env_traces_endpoint",
+				"OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_metrics_endpoint",
+			},
+			asserts: func(t *testing.T, c *Config, grpcOption bool) {
+				assert.Equal(t, "env_traces_endpoint", c.Traces.Endpoint)
+				assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint)
+				assert.Equal(t, true, c.Traces.Insecure)
+				assert.Equal(t, false, c.Metrics.Insecure)
+			},
+		},
 
 		// Certificate tests
 		{