support zipkin exporter endpoint env (#2490)

* support zipkin exporter endpoint env

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

* update interface and changelog

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

* add pr number

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

* fix lint

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

* add import

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

CHANGELOG.md

@@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+### Added
+
+- Support `OTEL_EXPORTER_ZIPKIN_ENDPOINT` env to specify zipkin collector endpoint (#2490)
+
 ### Changed
 
 - Jaeger exporter takes into additional 70 bytes overhead into consideration when sending UDP packets (#2489)

exporters/zipkin/env.go

@@ -0,0 +1,31 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package zipkin // import "go.opentelemetry.io/otel/exporters/zipkin"
+
+import "os"
+
+// Environment variable names
+const (
+	// Endpoint for Zipkin collector
+	envEndpoint = "OTEL_EXPORTER_ZIPKIN_ENDPOINT"
+)
+
+// envOr returns an env variable's value if it is exists or the default if not
+func envOr(key, defaultValue string) string {
+	if v, ok := os.LookupEnv(key); ok && v != "" {
+		return v
+	}
+	return defaultValue
+}

exporters/zipkin/env_test.go

@@ -0,0 +1,62 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package zipkin
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	ottest "go.opentelemetry.io/otel/internal/internaltest"
+)
+
+func TestEnvOrWithCollectorEndpointOptionsFromEnv(t *testing.T) {
+	testCases := []struct {
+		name                      string
+		envEndpoint               string
+		defaultCollectorEndpoint  string
+		expectedCollectorEndpoint string
+	}{
+		{
+			name:                      "overrides value via environment variables",
+			envEndpoint:               "http://localhost:19411/foo",
+			defaultCollectorEndpoint:  defaultCollectorURL,
+			expectedCollectorEndpoint: "http://localhost:19411/foo",
+		},
+		{
+			name:                      "environment variables is empty, will not overwrite value",
+			envEndpoint:               "",
+			defaultCollectorEndpoint:  defaultCollectorURL,
+			expectedCollectorEndpoint: defaultCollectorURL,
+		},
+	}
+
+	envStore := ottest.NewEnvStore()
+	envStore.Record(envEndpoint)
+	defer func() {
+		require.NoError(t, envStore.Restore())
+	}()
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			require.NoError(t, os.Setenv(envEndpoint, tc.envEndpoint))
+
+			endpoint := envOr(envEndpoint, tc.defaultCollectorEndpoint)
+
+			assert.Equal(t, tc.expectedCollectorEndpoint, endpoint)
+		})
+	}
+}

exporters/zipkin/zipkin.go

@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -30,12 +29,15 @@ import (
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 )
 
+const (
+	defaultCollectorURL = "http://localhost:9411/api/v2/spans"
+)
+
 // Exporter exports spans to the zipkin collector.
 type Exporter struct {
 	url    string
 	client *http.Client
 	logger *log.Logger
-	config config
 
 	stoppedMu sync.RWMutex
 	stopped   bool
@@ -79,7 +81,8 @@ func WithClient(client *http.Client) Option {
 // New creates a new Zipkin exporter.
 func New(collectorURL string, opts ...Option) (*Exporter, error) {
 	if collectorURL == "" {
-		return nil, errors.New("collector URL cannot be empty")
+		// Use endpoint from env var or default collector URL.
+		collectorURL = envOr(envEndpoint, defaultCollectorURL)
 	}
 	u, err := url.Parse(collectorURL)
 	if err != nil {
@@ -93,6 +96,7 @@ func New(collectorURL string, opts ...Option) (*Exporter, error) {
 	for _, opt := range opts {
 		opt.apply(&cfg)
 	}
+
 	if cfg.client == nil {
 		cfg.client = http.DefaultClient
 	}
@@ -100,7 +104,6 @@ func New(collectorURL string, opts ...Option) (*Exporter, error) {
 		url:    collectorURL,
 		client: cfg.client,
 		logger: cfg.logger,
-		config: cfg,
 	}, nil
 }
 

exporters/zipkin/zipkin_test.go

@@ -26,6 +26,8 @@ import (
 	"testing"
 	"time"
 
+	ottest "go.opentelemetry.io/otel/internal/internaltest"
+
 	zkmodel "github.com/openzipkin/zipkin-go/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -38,13 +40,9 @@ import (
 	"go.opentelemetry.io/otel/trace"
 )
 
-const (
-	collectorURL = "http://localhost:9411/api/v2/spans"
-)
-
 func TestNewRawExporter(t *testing.T) {
 	_, err := New(
-		collectorURL,
+		defaultCollectorURL,
 	)
 
 	assert.NoError(t, err)
@@ -56,15 +54,6 @@ func TestNewRawExporterShouldFailInvalidCollectorURL(t *testing.T) {
 		err error
 	)
 
-	// cannot be empty
-	exp, err = New(
-		"",
-	)
-
-	assert.Error(t, err)
-	assert.EqualError(t, err, "collector URL cannot be empty")
-	assert.Nil(t, exp)
-
 	// invalid URL
 	exp, err = New(
 		"localhost",
@@ -75,6 +64,40 @@ func TestNewRawExporterShouldFailInvalidCollectorURL(t *testing.T) {
 	assert.Nil(t, exp)
 }
 
+func TestNewRawExporterEmptyDefaultCollectorURL(t *testing.T) {
+	var (
+		exp *Exporter
+		err error
+	)
+
+	// use default collector URL if not specified
+	exp, err = New("")
+
+	assert.NoError(t, err)
+	assert.Equal(t, defaultCollectorURL, exp.url)
+}
+
+func TestNewRawExporterCollectorURLFromEnv(t *testing.T) {
+	var (
+		exp *Exporter
+		err error
+	)
+
+	expectedEndpoint := "http://localhost:19411/api/v2/spans"
+	envStore, err := ottest.SetEnvVariables(map[string]string{
+		envEndpoint: expectedEndpoint,
+	})
+	assert.NoError(t, err)
+	defer func() {
+		require.NoError(t, envStore.Restore())
+	}()
+
+	exp, err = New("")
+
+	assert.NoError(t, err)
+	assert.Equal(t, expectedEndpoint, exp.url)
+}
+
 type mockZipkinCollector struct {
 	t       *testing.T
 	url     string
@@ -309,7 +332,7 @@ func TestExporterShutdownHonorsTimeout(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 	defer cancel()
 
-	exp, err := New(collectorURL)
+	exp, err := New("")
 	require.NoError(t, err)
 
 	innerCtx, innerCancel := context.WithTimeout(ctx, time.Nanosecond)
@@ -322,7 +345,7 @@ func TestExporterShutdownHonorsCancel(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 	defer cancel()
 
-	exp, err := New(collectorURL)
+	exp, err := New("")
 	require.NoError(t, err)
 
 	innerCtx, innerCancel := context.WithCancel(ctx)
@@ -331,7 +354,7 @@ func TestExporterShutdownHonorsCancel(t *testing.T) {
 }
 
 func TestErrorOnExportShutdownExporter(t *testing.T) {
-	exp, err := New(collectorURL)
+	exp, err := New("")
 	require.NoError(t, err)
 	assert.NoError(t, exp.Shutdown(context.Background()))
 	assert.NoError(t, exp.ExportSpans(context.Background(), nil))