From 9653a6a4510cc2da8415e717e9703769fea646b6 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 23 Jul 2024 17:55:07 -0700 Subject: [PATCH 1/5] Correct the comment for the priority of options and environments on otlpmetricgrpc --- exporters/otlp/otlpmetric/otlpmetricgrpc/config.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/config.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/config.go index 38d7d60d403..db6e3714b3b 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/config.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/config.go @@ -66,8 +66,9 @@ func WithInsecure() Option { // // If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_METRICS_ENDPOINT // environment variable is set, and this option is not passed, that variable -// value will be used. If both are set, OTEL_EXPORTER_OTLP_METRICS_ENDPOINT -// will take precedence. +// value will be used. If both environment variables are set, +// OTEL_EXPORTER_OTLP_METRICS_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 WithEndpointURL are used, the last used option will // take precedence. @@ -84,8 +85,9 @@ func WithEndpoint(endpoint string) Option { // // If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_METRICS_ENDPOINT // environment variable is set, and this option is not passed, that variable -// value will be used. If both are set, OTEL_EXPORTER_OTLP_METRICS_ENDPOINT -// will take precedence. +// value will be used. If both environment variables are set, +// OTEL_EXPORTER_OTLP_METRICS_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 75b2902bea4f013cec6042eb320bb72c4f5e9973 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 23 Jul 2024 17:58:36 -0700 Subject: [PATCH 2/5] Correct the comment for the priority of options and environments on otlpmetrichttp --- exporters/otlp/otlpmetric/otlpmetrichttp/config.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/config.go b/exporters/otlp/otlpmetric/otlpmetrichttp/config.go index 4e08d9293da..bf05adcf1b1 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/config.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/config.go @@ -63,8 +63,9 @@ func (w wrappedOption) applyHTTPOption(cfg oconf.Config) oconf.Config { // // If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_METRICS_ENDPOINT // environment variable is set, and this option is not passed, that variable -// value will be used. If both are set, OTEL_EXPORTER_OTLP_METRICS_ENDPOINT -// will take precedence. +// value will be used. If both environment variables are set, +// OTEL_EXPORTER_OTLP_METRICS_ENDPOINT will take precedence. If an environment +// variable is set, and this option is passed, this option will take precedence. // // By default, if an environment variable is not set, and this option is not // passed, "localhost:4318" will be used. @@ -76,8 +77,9 @@ func WithEndpoint(endpoint string) Option { // // If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_METRICS_ENDPOINT // environment variable is set, and this option is not passed, that variable -// value will be used. If both are set, OTEL_EXPORTER_OTLP_METRICS_ENDPOINT -// will take precedence. +// value will be used. If both environment variables are set, +// OTEL_EXPORTER_OTLP_METRICS_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 8204fe6b2641e64d0b5d379c670b0843b6b74cca Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 23 Jul 2024 17:58:56 -0700 Subject: [PATCH 3/5] Add more tests --- .../internal/oconf/options_test.go | 38 ++++++++++++++++++- .../internal/oconf/options_test.go | 36 ++++++++++++++++++ .../otlpmetric/oconf/options_test.go.tmpl | 36 ++++++++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go index 8b935722d10..f5fd29966ba 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go @@ -129,6 +129,26 @@ func TestConfigs(t *testing.T) { assert.Equal(t, "/v1/metrics", c.Metrics.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.Metrics.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.Metrics.Endpoint) + }, + }, { name: "Test Environment Endpoint", env: map[string]string{ @@ -161,15 +181,31 @@ func TestConfigs(t *testing.T) { { name: "Test Mixed Environment and With Endpoint", opts: []GenericOption{ + WithEndpointURL("https://metrics_endpoint2/somepath"), WithEndpoint("metrics_endpoint"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { assert.Equal(t, "metrics_endpoint", c.Metrics.Endpoint) }, }, + { + name: "Test Mixed Environment and With Endpoint", + opts: []GenericOption{ + WithEndpoint("metrics_endpoint"), + WithEndpointURL("https://metrics_endpoint2/somepath"), + }, + env: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "metrics_endpoint2", c.Metrics.Endpoint) + }, + }, { name: "Test Environment Endpoint with HTTP scheme", env: map[string]string{ diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go index b9bdb1c006a..bd6a07a85f6 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go @@ -129,6 +129,26 @@ func TestConfigs(t *testing.T) { assert.Equal(t, "/v1/metrics", c.Metrics.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.Metrics.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.Metrics.Endpoint) + }, + }, { name: "Test Environment Endpoint", env: map[string]string{ @@ -161,15 +181,31 @@ func TestConfigs(t *testing.T) { { name: "Test Mixed Environment and With Endpoint", opts: []GenericOption{ + WithEndpointURL("https://metrics_endpoint2/somepath"), WithEndpoint("metrics_endpoint"), }, env: map[string]string{ "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { assert.Equal(t, "metrics_endpoint", c.Metrics.Endpoint) }, }, + { + name: "Test Mixed Environment and With Endpoint", + opts: []GenericOption{ + WithEndpoint("metrics_endpoint"), + WithEndpointURL("https://metrics_endpoint2/somepath"), + }, + env: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "metrics_endpoint2", c.Metrics.Endpoint) + }, + }, { name: "Test Environment Endpoint with HTTP scheme", env: map[string]string{ diff --git a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl index 22843320dc1..286658751cd 100644 --- a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl @@ -129,6 +129,26 @@ func TestConfigs(t *testing.T) { assert.Equal(t, "/v1/metrics", c.Metrics.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.Metrics.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.Metrics.Endpoint) + }, + }, { name: "Test Environment Endpoint", env: map[string]string{ @@ -161,15 +181,31 @@ func TestConfigs(t *testing.T) { { name: "Test Mixed Environment and With Endpoint", opts: []GenericOption{ + WithEndpointURL("https://metrics_endpoint2/somepath"), WithEndpoint("metrics_endpoint"), }, env: map[string]string{ "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { assert.Equal(t, "metrics_endpoint", c.Metrics.Endpoint) }, }, + { + name: "Test Mixed Environment and With Endpoint", + opts: []GenericOption{ + WithEndpoint("metrics_endpoint"), + WithEndpointURL("https://metrics_endpoint2/somepath"), + }, + env: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, "metrics_endpoint2", c.Metrics.Endpoint) + }, + }, { name: "Test Environment Endpoint with HTTP scheme", env: map[string]string{ From 8fa0e4018dae51b16e4421ffa919134c156703cc Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 23 Jul 2024 18:01:48 -0700 Subject: [PATCH 4/5] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64357fd3148..a4a79db719f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Correct the `Meter` name used in `go.opentelemetry.io/otel/example/prometheus`. (#5612) - Correct the `Tracer` names used in `go.opentelemetry.io/otel/example/zipkin`. (#5612) - Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5541) +- Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their corresponding environment variables in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5641) From 220b6cb5efb81c5a73497a609bf6a3c173076227 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 23 Jul 2024 18:12:04 -0700 Subject: [PATCH 5/5] Fix linter --- .../otlpmetric/otlpmetrichttp/internal/oconf/options_test.go | 4 ++-- internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go index bd6a07a85f6..1e44d3e379c 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go @@ -185,7 +185,7 @@ func TestConfigs(t *testing.T) { WithEndpoint("metrics_endpoint"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { @@ -199,7 +199,7 @@ func TestConfigs(t *testing.T) { WithEndpointURL("https://metrics_endpoint2/somepath"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { diff --git a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl index 286658751cd..ac041876fed 100644 --- a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl @@ -185,7 +185,7 @@ func TestConfigs(t *testing.T) { WithEndpoint("metrics_endpoint"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) { @@ -199,7 +199,7 @@ func TestConfigs(t *testing.T) { WithEndpointURL("https://metrics_endpoint2/somepath"), }, env: map[string]string{ - "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", + "OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint", "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT": "env_endpoint2", }, asserts: func(t *testing.T, c *Config, grpcOption bool) {