Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support more environment variables in configuration #323

Merged
merged 3 commits into from Jul 13, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -51,6 +51,9 @@ Property| Description
JAEGER_SERVICE_NAME | The service name
JAEGER_AGENT_HOST | The hostname for communicating with agent via UDP
JAEGER_AGENT_PORT | The port for communicating with agent via UDP
JAEGER_ENDPOINT | The HTTP endpoint for sending spans directly to a collector, i.e. http://jaeger-collector:14268/api/traces
JAEGER_USER | Username to send as part of "Basic" authentication to the collector endpoint
JAEGER_PASSWORD | Password to send as part of "Basic" authentication to the collector endpoint
JAEGER_REPORTER_LOG_SPANS | Whether the reporter should also log the spans
JAEGER_REPORTER_MAX_QUEUE_SIZE | The reporter's maximum queue size
JAEGER_REPORTER_FLUSH_INTERVAL | The reporter's flush interval (ms)
@@ -63,6 +66,12 @@ JAEGER_TAGS | A comma separated list of `name = value` tracer level tags, which
JAEGER_DISABLED | Whether the tracer is disabled or not. If true, the default `opentracing.NoopTracer` is used.
JAEGER_RPC_METRICS | Whether to store RPC metrics

By default, the client sends traces via UDP to the agent at `localhost:6831`. Use `JAEGER_AGENT_HOST` and
`JAEGER_AGENT_PORT` to send UDP traces to a different `host:port`. If `JAEGER_ENDPOINT` is set, the client sends traces
to the endpoint via `HTTP`, making the `JAEGER_AGENT_HOST` and `JAEGER_AGENT_PORT` unused. If `JAEGER_ENDPOINT` is
secured, HTTP basic authentication can be performed by setting the `JAEGER_USER` and `JAEGER_PASSWORD` environment
variables.

### Closing the tracer via `io.Closer`

The constructor function for Jaeger Tracer returns the tracer itself and an `io.Closer` instance.
@@ -27,6 +27,7 @@ import (
"github.com/uber/jaeger-client-go/internal/baggage/remote"
throttler "github.com/uber/jaeger-client-go/internal/throttler/remote"
"github.com/uber/jaeger-client-go/rpcmetrics"
"github.com/uber/jaeger-client-go/transport"
)

const defaultSamplingProbability = 0.001
@@ -108,6 +109,18 @@ type ReporterConfig struct {
// LocalAgentHostPort instructs reporter to send spans to jaeger-agent at this address
// Can be set by exporting an environment variable named JAEGER_AGENT_HOST / JAEGER_AGENT_PORT
LocalAgentHostPort string `yaml:"localAgentHostPort"`

// CollectorEndpoint instructs reporter to send spans to jaeger-collector at this URL
// Can be set by exporting an environment variable named JAEGER_ENDPOINT
CollectorEndpoint string `yaml:"collectorEndpoint"`

// User instructs reporter to include a user for basic http authentication when sending spans to jaeger-collector.
// Can be set by exporting an environment variable named JAEGER_USER
User string `yaml:"user"`

// Password instructs reporter to include a password for basic http authentication when sending spans to
// jaeger-collector. Can be set by exporting an environment variable named JAEGER_PASSWORD
Password string `yaml:"password"`
}

// BaggageRestrictionsConfig configures the baggage restrictions manager which can be used to whitelist
@@ -345,7 +358,7 @@ func (sc *SamplerConfig) NewSampler(
return nil, fmt.Errorf("Unknown sampler type %v", sc.Type)
}

