From b3c5c4dc8b6a6e54a4485d7528bac18a10ea2634 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 12 Jun 2024 20:13:36 -0700 Subject: [PATCH 1/4] Correct the comment of the priority of options and environments on otlptracegrpc --- .../otlptracegrpc/client_unit_test.go | 72 +++++++++++++++++++ .../otlp/otlptrace/otlptracegrpc/options.go | 10 +-- 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go index 5391030d7df..89eeb864129 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go @@ -202,3 +202,75 @@ func TestExportContextLinksStopSignal(t *testing.T) { return false }, 10*time.Second, time.Microsecond) } + +func TestWithEndpointWithEnv(t *testing.T) { + testCases := []struct { + name string + options []Option + envs map[string]string + want string + }{ + { + name: "WithEndpointURL last", + options: []Option{ + WithEndpoint("foo"), + WithEndpointURL("http://bar:8080/path"), + }, + want: "bar:8080", + }, + { + name: "WithEndpoint last", + options: []Option{ + WithEndpointURL("http://bar:8080/path"), + WithEndpoint("foo"), + }, + want: "foo", + }, + { + name: "OTEL_EXPORTER_OTLP_ENDPOINT only", + envs: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "foo2", + }, + want: "foo2", + }, + { + name: "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT only", + envs: map[string]string{ + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "bar2", + }, + want: "bar2", + }, + { + name: "both OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + envs: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "foo2", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "bar2", + }, + want: "bar2", + }, + { + name: "both options and envs", + envs: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "foo2", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "bar2", + }, + options: []Option{ + WithEndpointURL("http://bar:8080/path"), + WithEndpoint("foo"), + }, + want: "foo", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + for key, value := range tc.envs { + t.Setenv(key, value) + } + + client := newClient(tc.options...) + + assert.Equal(t, tc.want, client.endpoint) + }) + } +} diff --git a/exporters/otlp/otlptrace/otlptracegrpc/options.go b/exporters/otlp/otlptrace/otlptracegrpc/options.go index bbad0e6d01e..a4775504ba7 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/options.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/options.go @@ -59,8 +59,9 @@ func WithInsecure() 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 variable are set, +// OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If the 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. @@ -79,8 +80,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 variable are set, +// OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If the 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 c526ecc46aaec7fe52c1bff217ec7469159483b3 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Wed, 12 Jun 2024 20:23:33 -0700 Subject: [PATCH 2/4] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 245f4cffa4b..1e49adb9ef9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Document instrument name requirements in `go.opentelemetry.io/otel/metric`. (#5435) - Prevent random number generation data-race for experimental rand exemplars in `go.opentelemetry.io/otel/sdk/metric`. (#5456) - Fix counting number of dropped attributes of `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5464) +- Correct comments for the priority of options and environments in `WithEndpoint` and `WithEndpointURL` in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5508) ## [1.27.0/0.49.0/0.3.0] 2024-05-21 From 11a74556de528de09d1c5d5aa2f6aad95f563998 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 13 Jun 2024 09:05:43 -0700 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: David Ashpole --- exporters/otlp/otlptrace/otlptracegrpc/options.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/otlptrace/otlptracegrpc/options.go b/exporters/otlp/otlptrace/otlptracegrpc/options.go index a4775504ba7..740143f26d3 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/options.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/options.go @@ -59,7 +59,7 @@ func WithInsecure() 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 environment variable are set, +// value will be used. If both environment variables are set, // OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If the environment // variable is set, and this option is passed, this option will take precedence. // @@ -80,7 +80,7 @@ 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 environment variable are set, +// value will be used. If both environment variables are set, // OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If the environment // variable is set, and this option is passed, this option will take precedence. // From 905fcb76eee6fe21ba58f1464f35a3b91cbb986a Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 18 Jun 2024 09:24:56 -0700 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- .../otlp/otlptrace/otlptracegrpc/client_unit_test.go | 12 ++++++++++++ exporters/otlp/otlptrace/otlptracegrpc/options.go | 4 ++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e49adb9ef9..47d72eab223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Document instrument name requirements in `go.opentelemetry.io/otel/metric`. (#5435) - Prevent random number generation data-race for experimental rand exemplars in `go.opentelemetry.io/otel/sdk/metric`. (#5456) - Fix counting number of dropped attributes of `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5464) -- Correct comments for the priority of options and environments in `WithEndpoint` and `WithEndpointURL` in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5508) +- Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their coresponding environment variables in in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5508) ## [1.27.0/0.49.0/0.3.0] 2024-05-21 diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go index 89eeb864129..89f2351cda8 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go @@ -260,6 +260,18 @@ func TestWithEndpointWithEnv(t *testing.T) { }, want: "foo", }, + { + name: "both options and envs", + envs: map[string]string{ + "OTEL_EXPORTER_OTLP_ENDPOINT": "foo2", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "bar2", + }, + options: []Option{ + WithEndpoint("foo"), + WithEndpointURL("http://bar:8080/path"), + }, + want: "bar:8080", + }, } for _, tc := range testCases { diff --git a/exporters/otlp/otlptrace/otlptracegrpc/options.go b/exporters/otlp/otlptrace/otlptracegrpc/options.go index 740143f26d3..00ab1f20c6d 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/options.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/options.go @@ -60,7 +60,7 @@ func WithInsecure() 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 environment variables are set, -// OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If the environment +// 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 WithEndpointURL are used, the last used option will @@ -81,7 +81,7 @@ 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 environment variables are set, -// OTEL_EXPORTER_OTLP_TRACES_ENDPOINT will take precedence. If the environment +// 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