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

Converge on JAEGER_SAMPLING_ENDPOINT env variable #511

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -60,7 +60,8 @@ JAEGER_REPORTER_MAX_QUEUE_SIZE | The reporter's maximum queue size
JAEGER_REPORTER_FLUSH_INTERVAL | The reporter's flush interval, with units, e.g. "500ms" or "2s" ([valid units][timeunits])
JAEGER_SAMPLER_TYPE | The sampler type
JAEGER_SAMPLER_PARAM | The sampler parameter (number)
JAEGER_SAMPLER_MANAGER_HOST_PORT | The HTTP endpoint when using the remote sampler, i.e. http://jaeger-agent:5778/sampling
JAEGER_SAMPLER_MANAGER_HOST_PORT | (Deprecated) The HTTP endpoint when using the remote sampler
JAEGER_SAMPLING_ENDPOINT | The url for the remote sampling conf when using sampler type remote. Default is http://127.0.0.1:5778/sampling
JAEGER_SAMPLER_MAX_OPERATIONS | The maximum number of operations that the sampler will keep track of
JAEGER_SAMPLER_REFRESH_INTERVAL | How often the remotely controlled sampler will poll jaeger-agent for the appropriate sampling strategy, with units, e.g. "1m" or "30s" ([valid units][timeunits])
JAEGER_TAGS | 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:default}`, where the `:default` is optional, and identifies a value to be used if the environment variable cannot be found
Expand Down
7 changes: 4 additions & 3 deletions config/config.go
Expand Up @@ -72,12 +72,13 @@ type SamplerConfig struct {
// Can be set by exporting an environment variable named JAEGER_SAMPLER_PARAM
Param float64 `yaml:"param"`

// SamplingServerURL is the address of jaeger-agent's HTTP sampling server
// Can be set by exporting an environment variable named JAEGER_SAMPLER_MANAGER_HOST_PORT
// SamplingServerURL is the URL of sampling manager that can provide
// sampling strategy to this service.
// Can be set by exporting an environment variable named JAEGER_SAMPLING_ENDPOINT
SamplingServerURL string `yaml:"samplingServerURL"`

// SamplingRefreshInterval controls how often the remotely controlled sampler will poll
// jaeger-agent for the appropriate sampling strategy.
// sampling manager for the appropriate sampling strategy.
// Can be set by exporting an environment variable named JAEGER_SAMPLER_REFRESH_INTERVAL
SamplingRefreshInterval time.Duration `yaml:"samplingRefreshInterval"`

Expand Down
7 changes: 5 additions & 2 deletions config/config_env.go
Expand Up @@ -36,7 +36,8 @@ const (
envTags = "JAEGER_TAGS"
envSamplerType = "JAEGER_SAMPLER_TYPE"
envSamplerParam = "JAEGER_SAMPLER_PARAM"
envSamplerManagerHostPort = "JAEGER_SAMPLER_MANAGER_HOST_PORT"
envSamplerManagerHostPort = "JAEGER_SAMPLER_MANAGER_HOST_PORT" // Deprecated by envSamplingEndpoint
envSamplingEndpoint = "JAEGER_SAMPLING_ENDPOINT"
envSamplerMaxOperations = "JAEGER_SAMPLER_MAX_OPERATIONS"
envSamplerRefreshInterval = "JAEGER_SAMPLER_REFRESH_INTERVAL"
envReporterMaxQueueSize = "JAEGER_REPORTER_MAX_QUEUE_SIZE"
Expand Down Expand Up @@ -118,7 +119,9 @@ func (sc *SamplerConfig) samplerConfigFromEnv() (*SamplerConfig, error) {
}
}

if e := os.Getenv(envSamplerManagerHostPort); e != "" {
if e := os.Getenv(envSamplingEndpoint); e != "" {
sc.SamplingServerURL = e
} else if e := os.Getenv(envSamplerManagerHostPort); e != "" {
sc.SamplingServerURL = e
} else if e := os.Getenv(envAgentHost); e != "" {
// Fallback if we know the agent host - try the sampling endpoint there
Expand Down
31 changes: 23 additions & 8 deletions config/config_test.go
Expand Up @@ -147,7 +147,7 @@ func TestSamplerConfig(t *testing.T) {
// prepare
setEnv(t, envSamplerType, "const")
setEnv(t, envSamplerParam, "1")
setEnv(t, envSamplerManagerHostPort, "http://themaster")
setEnv(t, envSamplingEndpoint, "http://themaster:5778/sampling")
setEnv(t, envSamplerMaxOperations, "10")
setEnv(t, envSamplerRefreshInterval, "1m1s") // 61 seconds

Expand All @@ -167,14 +167,14 @@ func TestSamplerConfig(t *testing.T) {
// verify
assert.Equal(t, "const", cfg.Type)
assert.Equal(t, float64(1), cfg.Param)
assert.Equal(t, "http://themaster", cfg.SamplingServerURL)
assert.Equal(t, "http://themaster:5778/sampling", cfg.SamplingServerURL)
assert.Equal(t, 10, cfg.MaxOperations)
assert.Equal(t, 61000000000, int(cfg.SamplingRefreshInterval))

// cleanup
unsetEnv(t, envSamplerType)
unsetEnv(t, envSamplerParam)
unsetEnv(t, envSamplerManagerHostPort)
unsetEnv(t, envSamplingEndpoint)
unsetEnv(t, envSamplerMaxOperations)
unsetEnv(t, envSamplerRefreshInterval)
}
Expand Down Expand Up @@ -301,9 +301,9 @@ func TestNoServiceNameFromEnv(t *testing.T) {

func TestSamplerConfigFromEnv(t *testing.T) {
// prepare
setEnv(t, envSamplerType, "const")
setEnv(t, envSamplerType, "remote")
setEnv(t, envSamplerParam, "1")
setEnv(t, envSamplerManagerHostPort, "http://themaster")
setEnv(t, envSamplingEndpoint, "http://themaster:5778/sampling")
setEnv(t, envSamplerMaxOperations, "10")
setEnv(t, envSamplerRefreshInterval, "1m1s") // 61 seconds

Expand All @@ -312,20 +312,35 @@ func TestSamplerConfigFromEnv(t *testing.T) {
assert.NoError(t, err)

// verify
assert.Equal(t, "const", cfg.Sampler.Type)
assert.Equal(t, "remote", cfg.Sampler.Type)
assert.Equal(t, float64(1), cfg.Sampler.Param)
assert.Equal(t, "http://themaster", cfg.Sampler.SamplingServerURL)
assert.Equal(t, "http://themaster:5778/sampling", cfg.Sampler.SamplingServerURL)
assert.Equal(t, 10, cfg.Sampler.MaxOperations)
assert.Equal(t, 61000000000, int(cfg.Sampler.SamplingRefreshInterval))

// cleanup
unsetEnv(t, envSamplerType)
unsetEnv(t, envSamplerParam)
unsetEnv(t, envSamplerManagerHostPort)
unsetEnv(t, envSamplingEndpoint)
unsetEnv(t, envSamplerMaxOperations)
unsetEnv(t, envSamplerRefreshInterval)
}

func TestDeprecatedSamplerConfigFromEnv(t *testing.T) {
// prepare
setEnv(t, envSamplerManagerHostPort, "http://themaster")

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

// verify
assert.Equal(t, "http://themaster", cfg.Sampler.SamplingServerURL)

// cleanup
unsetEnv(t, envSamplerManagerHostPort)
}

func TestSamplerConfigOnAgentFromEnv(t *testing.T) {
// prepare
setEnv(t, envAgentHost, "theagent")
Expand Down
2 changes: 1 addition & 1 deletion constants.go
Expand Up @@ -102,5 +102,5 @@ const (

var (
// DefaultSamplingServerURL is the default url to fetch sampling config from, via http
DefaultSamplingServerURL = fmt.Sprintf("http://localhost:%d/sampling", DefaultSamplingServerPort)
DefaultSamplingServerURL = fmt.Sprintf("http://127.0.0.1:%d/sampling", DefaultSamplingServerPort)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a big reason.
I referred to csharp change, 127.0.0.1 was used in both readme.md and code.

JAEGER_SAMPLING_ENDPOINT | The url for the remote sampling conf when 
using sampler type remote. Default is http://127.0.0.1:5778/sampling

If you want, we can stay current code since localhost is 127.0.0.1 (except /etc/hosts lookup.)

)