From e43a01ca57ffadfaabfb6c8969e30fe1037df0e2 Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 18 Jun 2024 10:10:19 -0700 Subject: [PATCH] Correct the comment for the priority of options and environments on otlptracegrpc (#5508) part of #5379 --------- Co-authored-by: David Ashpole Co-authored-by: Chester Cheung Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + .../otlptracegrpc/client_unit_test.go | 84 +++++++++++++++++++ .../otlp/otlptrace/otlptracegrpc/options.go | 10 ++- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47c000336a3..a26f4b1a20a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix counting number of dropped attributes of `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5464) - Run the `Detect` method in `go.opentelemetry.io/otel/sdk/resource` in parallel. (#5402) - Fix panic in baggage creation when a member contains 0x80 char in key or value. (#5494) +- 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 5391030d7df..89f2351cda8 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go @@ -202,3 +202,87 @@ 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", + }, + { + 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 { + 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..00ab1f20c6d 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 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 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 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.