diff --git a/src/jaegertracing/ConfigTest.cpp b/src/jaegertracing/ConfigTest.cpp index 128c6007..914265e0 100644 --- a/src/jaegertracing/ConfigTest.cpp +++ b/src/jaegertracing/ConfigTest.cpp @@ -176,6 +176,18 @@ TEST(Config, testFromEnv) ASSERT_EQ(true, config.disabled()); ASSERT_EQ(std::string("host33:445"), config.reporter().localAgentHostPort()); + + testutils::EnvVariable::setEnv("JAEGER_AGENT_HOST", ""); + testutils::EnvVariable::setEnv("JAEGER_AGENT_PORT", ""); + testutils::EnvVariable::setEnv("JAEGER_ENDPOINT", ""); + testutils::EnvVariable::setEnv("JAEGER_REPORTER_MAX_QUEUE_SIZE", ""); + testutils::EnvVariable::setEnv("JAEGER_REPORTER_FLUSH_INTERVAL", ""); + testutils::EnvVariable::setEnv("JAEGER_REPORTER_LOG_SPANS", ""); + testutils::EnvVariable::setEnv("JAEGER_SAMPLER_PARAM", ""); + testutils::EnvVariable::setEnv("JAEGER_SAMPLER_TYPE", ""); + testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", ""); + testutils::EnvVariable::setEnv("JAEGER_TAGS", ""); + testutils::EnvVariable::setEnv("JAEGER_DISABLED", ""); } } // namespace jaegertracing diff --git a/src/jaegertracing/DynamicLoad.cpp b/src/jaegertracing/DynamicLoad.cpp index 7c6f4637..5f7d5fa6 100644 --- a/src/jaegertracing/DynamicLoad.cpp +++ b/src/jaegertracing/DynamicLoad.cpp @@ -41,13 +41,16 @@ static int makeTracerFactory(const char* opentracingVersion, return opentracing::incompatible_library_versions_error.value(); } - *tracerFactory = new (std::nothrow) jaegertracing::TracerFactory{}; + const auto jaegerTracerFactory = new (std::nothrow) jaegertracing::TracerFactory{}; + *tracerFactory = jaegerTracerFactory; if (*tracerFactory == nullptr) { *errorCategory = static_cast(&std::generic_category()); return static_cast(std::errc::not_enough_memory); } + jaegerTracerFactory->enableReadFromEnv(); + return 0; } -OPENTRACING_DECLARE_IMPL_FACTORY(makeTracerFactory) \ No newline at end of file +OPENTRACING_DECLARE_IMPL_FACTORY(makeTracerFactory) diff --git a/src/jaegertracing/TracerFactory.cpp b/src/jaegertracing/TracerFactory.cpp index 68a27d47..8ead064d 100644 --- a/src/jaegertracing/TracerFactory.cpp +++ b/src/jaegertracing/TracerFactory.cpp @@ -40,7 +40,9 @@ TracerFactory::MakeTracer(const char* configuration, auto tracerConfig = jaegertracing::Config::parse(yaml); - tracerConfig.fromEnv(); + if (readFromEnv) { + tracerConfig.fromEnv(); + } if (tracerConfig.serviceName().empty()) { errorMessage = "`service_name` not provided"; @@ -58,4 +60,8 @@ TracerFactory::MakeTracer(const char* configuration, return opentracing::make_unexpected( opentracing::invalid_configuration_error); } + +void TracerFactory::enableReadFromEnv() { + readFromEnv = true; +} } // namespace jaegertracing diff --git a/src/jaegertracing/TracerFactory.h b/src/jaegertracing/TracerFactory.h index 7369ac29..c0b4f7fb 100644 --- a/src/jaegertracing/TracerFactory.h +++ b/src/jaegertracing/TracerFactory.h @@ -26,6 +26,14 @@ class TracerFactory : public opentracing::TracerFactory { opentracing::expected> MakeTracer(const char* configuration, std::string& errorMessage) const noexcept override; + + TracerFactory() { + readFromEnv = false; + } + + void enableReadFromEnv(); + private: + bool readFromEnv; }; } // namespace jaegertracing diff --git a/src/jaegertracing/TracerFactoryTest.cpp b/src/jaegertracing/TracerFactoryTest.cpp index 5a3f376f..a3c32427 100644 --- a/src/jaegertracing/TracerFactoryTest.cpp +++ b/src/jaegertracing/TracerFactoryTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include "jaegertracing/Tracer.h" #include "jaegertracing/TracerFactory.h" #include "jaegertracing/Constants.h" #include "jaegertracing/testutils/EnvVariable.h" @@ -23,7 +24,6 @@ namespace jaegertracing { #ifdef JAEGERTRACING_WITH_YAML_CPP TEST(TracerFactory, testInvalidConfig) { - testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", ""); const char* invalidConfigTestCases[] = { "", "abc: {", R"({ @@ -67,7 +67,6 @@ TEST(TracerFactory, testValidConfig) "refreshInterval": 60 } })"; - testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", ""); TracerFactory tracerFactory; std::string errorMessage; auto tracerMaybe = tracerFactory.MakeTracer(config, errorMessage); @@ -75,15 +74,58 @@ TEST(TracerFactory, testValidConfig) ASSERT_TRUE(tracerMaybe); } -TEST(TracerFactory, testEnvironmentConfig) +TEST(TracerFactory, testWithoutReadFromEnv) +{ + const char* config = ""; + testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", "AService"); + TracerFactory tracerFactory; + std::string errorMessage; + auto tracerMaybe = tracerFactory.MakeTracer(config, errorMessage); + ASSERT_FALSE(tracerMaybe); + ASSERT_NE(errorMessage, ""); + + testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", ""); +} + +TEST(TracerFactory, testWithReadFromEnv) { const char* config = ""; testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", "AService"); TracerFactory tracerFactory; std::string errorMessage; + tracerFactory.enableReadFromEnv(); + auto tracerMaybe = tracerFactory.MakeTracer(config, errorMessage); + ASSERT_EQ(errorMessage, ""); + ASSERT_TRUE(tracerMaybe); + + auto tracer = tracerMaybe.value(); + const auto jaegerTracer = std::dynamic_pointer_cast(tracer); + ASSERT_EQ(std::string("AService"), jaegerTracer->serviceName()); + + testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", ""); +} + +TEST(TracerFactory, testFileConfigTakesPrecedence) +{ + + const char* config = R"( + { + "service_name": "test" + })"; + testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", "Ignored"); + TracerFactory tracerFactory; + std::string errorMessage; + // TODO: This should work, but doesn't yet. + // tracerFactory.enableReadFromEnv(); auto tracerMaybe = tracerFactory.MakeTracer(config, errorMessage); ASSERT_EQ(errorMessage, ""); ASSERT_TRUE(tracerMaybe); + + auto tracer = tracerMaybe.value(); + const auto jaegerTracer = std::dynamic_pointer_cast(tracer); + ASSERT_EQ(std::string("test"), jaegerTracer->serviceName()); + + testutils::EnvVariable::setEnv("JAEGER_SERVICE_NAME", ""); } #else TEST(TracerFactory, failsWithoutYAML)