From decc7c8afe7bee8a3a9ee8df9135e1cfd20d9263 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 7 Jul 2024 17:05:02 -0700 Subject: [PATCH 1/3] Correct the comment for the priority of options and environments on otlptracehttp --- exporters/otlp/otlptrace/otlptracehttp/options.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/otlptrace/otlptracehttp/options.go b/exporters/otlp/otlptrace/otlptracehttp/options.go index 6497f3ccdd0..3559c5664f4 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/options.go +++ b/exporters/otlp/otlptrace/otlptracehttp/options.go @@ -62,8 +62,10 @@ func (w wrappedOption) applyHTTPOption(cfg otlpconfig.Config) otlpconfig.Config // // If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_TRACES_ENDPOINT // environment variable is set, and this option is not passed, that variable -// value will be used. If both are set, OTEL_EXPORTER_OTLP_TRACES_ENDPOINT -// will take precedence. Note, both environment variables include the full +// value will be used. If both environment variables are set, +// OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If an environment +// variable is set, and this option is passed, this option will take precedence. +// Note, both environment variables include the full // scheme and path, while WithEndpoint sets only the host and port. // // If both this option and WithEndpointURL are used, the last used option will @@ -82,8 +84,9 @@ func WithEndpoint(endpoint string) Option { // // If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_TRACES_ENDPOINT // environment variable is set, and this option is not passed, that variable -// value will be used. If both are set, OTEL_EXPORTER_OTLP_TRACES_ENDPOINT -// will take precedence. +// value will be used. If both environment variables are set, +// OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If an environment +// variable is set, and this option is passed, this option will take precedence. // // If both this option and WithEndpoint are used, the last used option will // take precedence. From 1fe123377e4ab6478cbb599f338d5e0aa47e26ca Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 7 Jul 2024 17:05:27 -0700 Subject: [PATCH 2/3] Add more tests --- .../internal/otlpconfig/options_test.go | 38 ++++++++++++++++++- .../internal/otlpconfig/options_test.go | 38 ++++++++++++++++++- .../otlptrace/otlpconfig/options_test.go.tmpl | 38 ++++++++++++++++++- 3 files changed, 111 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options_test.go index 715c04fd6b3..46d0d105fb4 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options_test.go @@ -127,6 +127,26 @@ func TestConfigs(t *testing.T) { assert.Equal(t, "/v1/traces", c.Traces.URLPath) }, }, + { + name: "Test With Endpoint last used", + opts: []GenericOption{ + WithEndpointURL("https://someendpoint/somepath"), + WithEndpoint("someendpoint2"), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "someendpoint2", c.Traces.Endpoint) + }, + }, + { + name: "Test With WithEndpointURL last used", + opts: []GenericOption{ + WithEndpoint("someendpoint2"), + WithEndpointURL("https://someendpoint/somepath"), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "someendpoint", c.Traces.Endpoint) + }, + }, { name: "Test Environment Endpoint", env: map[string]string{ @@ -159,15 +179,31 @@ func TestConfigs(t *testing.T) { { name: "Test Mixed Environment and With Endpoint", opts: []GenericOption{ + WithEndpointURL("https://traces_endpoint2/somepath"), WithEndpoint("traces_endpoint"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { assert.Equal(t, "traces_endpoint", c.Traces.Endpoint) }, }, + { + name: "Test Mixed Environment and With Endpoint", + opts: []GenericOption{ + WithEndpoint("traces_endpoint"), + WithEndpointURL("https://traces_endpoint2/somepath"), + }, + env: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_endpoint2", + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "traces_endpoint2", c.Traces.Endpoint) + }, + }, { name: "Test Environment Endpoint with HTTP scheme", env: map[string]string{ diff --git a/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options_test.go index 1a4e5aab110..7ac0711d0eb 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options_test.go @@ -127,6 +127,26 @@ func TestConfigs(t *testing.T) { assert.Equal(t, "/v1/traces", c.Traces.URLPath) }, }, + { + name: "Test With Endpoint last used", + opts: []GenericOption{ + WithEndpointURL("https://someendpoint/somepath"), + WithEndpoint("someendpoint2"), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "someendpoint2", c.Traces.Endpoint) + }, + }, + { + name: "Test With WithEndpointURL last used", + opts: []GenericOption{ + WithEndpoint("someendpoint2"), + WithEndpointURL("https://someendpoint/somepath"), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "someendpoint", c.Traces.Endpoint) + }, + }, { name: "Test Environment Endpoint", env: map[string]string{ @@ -159,15 +179,31 @@ func TestConfigs(t *testing.T) { { name: "Test Mixed Environment and With Endpoint", opts: []GenericOption{ + WithEndpointURL("https://traces_endpoint2/somepath"), WithEndpoint("traces_endpoint"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { assert.Equal(t, "traces_endpoint", c.Traces.Endpoint) }, }, + { + name: "Test Mixed Environment and With Endpoint", + opts: []GenericOption{ + WithEndpoint("traces_endpoint"), + WithEndpointURL("https://traces_endpoint2/somepath"), + }, + env: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_endpoint2", + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "traces_endpoint2", c.Traces.Endpoint) + }, + }, { name: "Test Environment Endpoint with HTTP scheme", env: map[string]string{ diff --git a/internal/shared/otlp/otlptrace/otlpconfig/options_test.go.tmpl b/internal/shared/otlp/otlptrace/otlpconfig/options_test.go.tmpl index 8d670a5aa9a..2688db9db51 100644 --- a/internal/shared/otlp/otlptrace/otlpconfig/options_test.go.tmpl +++ b/internal/shared/otlp/otlptrace/otlpconfig/options_test.go.tmpl @@ -127,6 +127,26 @@ func TestConfigs(t *testing.T) { assert.Equal(t, "/v1/traces", c.Traces.URLPath) }, }, + { + name: "Test With Endpoint last used", + opts: []GenericOption{ + WithEndpointURL("https://someendpoint/somepath"), + WithEndpoint("someendpoint2"), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "someendpoint2", c.Traces.Endpoint) + }, + }, + { + name: "Test With WithEndpointURL last used", + opts: []GenericOption{ + WithEndpoint("someendpoint2"), + WithEndpointURL("https://someendpoint/somepath"), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "someendpoint", c.Traces.Endpoint) + }, + }, { name: "Test Environment Endpoint", env: map[string]string{ @@ -159,15 +179,31 @@ func TestConfigs(t *testing.T) { { name: "Test Mixed Environment and With Endpoint", opts: []GenericOption{ + WithEndpointURL("https://traces_endpoint2/somepath"), WithEndpoint("traces_endpoint"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { assert.Equal(t, "traces_endpoint", c.Traces.Endpoint) }, }, + { + name: "Test Mixed Environment and With Endpoint", + opts: []GenericOption{ + WithEndpoint("traces_endpoint"), + WithEndpointURL("https://traces_endpoint2/somepath"), + }, + env: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "env_endpoint2", + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "traces_endpoint2", c.Traces.Endpoint) + }, + }, { name: "Test Environment Endpoint with HTTP scheme", env: map[string]string{ From 13f3b2fd6b93b80e3e0b0e62654c4ef917181675 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Sun, 7 Jul 2024 17:11:04 -0700 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c01e6998e0b..d5893728cb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Fixed + +- Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their corresponding environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5584) + ## [1.28.0/0.50.0/0.4.0] 2024-07-02 ### Added