diff --git a/README.md b/README.md index 9347e054..18e83c01 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ The default sampling collector URL is `http://127.0.0.1:5778/sampling`. Similar ```yml sampler: - samplingServerURL: http://jaeger-agent.local:5778 + samplingServerURL: http://jaeger-agent.local:5778/sampling ``` ### Configuration via Environment @@ -103,8 +103,8 @@ 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) JAEGER_SAMPLER_TYPE | The [sampler type](https://www.jaegertracing.io/docs/latest/sampling/#client-sampling-configuration) -JAEGER_SAMPLER_PARAM | The sampler parameter (number) -JAEGER_SAMPLER_MANAGER_HOST_PORT | The host name and port when using the remote controlled sampler +JAEGER_SAMPLER_PARAM | The sampler parameter (double) +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_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 ## License diff --git a/src/jaegertracing/ConfigTest.cpp b/src/jaegertracing/ConfigTest.cpp index 5392fe9c..7236bdff 100644 --- a/src/jaegertracing/ConfigTest.cpp +++ b/src/jaegertracing/ConfigTest.cpp @@ -147,8 +147,9 @@ TEST(Config, testFromEnv) setEnv("JAEGER_REPORTER_FLUSH_INTERVAL", "45"); setEnv("JAEGER_REPORTER_LOG_SPANS", "true"); - setEnv("JAEGER_SAMPLER_PARAM", "33"); - setEnv("JAEGER_SAMPLER_TYPE", "const"); + setEnv("JAEGER_SAMPLER_TYPE", "remote"); + setEnv("JAEGER_SAMPLER_PARAM", "0.33"); + setEnv("JAEGER_SAMPLING_ENDPOINT", "http://myagent:1234"); setEnv("JAEGER_SERVICE_NAME", "AService"); setEnv("JAEGER_TAGS", "hostname=foobar,my.app.version=4.5.6"); @@ -163,8 +164,9 @@ TEST(Config, testFromEnv) config.reporter().bufferFlushInterval()); ASSERT_EQ(true, config.reporter().logSpans()); - ASSERT_EQ(33., config.sampler().param()); - ASSERT_EQ(std::string("const"), config.sampler().type()); + ASSERT_EQ(std::string("remote"), config.sampler().type()); + ASSERT_EQ(0.33, config.sampler().param()); + ASSERT_EQ(std::string("http://myagent:1234"), config.sampler().samplingServerURL()); ASSERT_EQ(std::string("AService"), config.serviceName()); diff --git a/src/jaegertracing/samplers/Config.cpp b/src/jaegertracing/samplers/Config.cpp index 33ea092a..75f1549b 100644 --- a/src/jaegertracing/samplers/Config.cpp +++ b/src/jaegertracing/samplers/Config.cpp @@ -36,11 +36,15 @@ void Config::fromEnv() const auto param = utils::EnvVariable::getStringVariable(kJAEGER_SAMPLER_PARAM_ENV_PROP); if (!param.empty()) { std::istringstream iss(param); - int paramVal = 0; + double paramVal = 0; if (iss >> paramVal) { _param = paramVal; } } + const auto samplingServerURL = utils::EnvVariable::getStringVariable(kJAEGER_SAMPLING_ENDPOINT_ENV_PROP); + if (!samplingServerURL.empty()) { + _samplingServerURL = samplingServerURL; + } } } // namespace samplers diff --git a/src/jaegertracing/samplers/Config.h b/src/jaegertracing/samplers/Config.h index 58df7f99..210feed9 100644 --- a/src/jaegertracing/samplers/Config.h +++ b/src/jaegertracing/samplers/Config.h @@ -47,7 +47,7 @@ class Config { static constexpr auto kJAEGER_SAMPLER_TYPE_ENV_PROP = "JAEGER_SAMPLER_TYPE"; static constexpr auto kJAEGER_SAMPLER_PARAM_ENV_PROP = "JAEGER_SAMPLER_PARAM"; - static constexpr auto kJAEGER_SAMPLER_MANAGER_HOST_PORT_ENV_PROP = "JAEGER_SAMPLER_MANAGER_HOST_PORT"; + static constexpr auto kJAEGER_SAMPLING_ENDPOINT_ENV_PROP = "JAEGER_SAMPLING_ENDPOINT"; static Clock::duration defaultSamplingRefreshInterval() { diff --git a/src/jaegertracing/samplers/SamplerTest.cpp b/src/jaegertracing/samplers/SamplerTest.cpp index 6fb40677..3c082d32 100644 --- a/src/jaegertracing/samplers/SamplerTest.cpp +++ b/src/jaegertracing/samplers/SamplerTest.cpp @@ -331,6 +331,17 @@ TEST(Sampler, testRemotelyControlledSampler) mockAgent->start(); const auto logger = logging::nullLogger(); const auto metrics = metrics::Metrics::makeNullMetrics(); + + // Make sure remote sampling probability is 1 + sampling_manager::thrift::SamplingStrategyResponse config; + config.__set_strategyType( + sampling_manager::thrift::SamplingStrategyType::PROBABILISTIC); + sampling_manager::thrift::ProbabilisticSamplingStrategy probaStrategy; + probaStrategy.__set_samplingRate(1.0); + config.__set_probabilisticSampling(probaStrategy); + mockAgent->addSamplingStrategy("test-service", config); + + // Default probability of 0.5, switches to 1 when downloaded RemotelyControlledSampler sampler( "test-service", "http://" + mockAgent->samplingServerAddress().authority(), @@ -339,6 +350,9 @@ TEST(Sampler, testRemotelyControlledSampler) std::chrono::milliseconds(100), *logger, *metrics); + + // Wait a bit for remote config download to be done + std::this_thread::sleep_for(std::chrono::milliseconds(100)); std::random_device device; std::mt19937_64 rng; rng.seed(device()); @@ -347,7 +361,8 @@ TEST(Sampler, testRemotelyControlledSampler) RemotelyControlledSampler::Clock::now() - startTime) .count() < 1;) { TraceID traceID(rng(), rng()); - sampler.isSampled(traceID, kTestOperationName); + // If probability was 0.5 we could reasonnably assume one of 50 samples fail + ASSERT_TRUE(sampler.isSampled(traceID, kTestOperationName).isSampled()); std::this_thread::sleep_for(std::chrono::milliseconds(20)); } sampler.close();