// NewReporter instantiates a new reporter that submits spans to tcollector
// NewReporter instantiates a new reporter that submits spans to the collector
func (rc *ReporterConfig) NewReporter(
serviceName string,
metrics *jaeger.Metrics,
@@ -369,5 +382,13 @@ func (rc *ReporterConfig) NewReporter(
}

func (rc *ReporterConfig) newTransport() (jaeger.Transport, error) {
return jaeger.NewUDPTransport(rc.LocalAgentHostPort, 0)
switch {
case rc.CollectorEndpoint != "" && rc.User != "" && rc.Password != "":
return transport.NewHTTPTransport(rc.CollectorEndpoint, transport.HTTPBatchSize(1),
transport.HTTPBasicAuth(rc.User, rc.Password)), nil
case rc.CollectorEndpoint != "":
return transport.NewHTTPTransport(rc.CollectorEndpoint, transport.HTTPBatchSize(1)), nil
default:
return jaeger.NewUDPTransport(rc.LocalAgentHostPort, 0)
}
}
@@ -16,6 +16,7 @@ package config

import (
"fmt"
"net/url"
"os"
"strconv"
"strings"
@@ -41,6 +42,9 @@ const (
envReporterMaxQueueSize = "JAEGER_REPORTER_MAX_QUEUE_SIZE"
envReporterFlushInterval = "JAEGER_REPORTER_FLUSH_INTERVAL"
envReporterLogSpans = "JAEGER_REPORTER_LOG_SPANS"
envEndpoint = "JAEGER_ENDPOINT"
envUser = "JAEGER_USER"
envPassword = "JAEGER_PASSWORD"
envAgentHost = "JAEGER_AGENT_HOST"
envAgentPort = "JAEGER_AGENT_PORT"
)
@@ -157,11 +161,17 @@ func reporterConfigFromEnv() (*ReporterConfig, error) {

host := jaeger.DefaultUDPSpanServerHost
if e := os.Getenv(envAgentHost); e != "" {
if ep := os.Getenv(envEndpoint); ep != "" {

This comment has been minimized.

Copy link
@black-adder

black-adder Jul 13, 2018

Contributor

how about we read the envEndpoint once and use ep in all the checks?

This comment has been minimized.

Copy link
@danehans

danehans Jul 13, 2018

Author Contributor

ack

return nil, errors.Errorf("cannot set env vars %s and %s together", envAgentHost, envEndpoint)
}
host = e
}

port := jaeger.DefaultUDPSpanServerPort
if e := os.Getenv(envAgentPort); e != "" {
if ep := os.Getenv(envEndpoint); ep != "" {
return nil, errors.Errorf("cannot set env vars %s and %s together", envAgentPort, envEndpoint)
}
if value, err := strconv.ParseInt(e, 10, 0); err == nil {
port = int(value)
} else {
@@ -173,6 +183,28 @@ func reporterConfigFromEnv() (*ReporterConfig, error) {
// were not explicitly passed
rc.LocalAgentHostPort = fmt.Sprintf("%s:%d", host, port)

if e := os.Getenv(envEndpoint); e != "" {
u, err := url.ParseRequestURI(e)
if err != nil {
return nil, errors.Wrapf(err, "cannot parse env var %s=%s", envEndpoint, e)
}
rc.CollectorEndpoint = fmt.Sprintf("%s", u)
}

if e := os.Getenv(envUser); e != "" {

This comment has been minimized.

Copy link
@black-adder

black-adder Jul 13, 2018

Contributor

can we use xor here instead and combine everything into 1 check? Albeit, it might not be a worthwhile optimization given this only happens during initialization but it's always fun to use xor every now and again.

if p := os.Getenv(envPassword); p == "" {
return nil, errors.Errorf("you must set env var %s when using %s", envPassword, envUser)
}
rc.User = e
}

if e := os.Getenv(envPassword); e != "" {
if u := os.Getenv(envUser); u == "" {
return nil, errors.Errorf("you must set env var %s when using %s", envUser, envPassword)
}
rc.Password = e
}

return rc, nil
}

@@ -29,6 +29,7 @@ import (

"github.com/uber/jaeger-client-go"
"github.com/uber/jaeger-client-go/log"
"github.com/uber/jaeger-client-go/transport"
)

func TestNewSamplerConst(t *testing.T) {
@@ -147,6 +148,8 @@ func TestReporterConfigFromEnv(t *testing.T) {
os.Setenv(envReporterLogSpans, "true")
os.Setenv(envAgentHost, "nonlocalhost")
os.Setenv(envAgentPort, "6832")
os.Setenv(envUser, "user")
os.Setenv(envPassword, "password")

// test
cfg, err := FromEnv()
@@ -158,12 +161,25 @@ func TestReporterConfigFromEnv(t *testing.T) {
assert.Equal(t, true, cfg.Reporter.LogSpans)
assert.Equal(t, "nonlocalhost:6832", cfg.Reporter.LocalAgentHostPort)

// Test HTTP transport
os.Unsetenv(envAgentHost)
os.Unsetenv(envAgentPort)
os.Setenv(envEndpoint, "http://1.2.3.4:5678/api/traces")

// test
cfg, err = FromEnv()
assert.NoError(t, err)

// verify
assert.Equal(t, "http://1.2.3.4:5678/api/traces", cfg.Reporter.CollectorEndpoint)

// cleanup
os.Unsetenv(envReporterMaxQueueSize)
os.Unsetenv(envReporterFlushInterval)
os.Unsetenv(envReporterLogSpans)
os.Unsetenv(envAgentHost)
os.Unsetenv(envAgentPort)
os.Unsetenv(envEndpoint)
os.Unsetenv(envUser)
os.Unsetenv(envPassword)
}

func TestParsingErrorsFromEnv(t *testing.T) {
@@ -209,10 +225,18 @@ func TestParsingErrorsFromEnv(t *testing.T) {
envVar: envAgentPort,
value: "NOT_AN_INT",
},
{
envVar: envEndpoint,
value: "NOT_A_URL",
},
}

for _, test := range tests {
os.Setenv(test.envVar, test.value)
if test.envVar == envEndpoint {
os.Unsetenv(envAgentHost)
os.Unsetenv(envAgentPort)
}
_, err := FromEnv()
require.Error(t, err)
assert.Contains(t, err.Error(), fmt.Sprintf("cannot parse env var %s=%s", test.envVar, test.value))
@@ -221,6 +245,62 @@ func TestParsingErrorsFromEnv(t *testing.T) {

}

func TestParsingUserPasswordErrorEnv(t *testing.T) {
tests := []struct {
envVar string
value string
err string
}{
{
envVar: envUser,
value: "user",
err: fmt.Sprintf("you must set env var %s when using %s", envPassword, envUser),
},
{
envVar: envPassword,
value: "password",
err: fmt.Sprintf("you must set env var %s when using %s", envUser, envPassword),
},
}

for _, test := range tests {
os.Setenv(test.envVar, test.value)
_, err := FromEnv()
require.Error(t, err)
assert.Contains(t, err.Error(), test.err)
os.Unsetenv(test.envVar)
}
}

func TestHostPortEndpointEnvError(t *testing.T) {
tests := []struct {
envVar string
value string
err string
}{
{
envVar: envAgentHost,
value: "user",
err: fmt.Sprintf("cannot set env vars %s and %s together", envAgentHost, envEndpoint),
},
{
envVar: envAgentPort,
value: "password",
err: fmt.Sprintf("cannot set env vars %s and %s together", envAgentPort, envEndpoint),
},
}

os.Setenv(envEndpoint, "http://1.2.3.4:5678/api/traces")
for _, test := range tests {
os.Setenv(test.envVar, test.value)
_, err := FromEnv()
require.Error(t, err)
assert.Contains(t, err.Error(), test.err)
os.Unsetenv(test.envVar)
}
os.Unsetenv(envEndpoint)
}

func TestInvalidSamplerType(t *testing.T) {
cfg := &SamplerConfig{MaxOperations: 10}
s, err := cfg.NewSampler("x", jaeger.NewNullMetrics())
@@ -230,6 +310,22 @@ func TestInvalidSamplerType(t *testing.T) {
rcs.Close()
}

func TestUDPTransportType(t *testing.T) {
rc := &ReporterConfig{LocalAgentHostPort: "localhost:1234"}
expect, _ := jaeger.NewUDPTransport(rc.LocalAgentHostPort, 0)
sender, err := rc.newTransport()
require.NoError(t, err)
require.IsType(t, expect, sender)
}

func TestHTTPTransportType(t *testing.T) {
rc := &ReporterConfig{CollectorEndpoint: "http://1.2.3.4:5678/api/traces"}
expect := transport.NewHTTPTransport(rc.CollectorEndpoint)
sender, err := rc.newTransport()
require.NoError(t, err)
require.IsType(t, expect, sender)
}

func TestDefaultConfig(t *testing.T) {
cfg := Configuration{}
_, _, err := cfg.New("", Metrics(metrics.NullFactory), Logger(log.NullLogger))
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.