From 21c1641831ca19e3acf341cc11459c87b9791f2f Mon Sep 17 00:00:00 2001 From: Marco Hofstetter Date: Thu, 13 May 2021 01:17:37 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../otlp/internal/otlpconfig/envconfig.go | 35 ++++++++- exporters/otlp/internal/otlpconfig/options.go | 19 +++++ .../otlp/internal/otlpconfig/options_test.go | 78 +++++++++++++++++++ 4 files changed, 130 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2cda4b4fd1..af0c2db0607 100644 --- 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 index b52f3dc537a..e17f2f1d102 100644 --- 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 index 53fe06e0618..4f4387a1a94 100644 --- 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 index 7a70ba53619..eba7e2b1e24 100644 --- 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 {