From 1e06c7260cdf2ed728bc627bd01d67b55e443c9d Mon Sep 17 00:00:00 2001
From: Sam Xie <sam@samxie.me>
Date: Wed, 12 Jun 2024 20:13:36 -0700
Subject: [PATCH] Correct the comment of the priority of options and
 environments on otlptracegrpc

---
 .../otlptrace/otlptracegrpc/client_test.go    | 13 ++++
 .../otlptracegrpc/client_unit_test.go         | 72 +++++++++++++++++++
 .../otlp/otlptrace/otlptracegrpc/options.go   | 10 +--
 3 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go
index 4d626d30b84..5b9518b275a 100644
--- a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go
+++ b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go
@@ -420,3 +420,16 @@ func TestCustomUserAgent(t *testing.T) {
 	headers := mc.getHeaders()
 	require.Contains(t, headers.Get("user-agent")[0], customUserAgent)
 }
+
+func TestNewClient(t *testing.T) {
+	mc := runMockCollector(t)
+	t.Cleanup(func() { require.NoError(t, mc.stop()) })
+
+	ctx := context.Background()
+	client := otlptracegrpc.NewClient(
+		otlptracegrpc.WithInsecure(),
+		otlptracegrpc.WithEndpoint(mc.endpoint),
+	)
+	t.Cleanup(func() { require.NoError(t, client.Stop(ctx)) })
+	require.NoError(t, client.Start(ctx))
+}
diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_unit_test.go
index 5391030d7df..e9d7f31c0e0 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://test:8080/path"),
+				WithEndpoint("someendpoint"),
+			},
+			want: "someendpoint",
+		},
+	}
+
+	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.