diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5b50dc4ad..88706af9b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `"go.opentelemetry.io/otel/exporter/otel/otlpmetric/otlpmetricgrpc".RetrySettings` type is renamed to `RetryConfig`. (#2425) - The `go.opentelemetry.io/otel/exporter/otel/*` gRPC exporters now default to using the host's root CA set if none are provided by the user and `WithInsecure` is not specified. (#1584, #2432) +### Fixed + +- The `go.opentelemetry.io/otel/exporter/otel/*` exporters are updated to handle per-signal and universal endpoints according to the OpenTelemetry specification. + Any per-signal endpoint set via an `OTEL_EXPORTER_OTLP__ENDPOINT` environment variable is now used without modification of the path. + When `OTEL_EXPORTER_OTLP_ENDPOINT` is set, if it contains a path, that path is used as a base path which per-signal paths are appended to. (#2338 #2433) + ### Deprecated - Deprecated the `"go.opentelemetry.io/otel/exporter/otel/otlpmetric/otlpmetrichttp".WithMaxAttempts` `Option`, use the new `WithRetry` `Option` instead. (#2425) diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go index 1d1663415cf..a62b548496f 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go @@ -20,7 +20,7 @@ import ( "io/ioutil" "net/url" "os" - "regexp" + "path" "strconv" "strings" "time" @@ -28,14 +28,10 @@ import ( "go.opentelemetry.io/otel" ) -var ( - httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`) - - DefaultEnvOptionsReader = EnvOptionsReader{ - GetEnv: os.Getenv, - ReadFile: ioutil.ReadFile, - } -) +var DefaultEnvOptionsReader = EnvOptionsReader{ + GetEnv: os.Getenv, + ReadFile: ioutil.ReadFile, +} func ApplyGRPCEnvConfigs(cfg *Config) { DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) @@ -68,23 +64,55 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { var opts []GenericOption // Endpoint - if v, ok := e.getEnvValue("ENDPOINT"); ok { - if isInsecureEndpoint(v) { - opts = append(opts, WithInsecure()) - } else { - opts = append(opts, WithSecure()) - } - - opts = append(opts, WithEndpoint(trimSchema(v))) - } if v, ok := e.getEnvValue("METRICS_ENDPOINT"); ok { - if isInsecureEndpoint(v) { - opts = append(opts, WithInsecure()) - } else { - opts = append(opts, WithSecure()) + u, err := url.Parse(v) + // Ignore invalid values. + if err == nil { + // This is used to set the scheme for OTLP/HTTP. + if insecureSchema(u.Scheme) { + opts = append(opts, WithInsecure()) + } else { + opts = append(opts, WithSecure()) + } + opts = append(opts, newSplitOption(func(cfg *Config) { + cfg.Metrics.Endpoint = u.Host + // For endpoint URLs for OTLP/HTTP per-signal variables, the + // URL MUST be used as-is without any modification. The only + // exception is that if an URL contains no path part, the root + // path / MUST be used. + path := u.Path + if path == "" { + path = "/" + } + cfg.Metrics.URLPath = path + }, func(cfg *Config) { + // For OTLP/gRPC endpoints, this is the target to which the + // exporter is going to send telemetry. + cfg.Metrics.Endpoint = path.Join(u.Host, u.Path) + })) + } + } else if v, ok = e.getEnvValue("ENDPOINT"); ok { + u, err := url.Parse(v) + // Ignore invalid values. + if err == nil { + // This is used to set the scheme for OTLP/HTTP. + if insecureSchema(u.Scheme) { + opts = append(opts, WithInsecure()) + } else { + opts = append(opts, WithSecure()) + } + opts = append(opts, newSplitOption(func(cfg *Config) { + cfg.Metrics.Endpoint = u.Host + // For OTLP/HTTP endpoint URLs without a per-signal + // configuration, the passed endpoint is used as a base URL + // and the signals are sent to these paths relative to that. + cfg.Metrics.URLPath = path.Join(u.Path, DefaultMetricsPath) + }, func(cfg *Config) { + // For OTLP/gRPC endpoints, this is the target to which the + // exporter is going to send telemetry. + cfg.Metrics.Endpoint = path.Join(u.Host, u.Path) + })) } - - opts = append(opts, WithEndpoint(trimSchema(v))) } // Certificate File @@ -134,12 +162,13 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { return opts } -func isInsecureEndpoint(endpoint string) bool { - return strings.HasPrefix(strings.ToLower(endpoint), "http://") || strings.HasPrefix(strings.ToLower(endpoint), "unix://") -} - -func trimSchema(endpoint string) string { - return httpSchemeRegexp.ReplaceAllString(endpoint, "") +func insecureSchema(schema string) bool { + switch strings.ToLower(schema) { + case "http", "unix": + return true + default: + return false + } } // getEnvValue gets an OTLP environment variable value of the specified key using the GetEnv function. diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go index cd6694cce43..ee267e4fd00 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go @@ -96,20 +96,30 @@ func TestConfigs(t *testing.T) { { name: "Test Environment Endpoint", env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "https://env.endpoint/prefix", }, asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_endpoint", c.Metrics.Endpoint) + assert.False(t, c.Metrics.Insecure) + if grpcOption { + assert.Equal(t, "env.endpoint/prefix", c.Metrics.Endpoint) + } else { + assert.Equal(t, "env.endpoint", c.Metrics.Endpoint) + assert.Equal(t, "/prefix/v1/metrics", c.Metrics.URLPath) + } }, }, { name: "Test Environment Signal Specific Endpoint", env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "overrode_by_signal_specific", - "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_metrics_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "https://overrode.by.signal.specific/env/var", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "http://env.metrics.endpoint", }, asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint) + assert.True(t, c.Metrics.Insecure) + assert.Equal(t, "env.metrics.endpoint", c.Metrics.Endpoint) + if !grpcOption { + assert.Equal(t, "/", c.Metrics.URLPath) + } }, }, { @@ -154,37 +164,15 @@ func TestConfigs(t *testing.T) { 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_METRICS_ENDPOINT": "https://env_metrics_endpoint", - }, - asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint) - 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_METRICS_ENDPOINT": "env_metrics_endpoint", - }, - asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint) - 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_METRICS_ENDPOINT": "env_metrics_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "HTTPS://overrode_by_signal_specific", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "HtTp://env_metrics_endpoint", }, asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { assert.Equal(t, "env_metrics_endpoint", c.Metrics.Endpoint) - assert.Equal(t, false, c.Metrics.Insecure) + assert.Equal(t, true, c.Metrics.Insecure) }, }, diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index b78067dc075..55334a221c6 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "net" "net/http" + "net/url" "path" "strconv" "strings" @@ -199,8 +200,8 @@ func (d *client) UploadMetrics(ctx context.Context, protoMetrics []*metricpb.Res } func (d *client) newRequest(body []byte) (request, error) { - address := fmt.Sprintf("%s://%s%s", d.getScheme(), d.cfg.Endpoint, d.cfg.URLPath) - r, err := http.NewRequest(http.MethodPost, address, nil) + u := url.URL{Scheme: d.getScheme(), Host: d.cfg.Endpoint, Path: d.cfg.URLPath} + r, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { return request{Request: r}, err } diff --git a/exporters/otlp/otlptrace/README.md b/exporters/otlp/otlptrace/README.md index 451ad6dc4df..8a40a86a246 100644 --- a/exporters/otlp/otlptrace/README.md +++ b/exporters/otlp/otlptrace/README.md @@ -33,8 +33,10 @@ The `otlptracehttp` package implements a client for the span exporter that sends ### Environment Variables -The following environment variables can be used -(instead of options objects) to override the default configuration. +The following environment variables can be used (instead of options objects) to +override the default configuration. For more information about how each of +these environment variables is interpreted, see [the OpenTelemetry +specification](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/protocol/exporter.md). | Environment variable | Option | Default value | | ------------------------------------------------------------------------ |------------------------------ | ----------------------------------- | diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go index 570d31ba8fa..2d1937beda2 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go @@ -20,7 +20,7 @@ import ( "io/ioutil" "net/url" "os" - "regexp" + "path" "strconv" "strings" "time" @@ -28,14 +28,10 @@ import ( "go.opentelemetry.io/otel" ) -var ( - httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`) - - DefaultEnvOptionsReader = EnvOptionsReader{ - GetEnv: os.Getenv, - ReadFile: ioutil.ReadFile, - } -) +var DefaultEnvOptionsReader = EnvOptionsReader{ + GetEnv: os.Getenv, + ReadFile: ioutil.ReadFile, +} func ApplyGRPCEnvConfigs(cfg *Config) { DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) @@ -68,23 +64,55 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { var opts []GenericOption // Endpoint - if v, ok := e.getEnvValue("ENDPOINT"); ok { - 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 { - if isInsecureEndpoint(v) { - opts = append(opts, WithInsecure()) - } else { - opts = append(opts, WithSecure()) + u, err := url.Parse(v) + // Ignore invalid values. + if err == nil { + // This is used to set the scheme for OTLP/HTTP. + if insecureSchema(u.Scheme) { + opts = append(opts, WithInsecure()) + } else { + opts = append(opts, WithSecure()) + } + opts = append(opts, newSplitOption(func(cfg *Config) { + cfg.Traces.Endpoint = u.Host + // For endpoint URLs for OTLP/HTTP per-signal variables, the + // URL MUST be used as-is without any modification. The only + // exception is that if an URL contains no path part, the root + // path / MUST be used. + path := u.Path + if path == "" { + path = "/" + } + cfg.Traces.URLPath = path + }, func(cfg *Config) { + // For OTLP/gRPC endpoints, this is the target to which the + // exporter is going to send telemetry. + cfg.Traces.Endpoint = path.Join(u.Host, u.Path) + })) + } + } else if v, ok = e.getEnvValue("ENDPOINT"); ok { + u, err := url.Parse(v) + // Ignore invalid values. + if err == nil { + // This is used to set the scheme for OTLP/HTTP. + if insecureSchema(u.Scheme) { + opts = append(opts, WithInsecure()) + } else { + opts = append(opts, WithSecure()) + } + opts = append(opts, newSplitOption(func(cfg *Config) { + cfg.Traces.Endpoint = u.Host + // For OTLP/HTTP endpoint URLs without a per-signal + // configuration, the passed endpoint is used as a base URL + // and the signals are sent to these paths relative to that. + cfg.Traces.URLPath = path.Join(u.Path, DefaultTracesPath) + }, func(cfg *Config) { + // For OTLP/gRPC endpoints, this is the target to which the + // exporter is going to send telemetry. + cfg.Traces.Endpoint = path.Join(u.Host, u.Path) + })) } - - opts = append(opts, WithEndpoint(trimSchema(v))) } // Certificate File @@ -133,12 +161,13 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { return opts } -func isInsecureEndpoint(endpoint string) bool { - return strings.HasPrefix(strings.ToLower(endpoint), "http://") || strings.HasPrefix(strings.ToLower(endpoint), "unix://") -} - -func trimSchema(endpoint string) string { - return httpSchemeRegexp.ReplaceAllString(endpoint, "") +func insecureSchema(schema string) bool { + switch strings.ToLower(schema) { + case "http", "unix": + return true + default: + return false + } } // getEnvValue gets an OTLP environment variable value of the specified key using the GetEnv function. diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go index 59bd09eab8c..20decbc37bc 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go @@ -96,20 +96,30 @@ func TestConfigs(t *testing.T) { { name: "Test Environment Endpoint", env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "https://env.endpoint/prefix", }, asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_endpoint", c.Traces.Endpoint) + assert.False(t, c.Traces.Insecure) + if grpcOption { + assert.Equal(t, "env.endpoint/prefix", c.Traces.Endpoint) + } else { + assert.Equal(t, "env.endpoint", c.Traces.Endpoint) + assert.Equal(t, "/prefix/v1/traces", c.Traces.URLPath) + } }, }, { name: "Test Environment Signal Specific Endpoint", env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "overrode_by_signal_specific", - "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_traces_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "https://overrode.by.signal.specific/env/var", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "http://env.traces.endpoint", }, asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_traces_endpoint", c.Traces.Endpoint) + assert.True(t, c.Traces.Insecure) + assert.Equal(t, "env.traces.endpoint", c.Traces.Endpoint) + if !grpcOption { + assert.Equal(t, "/", c.Traces.URLPath) + } }, }, { @@ -154,32 +164,10 @@ func TestConfigs(t *testing.T) { assert.Equal(t, false, c.Traces.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", - }, - asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_traces_endpoint", c.Traces.Endpoint) - assert.Equal(t, true, c.Traces.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", - }, - asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { - assert.Equal(t, "env_traces_endpoint", c.Traces.Endpoint) - assert.Equal(t, true, c.Traces.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_ENDPOINT": "HTTPS://overrode_by_signal_specific", "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "HtTp://env_traces_endpoint", }, asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index 93a7e4a7e86..dbc6c5ba785 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "net" "net/http" + "net/url" "path" "strconv" "strings" @@ -201,8 +202,8 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc } func (d *client) newRequest(body []byte) (request, error) { - address := fmt.Sprintf("%s://%s%s", d.getScheme(), d.cfg.Endpoint, d.cfg.URLPath) - r, err := http.NewRequest(http.MethodPost, address, nil) + u := url.URL{Scheme: d.getScheme(), Host: d.cfg.Endpoint, Path: d.cfg.URLPath} + r, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { return request{Request: r}, err }