Update Jaeger environment variables (#1752)

* Update Jaeger Environment Variables

* Update CHANGELOG

* Add Jaeger environment vars envAgentHost, envAgentPort; remove envDisabled

* Fix broken test due to setting nonexistant env var

* fix function name

* add default values for agent hostPort

* remove agentEndpoint arg

* add agent host/port options

* update client params

* add envOr function

CHANGELOG.md

@@ -10,6 +10,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Added
 
+- Added Jaeger Environment variables: `OTEL_EXPORTER_JAEGER_AGENT_HOST`, `OTEL_EXPORTER_JAEGER_AGENT_PORT`
+  These environment variables can be used to override Jaeger agent hostname and port (#1752)
 - The OTLP exporter now has two new convenience functions, `NewExportPipeline` and `InstallNewPipeline`, setup and install the exporter in tracing and metrics pipelines. (#1373)
 - Adds test to check BatchSpanProcessor ignores `OnEnd` and `ForceFlush` post `Shutdown`. (#1772)
 - Option `ExportTimeout` was added to batch span processor. (#1755)
@@ -46,6 +48,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
+- Updated Jaeger Environment Variables: `JAEGER_ENDPOINT`, `JAEGER_USER`, `JAEGER_PASSWORD`
+  to `OTEL_EXPORTER_JAEGER_ENDPOINT`, `OTEL_EXPORTER_JAEGER_USER`, `OTEL_EXPORTER_JAEGER_PASSWORD` 
+  in compliance with OTel spec (#1752)
 - Span `RecordError` now records an `exception` event to comply with the semantic convention specification. (#1492)
 - Jaeger exporter was updated to use thrift v0.14.1. (#1712)
 - Migrate from using internally built and maintained version of the OTLP to the one hosted at `go.opentelemetry.io/proto/otlp`. (#1713)
@@ -66,6 +71,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Removed
 
+- Removed Jaeger Environment variables: `JAEGER_SERVICE_NAME`, `JAEGER_DISABLED`, `JAEGER_TAGS`
+  These environment variables will no longer be used to override values of the Jaeger exporter (#1752)
 - No longer set the links for a `Span` in `go.opentelemetry.io/otel/sdk/trace` that is configured to be a new root.
   This is unspecified behavior that the OpenTelemetry community plans to standardize in the future.
   To prevent backwards incompatible changes when it is specified, these links are removed. (#1726)

exporters/trace/jaeger/agent.go

@@ -49,7 +49,8 @@ type udpConn interface {
 }
 
 type agentClientUDPParams struct {
-	HostPort                 string
+	Host                     string
+	Port                     string
 	MaxPacketSize            int
 	Logger                   *log.Logger
 	AttemptReconnecting      bool
@@ -58,8 +59,9 @@ type agentClientUDPParams struct {
 
 // newAgentClientUDP creates a client that sends spans to Jaeger Agent over UDP.
 func newAgentClientUDP(params agentClientUDPParams) (*agentClientUDP, error) {
+	hostPort := net.JoinHostPort(params.Host, params.Port)
 	// validate hostport
-	if _, _, err := net.SplitHostPort(params.HostPort); err != nil {
+	if _, _, err := net.SplitHostPort(hostPort); err != nil {
 		return nil, err
 	}
 
@@ -80,12 +82,12 @@ func newAgentClientUDP(params agentClientUDPParams) (*agentClientUDP, error) {
 
 	if params.AttemptReconnecting {
 		// host is hostname, setup resolver loop in case host record changes during operation
-		connUDP, err = newReconnectingUDPConn(params.HostPort, params.MaxPacketSize, params.AttemptReconnectInterval, net.ResolveUDPAddr, net.DialUDP, params.Logger)
+		connUDP, err = newReconnectingUDPConn(hostPort, params.MaxPacketSize, params.AttemptReconnectInterval, net.ResolveUDPAddr, net.DialUDP, params.Logger)
 		if err != nil {
 			return nil, err
 		}
 	} else {
-		destAddr, err := net.ResolveUDPAddr("udp", params.HostPort)
+		destAddr, err := net.ResolveUDPAddr("udp", hostPort)
 		if err != nil {
 			return nil, err
 		}

exporters/trace/jaeger/agent_test.go

@@ -23,12 +23,10 @@ import (
 )
 
 func TestNewAgentClientUDPWithParamsBadHostport(t *testing.T) {
-	hostPort := "blahblah"
-
 	agentClient, err := newAgentClientUDP(agentClientUDPParams{
-		HostPort: hostPort,
+		Host: "blahblah",
+		Port: "",
 	})
-
 	assert.Error(t, err)
 	assert.Nil(t, agentClient)
 }
@@ -37,9 +35,12 @@ func TestNewAgentClientUDPWithParams(t *testing.T) {
 	mockServer, err := newUDPListener()
 	require.NoError(t, err)
 	defer mockServer.Close()
+	host, port, err := net.SplitHostPort(mockServer.LocalAddr().String())
+	assert.NoError(t, err)
 
 	agentClient, err := newAgentClientUDP(agentClientUDPParams{
-		HostPort:            mockServer.LocalAddr().String(),
+		Host:                host,
+		Port:                port,
 		MaxPacketSize:       25000,
 		AttemptReconnecting: true,
 	})
@@ -58,9 +59,12 @@ func TestNewAgentClientUDPWithParamsDefaults(t *testing.T) {
 	mockServer, err := newUDPListener()
 	require.NoError(t, err)
 	defer mockServer.Close()
+	host, port, err := net.SplitHostPort(mockServer.LocalAddr().String())
+	assert.NoError(t, err)
 
 	agentClient, err := newAgentClientUDP(agentClientUDPParams{
-		HostPort:            mockServer.LocalAddr().String(),
+		Host:                host,
+		Port:                port,
 		AttemptReconnecting: true,
 	})
 	assert.NoError(t, err)
@@ -78,9 +82,12 @@ func TestNewAgentClientUDPWithParamsReconnectingDisabled(t *testing.T) {
 	mockServer, err := newUDPListener()
 	require.NoError(t, err)
 	defer mockServer.Close()
+	host, port, err := net.SplitHostPort(mockServer.LocalAddr().String())
+	assert.NoError(t, err)
 
 	agentClient, err := newAgentClientUDP(agentClientUDPParams{
-		HostPort:            mockServer.LocalAddr().String(),
+		Host:                host,
+		Port:                port,
 		Logger:              nil,
 		AttemptReconnecting: false,
 	})

exporters/trace/jaeger/env.go

@@ -15,33 +15,35 @@
 package jaeger // import "go.opentelemetry.io/otel/exporters/trace/jaeger"
 
 import (
-	"errors"
 	"os"
-	"strconv"
-	"strings"
-
-	"go.opentelemetry.io/otel/attribute"
 )
 
 // Environment variable names
 const (
-	// The service name.
-	envServiceName = "JAEGER_SERVICE_NAME"
-	// Whether the exporter is disabled or not. (default false).
-	envDisabled = "JAEGER_DISABLED"
-	// A comma separated list of name=value tracer-level tags, which get added to all reported spans.
-	// The value can also refer to an environment variable using the format ${envVarName:defaultValue}.
-	envTags = "JAEGER_TAGS"
+	// Hostname for the Jaeger agent, part of address where exporter sends spans
+	// i.e.	"localhost"
+	envAgentHost = "OTEL_EXPORTER_JAEGER_AGENT_HOST"
+	// Port for the Jaeger agent, part of address where exporter sends spans
+	// i.e. 6832
+	envAgentPort = "OTEL_EXPORTER_JAEGER_AGENT_PORT"
 	// The HTTP endpoint for sending spans directly to a collector,
 	// i.e. http://jaeger-collector:14268/api/traces.
-	envEndpoint = "JAEGER_ENDPOINT"
+	envEndpoint = "OTEL_EXPORTER_JAEGER_ENDPOINT"
 	// Username to send as part of "Basic" authentication to the collector endpoint.
-	envUser = "JAEGER_USER"
+	envUser = "OTEL_EXPORTER_JAEGER_USER"
 	// Password to send as part of "Basic" authentication to the collector endpoint.
-	envPassword = "JAEGER_PASSWORD"
+	envPassword = "OTEL_EXPORTER_JAEGER_PASSWORD"
 )
 
-// CollectorEndpointFromEnv return environment variable value of JAEGER_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
+}
+
+// CollectorEndpointFromEnv return environment variable value of OTEL_EXPORTER_JAEGER_ENDPOINT
 func CollectorEndpointFromEnv() string {
 	return os.Getenv(envEndpoint)
 }
@@ -54,76 +56,7 @@ func WithCollectorEndpointOptionFromEnv() CollectorEndpointOption {
 			o.username = e
 		}
 		if e := os.Getenv(envPassword); e != "" {
-			o.password = os.Getenv(envPassword)
-		}
-	}
-}
-
-// WithDisabledFromEnv uses environment variables and overrides disabled field.
-func WithDisabledFromEnv() Option {
-	return func(o *options) {
-		if e := os.Getenv(envDisabled); e != "" {
-			if v, err := strconv.ParseBool(e); err == nil {
-				o.Disabled = v
-			}
-		}
-	}
-}
-
-var errTagValueNotFound = errors.New("missing tag value")
-var errTagEnvironmentDefaultValueNotFound = errors.New("missing default value for tag environment value")
-
-// parseTags parses the given string into a collection of Tags.
-// Spec for this value:
-// - comma separated list of key=value
-// - value can be specified using the notation ${envVar:defaultValue}, where `envVar`
-// is an environment variable and `defaultValue` is the value to use in case the env var is not set
-func parseTags(sTags string) ([]attribute.KeyValue, error) {
-	pairs := strings.Split(sTags, ",")
-	tags := make([]attribute.KeyValue, len(pairs))
-	for i, p := range pairs {
-		field := strings.SplitN(p, "=", 2)
-		if len(field) != 2 {
-			return nil, errTagValueNotFound
+			o.password = e
 		}
-		k, v := strings.TrimSpace(field[0]), strings.TrimSpace(field[1])
-
-		if strings.HasPrefix(v, "${") && strings.HasSuffix(v, "}") {
-			ed := strings.SplitN(v[2:len(v)-1], ":", 2)
-			if len(ed) != 2 {
-				return nil, errTagEnvironmentDefaultValueNotFound
-			}
-			e, d := ed[0], ed[1]
-			v = os.Getenv(e)
-			if v == "" && d != "" {
-				v = d
-			}
-		}
-
-		tags[i] = parseKeyValue(k, v)
-	}
-
-	return tags, nil
-}
-
-func parseKeyValue(k, v string) attribute.KeyValue {
-	return attribute.KeyValue{
-		Key:   attribute.Key(k),
-		Value: parseValue(v),
 	}
 }
-
-func parseValue(str string) attribute.Value {
-	if v, err := strconv.ParseInt(str, 10, 64); err == nil {
-		return attribute.Int64Value(v)
-	}
-	if v, err := strconv.ParseFloat(str, 64); err == nil {
-		return attribute.Float64Value(v)
-	}
-	if v, err := strconv.ParseBool(str); err == nil {
-		return attribute.BoolValue(v)
-	}
-
-	// Fallback
-	return attribute.StringValue(str)
-}