Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for scheme in OTEL_EXPORTER_OTLP_ENDPOINT #1886

Merged
merged 5 commits into from May 12, 2021
Merged

Add support for scheme in OTEL_EXPORTER_OTLP_ENDPOINT #1886

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9b3a6c0
Add support for scheme in OTEL_EXPORTER_OTLP_ENDPOINT according to th…
May 7, 2021
a757474
Changes after review from pellared
mhofstetter May 12, 2021
bc54d52
Changes after review from pellared - 2
mhofstetter May 12, 2021
7c8a82e
Changes after review from pellared - 3
mhofstetter May 12, 2021
12fcadd
Merge branch 'main' into feature/env-spec
Aneurysm9 May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -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)
mhofstetter marked this conversation as resolved.
Show resolved Hide resolved

### Changed

Expand Down
18 changes: 15 additions & 3 deletions exporters/otlp/internal/otlpconfig/envconfig.go
View file
Open in desktop
Expand Up @@ -71,13 +71,13 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption {

// Endpoint
if v, ok := e.getEnvValue("ENDPOINT"); ok {
opts = append(opts, WithEndpoint(v))
opts = append(opts, endpointOptions(v, WithEndpoint, withInsecure)...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be simplified, and clarified by doing something like:

opts = append(opts, 
    WithEndpoint(trimSchema(v)),
    withInsecure(strings.HasPrefix(v, "http://")),
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it towards this direction, but with an explicit WithSecure function which makes it a little bit more verbose

}
if v, ok := e.getEnvValue("TRACES_ENDPOINT"); ok {
opts = append(opts, WithTracesEndpoint(v))
opts = append(opts, endpointOptions(v, WithTracesEndpoint, withInsecureTraces)...)
}
if v, ok := e.getEnvValue("METRICS_ENDPOINT"); ok {
opts = append(opts, WithMetricsEndpoint(v))
opts = append(opts, endpointOptions(v, WithMetricsEndpoint, withInsecureMetrics)...)
}

// Certificate File
Expand Down Expand Up @@ -145,6 +145,18 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption {
return opts
}

func endpointOptions(endpoint string, endpointOption func(endpoint string) GenericOption, insecureOption func(insecure bool) GenericOption) []GenericOption {
var endpointOptions []GenericOption

endpointOptions = append(endpointOptions, insecureOption(strings.HasPrefix(endpoint, "http://")))
mhofstetter marked this conversation as resolved.
Show resolved Hide resolved

endpoint = strings.Replace(endpoint, "https://", "", 1)
endpoint = strings.Replace(endpoint, "http://", "", 1)
mhofstetter marked this conversation as resolved.
Show resolved Hide resolved
endpointOptions = append(endpointOptions, endpointOption(endpoint))
mhofstetter marked this conversation as resolved.
Show resolved Hide resolved

return endpointOptions
}

// 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) {
Expand Down
20 changes: 16 additions & 4 deletions exporters/otlp/internal/otlpconfig/options.go
View file
Open in desktop
Expand Up @@ -297,21 +297,33 @@ func WithMetricsTLSClientConfig(tlsCfg *tls.Config) GenericOption {
}

func WithInsecure() GenericOption {
return withInsecure(true)
}

func withInsecure(insecure bool) GenericOption {
return newGenericOption(func(cfg *Config) {
cfg.Traces.Insecure = true
cfg.Metrics.Insecure = true
cfg.Traces.Insecure = insecure
cfg.Metrics.Insecure = insecure
})
}

func WithInsecureTraces() GenericOption {
return withInsecureTraces(true)
}

func withInsecureTraces(insecure bool) GenericOption {
return newGenericOption(func(cfg *Config) {
cfg.Traces.Insecure = true
cfg.Traces.Insecure = insecure
})
}

func WithInsecureMetrics() GenericOption {
return withInsecureMetrics(true)
}

func withInsecureMetrics(insecure bool) GenericOption {
return newGenericOption(func(cfg *Config) {
cfg.Metrics.Insecure = true
cfg.Metrics.Insecure = insecure
})
}

Expand Down
52 changes: 52 additions & 0 deletions exporters/otlp/internal/otlpconfig/options_test.go
View file
Open in desktop
Expand Up @@ -124,6 +124,58 @@ 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 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)
},
},

// Certificate tests
{
Expand Down