From c5d006c07ab78e6b21690e86c859e5f6752d0086 Mon Sep 17 00:00:00 2001 From: Sai Nadendla Date: Fri, 9 Apr 2021 09:56:26 -0700 Subject: [PATCH] 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 | 7 + exporters/trace/jaeger/agent.go | 10 +- exporters/trace/jaeger/agent_test.go | 21 +- exporters/trace/jaeger/env.go | 105 ++------- exporters/trace/jaeger/env_test.go | 311 ++++++-------------------- exporters/trace/jaeger/jaeger.go | 1 - exporters/trace/jaeger/jaeger_test.go | 51 +---- exporters/trace/jaeger/uploader.go | 40 +++- 8 files changed, 152 insertions(+), 394 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f0962f4c..583bcbc79 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/exporters/trace/jaeger/agent.go b/exporters/trace/jaeger/agent.go index 626e53f27..76b2aff51 100644 --- a/exporters/trace/jaeger/agent.go +++ b/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 } diff --git a/exporters/trace/jaeger/agent_test.go b/exporters/trace/jaeger/agent_test.go index f449f07e9..2b787d97a 100644 --- a/exporters/trace/jaeger/agent_test.go +++ b/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, }) diff --git a/exporters/trace/jaeger/env.go b/exporters/trace/jaeger/env.go index 52fd4168f..92d4a8174 100644 --- a/exporters/trace/jaeger/env.go +++ b/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) + o.password = e } } } - -// 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 - } - 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) -} diff --git a/exporters/trace/jaeger/env_test.go b/exporters/trace/jaeger/env_test.go index 5a30d6abb..12c782834 100644 --- a/exporters/trace/jaeger/env_test.go +++ b/exporters/trace/jaeger/env_test.go @@ -15,209 +15,26 @@ package jaeger import ( - "math" "os" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel/attribute" ottest "go.opentelemetry.io/otel/internal/internaltest" ) -func Test_parseTags(t *testing.T) { - envStore, err := ottest.SetEnvVariables(map[string]string{ - "existing": "not-default", - }) - require.NoError(t, err) - defer func() { - require.NoError(t, envStore.Restore()) - }() - - testCases := []struct { - name string - tagStr string - expectedTags []attribute.KeyValue - expectedError error - }{ - { - name: "string", - tagStr: "key=value", - expectedTags: []attribute.KeyValue{ - { - Key: "key", - Value: attribute.StringValue("value"), - }, - }, - }, - { - name: "int64", - tagStr: "k=9223372036854775807,k2=-9223372036854775808", - expectedTags: []attribute.KeyValue{ - { - Key: "k", - Value: attribute.Int64Value(math.MaxInt64), - }, - { - Key: "k2", - Value: attribute.Int64Value(math.MinInt64), - }, - }, - }, - { - name: "float64", - tagStr: "k=1.797693134862315708145274237317043567981e+308,k2=4.940656458412465441765687928682213723651e-324,k3=-1.2", - expectedTags: []attribute.KeyValue{ - { - Key: "k", - Value: attribute.Float64Value(math.MaxFloat64), - }, - { - Key: "k2", - Value: attribute.Float64Value(math.SmallestNonzeroFloat64), - }, - { - Key: "k3", - Value: attribute.Float64Value(-1.2), - }, - }, - }, - { - name: "multiple type values", - tagStr: "k=v,k2=123, k3=v3 ,k4=-1.2, k5=${existing:default},k6=${nonExisting:default}", - expectedTags: []attribute.KeyValue{ - { - Key: "k", - Value: attribute.StringValue("v"), - }, - { - Key: "k2", - Value: attribute.Int64Value(123), - }, - { - Key: "k3", - Value: attribute.StringValue("v3"), - }, - { - Key: "k4", - Value: attribute.Float64Value(-1.2), - }, - { - Key: "k5", - Value: attribute.StringValue("not-default"), - }, - { - Key: "k6", - Value: attribute.StringValue("default"), - }, - }, - }, - { - name: "malformed: only have key", - tagStr: "key", - expectedError: errTagValueNotFound, - }, - { - name: "malformed: environment key has no default value", - tagStr: "key=${foo}", - expectedError: errTagEnvironmentDefaultValueNotFound, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - tags, err := parseTags(tc.tagStr) - if tc.expectedError == nil { - assert.NoError(t, err) - assert.Equal(t, tc.expectedTags, tags) - } else { - assert.Error(t, err) - assert.Equal(t, tc.expectedError, err) - assert.Equal(t, tc.expectedTags, tags) - } - }) - } -} - -func Test_parseValue(t *testing.T) { - testCases := []struct { - name string - str string - expected attribute.Value - }{ - { - name: "bool: true", - str: "true", - expected: attribute.BoolValue(true), - }, - { - name: "bool: false", - str: "false", - expected: attribute.BoolValue(false), - }, - { - name: "int64: 012340", - str: "012340", - expected: attribute.Int64Value(12340), - }, - { - name: "int64: -012340", - str: "-012340", - expected: attribute.Int64Value(-12340), - }, - { - name: "int64: 0", - str: "0", - expected: attribute.Int64Value(0), - }, - { - name: "float64: -0.1", - str: "-0.1", - expected: attribute.Float64Value(-0.1), - }, - { - name: "float64: 00.001", - str: "00.001", - expected: attribute.Float64Value(0.001), - }, - { - name: "float64: 1E23", - str: "1E23", - expected: attribute.Float64Value(1e23), - }, - { - name: "string: foo", - str: "foo", - expected: attribute.StringValue("foo"), - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - v := parseValue(tc.str) - assert.Equal(t, tc.expected, v) - }) - } -} - func TestNewRawExporterWithEnv(t *testing.T) { const ( collectorEndpoint = "http://localhost" username = "user" password = "password" - serviceName = "test-service" - disabled = "false" - tags = "key=value" ) envStore, err := ottest.SetEnvVariables(map[string]string{ - envEndpoint: collectorEndpoint, - envUser: username, - envPassword: password, - envDisabled: disabled, - envServiceName: serviceName, - envTags: tags, + envEndpoint: collectorEndpoint, + envUser: username, + envPassword: password, }) require.NoError(t, err) defer func() { @@ -227,12 +44,9 @@ func TestNewRawExporterWithEnv(t *testing.T) { // Create Jaeger Exporter with environment variables exp, err := NewRawExporter( WithCollectorEndpoint(CollectorEndpointFromEnv(), WithCollectorEndpointOptionFromEnv()), - WithDisabled(true), - WithDisabledFromEnv(), ) assert.NoError(t, err) - assert.Equal(t, false, exp.o.Disabled) require.IsType(t, &collectorUploader{}, exp.uploader) uploader := exp.uploader.(*collectorUploader) @@ -246,18 +60,12 @@ func TestNewRawExporterWithEnvImplicitly(t *testing.T) { collectorEndpoint = "http://localhost" username = "user" password = "password" - serviceName = "test-service" - disabled = "false" - tags = "key=value" ) envStore, err := ottest.SetEnvVariables(map[string]string{ - envEndpoint: collectorEndpoint, - envUser: username, - envPassword: password, - envDisabled: disabled, - envServiceName: serviceName, - envTags: tags, + envEndpoint: collectorEndpoint, + envUser: username, + envPassword: password, }) require.NoError(t, err) defer func() { @@ -267,12 +75,9 @@ func TestNewRawExporterWithEnvImplicitly(t *testing.T) { // Create Jaeger Exporter with environment variables exp, err := NewRawExporter( WithCollectorEndpoint("should be overwritten"), - WithDisabled(true), ) assert.NoError(t, err) - // NewRawExporter will ignore Disabled env - assert.Equal(t, true, exp.o.Disabled) require.IsType(t, &collectorUploader{}, exp.uploader) uploader := exp.uploader.(*collectorUploader) @@ -281,6 +86,72 @@ func TestNewRawExporterWithEnvImplicitly(t *testing.T) { assert.Equal(t, password, uploader.password) } +func TestEnvOrWithAgentHostPortFromEnv(t *testing.T) { + testCases := []struct { + name string + envAgentHost string + envAgentPort string + defaultHost string + defaultPort string + expectedHost string + expectedPort string + }{ + { + name: "overrides default host/port values via environment variables", + envAgentHost: "localhost", + envAgentPort: "6832", + defaultHost: "hostNameToBeReplaced", + defaultPort: "8203", + expectedHost: "localhost", + expectedPort: "6832", + }, + { + name: "envAgentHost is empty, will not overwrite default host value", + envAgentHost: "", + envAgentPort: "6832", + defaultHost: "hostNameNotToBeReplaced", + defaultPort: "8203", + expectedHost: "hostNameNotToBeReplaced", + expectedPort: "6832", + }, + { + name: "envAgentPort is empty, will not overwrite default port value", + envAgentHost: "localhost", + envAgentPort: "", + defaultHost: "hostNameToBeReplaced", + defaultPort: "8203", + expectedHost: "localhost", + expectedPort: "8203", + }, + { + name: "envAgentHost and envAgentPort are empty, will not overwrite default host/port values", + envAgentHost: "", + envAgentPort: "", + defaultHost: "hostNameNotToBeReplaced", + defaultPort: "8203", + expectedHost: "hostNameNotToBeReplaced", + expectedPort: "8203", + }, + } + + envStore := ottest.NewEnvStore() + envStore.Record(envAgentHost) + envStore.Record(envAgentPort) + defer func() { + require.NoError(t, envStore.Restore()) + }() + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.NoError(t, os.Setenv(envAgentHost, tc.envAgentHost)) + require.NoError(t, os.Setenv(envAgentPort, tc.envAgentPort)) + host := envOr(envAgentHost, tc.defaultHost) + port := envOr(envAgentPort, tc.defaultPort) + assert.Equal(t, tc.expectedHost, host) + assert.Equal(t, tc.expectedPort, port) + }) + } +} + func TestCollectorEndpointFromEnv(t *testing.T) { const ( collectorEndpoint = "http://localhost" @@ -351,41 +222,3 @@ func TestWithCollectorEndpointOptionFromEnv(t *testing.T) { }) } } - -func TestWithDisabledFromEnv(t *testing.T) { - testCases := []struct { - name string - env string - options options - expectedOptions options - }{ - { - name: "overwriting", - env: "true", - options: options{}, - expectedOptions: options{Disabled: true}, - }, - { - name: "no overwriting", - env: "", - options: options{Disabled: true}, - expectedOptions: options{Disabled: true}, - }, - } - - envStore := ottest.NewEnvStore() - envStore.Record(envDisabled) - defer func() { - require.NoError(t, envStore.Restore()) - }() - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - require.NoError(t, os.Setenv(envDisabled, tc.env)) - - f := WithDisabledFromEnv() - f(&tc.options) - - assert.Equal(t, tc.expectedOptions, tc.options) - }) - } -} diff --git a/exporters/trace/jaeger/jaeger.go b/exporters/trace/jaeger/jaeger.go index 74c882669..b8481edb2 100644 --- a/exporters/trace/jaeger/jaeger.go +++ b/exporters/trace/jaeger/jaeger.go @@ -146,7 +146,6 @@ func NewRawExporter(endpointOption EndpointOption, opts ...Option) (*Exporter, e // with the recommended setup for trace provider func NewExportPipeline(endpointOption EndpointOption, opts ...Option) (trace.TracerProvider, func(), error) { o := options{} - opts = append(opts, WithDisabledFromEnv()) for _, opt := range opts { opt(&o) } diff --git a/exporters/trace/jaeger/jaeger_test.go b/exporters/trace/jaeger/jaeger_test.go index 169bd3fa3..d74e6cde6 100644 --- a/exporters/trace/jaeger/jaeger_test.go +++ b/exporters/trace/jaeger/jaeger_test.go @@ -43,7 +43,6 @@ import ( const ( collectorEndpoint = "http://localhost:14268/api/traces" - agentEndpoint = "localhost:6831" ) func TestInstallNewPipeline(t *testing.T) { @@ -60,7 +59,7 @@ func TestInstallNewPipeline(t *testing.T) { }, { name: "with agent endpoint", - endpoint: WithAgentEndpoint(agentEndpoint), + endpoint: WithAgentEndpoint(), expectedProvider: &sdktrace.TracerProvider{}, }, { @@ -154,24 +153,6 @@ func TestNewExportPipeline(t *testing.T) { } } -func TestNewExportPipelineWithDisabledFromEnv(t *testing.T) { - envStore, err := ottest.SetEnvVariables(map[string]string{ - envDisabled: "true", - }) - require.NoError(t, err) - envStore.Record(envDisabled) - defer func() { - require.NoError(t, envStore.Restore()) - }() - - tp, fn, err := NewExportPipeline( - WithCollectorEndpoint(collectorEndpoint), - ) - defer fn() - assert.NoError(t, err) - assert.IsType(t, trace.NewNoopTracerProvider(), tp) -} - func TestNewRawExporter(t *testing.T) { testCases := []struct { name string @@ -189,7 +170,7 @@ func TestNewRawExporter(t *testing.T) { }, { name: "default exporter with agent endpoint", - endpoint: WithAgentEndpoint(agentEndpoint), + endpoint: WithAgentEndpoint(), expectedServiceName: "unknown_service", expectedBufferMaxCount: bundler.DefaultBufferedByteLimit, expectedBatchMaxCount: bundler.DefaultBundleCountThreshold, @@ -234,16 +215,6 @@ func TestNewRawExporterShouldFail(t *testing.T) { endpoint: WithCollectorEndpoint(""), expectedErrMsg: "collectorEndpoint must not be empty", }, - { - name: "with empty agent endpoint", - endpoint: WithAgentEndpoint(""), - expectedErrMsg: "agentEndpoint must not be empty", - }, - { - name: "with invalid agent endpoint", - endpoint: WithAgentEndpoint("localhost"), - expectedErrMsg: "address localhost: missing port in address", - }, } for _, tc := range testCases { @@ -266,7 +237,7 @@ func TestNewRawExporterShouldFailIfCollectorUnset(t *testing.T) { require.NoError(t, envStore.Restore()) }() - // If the user sets the environment variable JAEGER_ENDPOINT, endpoint will always get a value. + // If the user sets the environment variable OTEL_EXPORTER_JAEGER_ENDPOINT, endpoint will always get a value. require.NoError(t, os.Unsetenv(envEndpoint)) _, err := NewRawExporter( @@ -300,14 +271,6 @@ func withTestCollectorEndpointInjected(ce *testCollectorEndpoint) func() (batchU } func TestExporter_ExportSpan(t *testing.T) { - envStore, err := ottest.SetEnvVariables(map[string]string{ - envDisabled: "false", - }) - require.NoError(t, err) - defer func() { - require.NoError(t, envStore.Restore()) - }() - const ( serviceName = "test-service" tagKey = "key" @@ -938,14 +901,6 @@ func TestProcess(t *testing.T) { } func TestNewExporterPipelineWithOptions(t *testing.T) { - envStore, err := ottest.SetEnvVariables(map[string]string{ - envDisabled: "false", - }) - require.NoError(t, err) - defer func() { - require.NoError(t, envStore.Restore()) - }() - const ( serviceName = "test-service" eventCountLimit = 10 diff --git a/exporters/trace/jaeger/uploader.go b/exporters/trace/jaeger/uploader.go index e87543a92..bfaa6ebc1 100644 --- a/exporters/trace/jaeger/uploader.go +++ b/exporters/trace/jaeger/uploader.go @@ -37,21 +37,23 @@ type batchUploader interface { type EndpointOption func() (batchUploader, error) -// WithAgentEndpoint instructs exporter to send spans to jaeger-agent at this address. -// For example, localhost:6831. -func WithAgentEndpoint(agentEndpoint string, options ...AgentEndpointOption) EndpointOption { +// WithAgentEndpoint configures the Jaeger exporter to send spans to a jaeger-agent. This will +// use the following environment variables for configuration if no explicit option is provided: +// +// - OTEL_EXPORTER_JAEGER_AGENT_HOST is used for the agent address host +// - OTEL_EXPORTER_JAEGER_AGENT_PORT is used for the agent address port +// +// The passed options will take precedence over any environment variables and default values +// will be used if neither are provided. +func WithAgentEndpoint(options ...AgentEndpointOption) EndpointOption { return func() (batchUploader, error) { - if agentEndpoint == "" { - return nil, errors.New("agentEndpoint must not be empty") - } - o := &AgentEndpointOptions{ agentClientUDPParams{ - HostPort: agentEndpoint, AttemptReconnecting: true, + Host: envOr(envAgentHost, "localhost"), + Port: envOr(envAgentPort, "6832"), }, } - for _, opt := range options { opt(o) } @@ -71,6 +73,26 @@ type AgentEndpointOptions struct { agentClientUDPParams } +// WithAgentHost sets a host to be used in the agent client endpoint. +// This option overrides any value set for the +// OTEL_EXPORTER_JAEGER_AGENT_HOST environment variable. +// If this option is not passed and the env var is not set, "localhost" will be used by default. +func WithAgentHost(host string) AgentEndpointOption { + return func(o *AgentEndpointOptions) { + o.Host = host + } +} + +// WithAgentPort sets a port to be used in the agent client endpoint. +// This option overrides any value set for the +// OTEL_EXPORTER_JAEGER_AGENT_PORT environment variable. +// If this option is not passed and the env var is not set, "6832" will be used by default. +func WithAgentPort(port string) AgentEndpointOption { + return func(o *AgentEndpointOptions) { + o.Port = port + } +} + // WithLogger sets a logger to be used by agent client. func WithLogger(logger *log.Logger) AgentEndpointOption { return func(o *AgentEndpointOptions) {