diff --git a/.bazelrc b/.bazelrc new file mode 120000 index 00000000..4baddac0 --- /dev/null +++ b/.bazelrc @@ -0,0 +1 @@ +.bazelrc.absl \ No newline at end of file diff --git a/.bazelrc.absl b/.bazelrc.absl new file mode 100644 index 00000000..3fd91881 --- /dev/null +++ b/.bazelrc.absl @@ -0,0 +1,15 @@ +# In order to support both Unixen and Windows, different styles of compiler +# flags must be used. +# +# This .bazelrc file specifies different compiler flags for Linux/Darwin versus +# Windows. +# +# This bazelrc defines the `DD_USE_ABSEIL_FOR_ENVOY` preprocessor macro, and so +# the resulting library will use `absl::string_view` and `absl::optional` +# instead of their standard (`std`) equivalents. + +build --enable_platform_specific_config + +build:linux --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' --cxxopt='-DDD_USE_ABSEIL_FOR_ENVOY' +build:macos --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' --cxxopt='-DDD_USE_ABSEIL_FOR_ENVOY' +build:windows --cxxopt='/std:c++17' --cxxopt='/DDD_USE_ABSEIL_FOR_ENVOY' --linkopt='ws2_32.lib' diff --git a/.bazelrc.std b/.bazelrc.std new file mode 100644 index 00000000..802f014d --- /dev/null +++ b/.bazelrc.std @@ -0,0 +1,15 @@ +# In order to support both Unixen and Windows, different styles of compiler +# flags must be used. +# +# This .bazelrc file specifies different compiler flags for Linux/Darwin versus +# Windows. +# +# This bazelrc does _not_ define the `DD_USE_ABSEIL_FOR_ENVOY` preprocessor +# macro, and so the resulting library will use `std::string_view` and +# `std::optional` instead of their Abseil equivalents. + +build --enable_platform_specific_config + +build:linux --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' +build:macos --cxxopt='-std=c++17' --cxxopt='-Wall' --cxxopt='-Wextra' --cxxopt='-pedantic' +build:windows --cxxopt='/std:c++17' --linkopt='ws2_32.lib' diff --git a/.circleci/config.yml b/.circleci/config.yml index 9963a8ef..4ff43253 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,5 +1,8 @@ version: 2.1 +orbs: + windows: circleci/windows@5.0 + executors: docker-amd64: docker: @@ -27,18 +30,38 @@ jobs: - checkout - run: find bin/ -executable -type f -print0 | xargs -0 shellcheck - build-bazel: + test-windows-bazel: + parameters: + # `bazelrc` is the path to the .bazelrc file to use in the build/test. + # The repository has two flavors: one that uses Abseil types (for use with + # Envoy), and one that uses std types. + bazelrc: + type: string + executor: windows/server-2022 + environment: + MAKE_JOB_COUNT: 4 + steps: + - checkout + - run: choco install -y bazelisk + - run: bazelisk --bazelrc=<< parameters.bazelrc >> test --jobs $env:MAKE_JOB_COUNT dd_trace_cpp_test + + test-bazel: parameters: toolchain: type: string arch: type: string + # `bazelrc` is the path to the .bazelrc file to use in the build/test. + # The repository has two flavors: one that uses Abseil types (for use with + # Envoy), and one that uses std types. + bazelrc: + type: string executor: docker-<< parameters.arch >> environment: MAKE_JOB_COUNT: 8 steps: - checkout - - run: bin/with-toolchain << parameters.toolchain >> bazelisk build --jobs $MAKE_JOB_COUNT dd_trace_cpp + - run: bin/with-toolchain << parameters.toolchain >> bazelisk --bazelrc=<< parameters.bazelrc >> test --jobs $MAKE_JOB_COUNT dd_trace_cpp_test test-cmake: parameters: @@ -113,7 +136,7 @@ jobs: DD_TRACE_CPP_COMMIT: << pipeline.git.revision >> command: | echo "https://github.com/DataDog/dd-trace-cpp@$DD_TRACE_CPP_COMMIT" > ~/project/system-tests/binaries/cpp-load-from-git - cd system-tests && ./build.sh -i runner && ./run.sh PARAMETRIC --log-cli-level=DEBUG + cd system-tests && ./build.sh -i runner && ./run.sh PARAMETRIC --log-cli-level=DEBUG - run: name: Collect artifacts command: tar -cvzf logs_cpp_parametric_dev.tar.gz -C system-tests logs_parametric @@ -127,17 +150,22 @@ workflows: jobs: - format - shellcheck + - test-windows-bazel: + matrix: + parameters: + bazelrc: [".bazelrc.absl", ".bazelrc.std"] - test-cmake: matrix: parameters: toolchain: ["gnu", "llvm"] sanitize: ["on", "off"] arch: ["amd64", "arm64"] - - build-bazel: + - test-bazel: matrix: parameters: arch: ["amd64", "arm64"] toolchain: ["gnu", "llvm"] + bazelrc: [".bazelrc.absl", ".bazelrc.std"] - coverage - system-tests: filters: diff --git a/.gitignore b/.gitignore index c96a43bd..478c3c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /bazel-* /.build/ /.coverage/ -MODULE.bazel.lock +/MODULE.bazel +/MODULE.bazel.lock diff --git a/BUILD.bazel b/BUILD.bazel index ae4a9631..e17662b3 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -117,14 +117,6 @@ cc_library( "src/datadog/version.h", "src/datadog/w3c_propagation.h", ], - copts = [ - "-Wall", - "-Wextra", - "-Werror", - "-pedantic", - "-std=c++17", - "-DDD_USE_ABSEIL_FOR_ENVOY", - ], strip_include_prefix = "src/", visibility = ["//visibility:public"], deps = [ @@ -132,3 +124,57 @@ cc_library( "@com_google_absl//absl/types:optional", ], ) + + +cc_test( + name = "dd_trace_cpp_test", + srcs = [ + # test driver + "test/main.cpp", + + # wrapper around Catch2 + "test/test.cpp", + "test/test.h", + "test/catch.hpp", + + # mocks + "test/mocks/collectors.cpp", + "test/mocks/collectors.h", + "test/mocks/dict_readers.cpp", + "test/mocks/dict_readers.h", + "test/mocks/dict_writers.cpp", + "test/mocks/dict_writers.h", + "test/mocks/event_schedulers.cpp", + "test/mocks/event_schedulers.h", + "test/mocks/http_clients.cpp", + "test/mocks/http_clients.h", + "test/mocks/loggers.cpp", + "test/mocks/loggers.h", + + # utilities + "test/matchers.cpp", + "test/matchers.h", + + # test cases + "test/test_cerr_logger.cpp", + # "test/test_curl.cpp", no curl: bazel build/test is for Envoy. + "test/test_datadog_agent.cpp", + "test/test_glob.cpp", + "test/test_limiter.cpp", + "test/test_metrics.cpp", + "test/test_msgpack.cpp", + "test/test_parse_util.cpp", + "test/test_smoke.cpp", + "test/test_span.cpp", + "test/test_span_sampler.cpp", + "test/test_trace_id.cpp", + "test/test_trace_segment.cpp", + "test/test_tracer_config.cpp", + "test/test_tracer_telemetry.cpp", + "test/test_tracer.cpp", + "test/test_trace_sampler.cpp", + ], + deps = [ + "dd_trace_cpp", + ], +) diff --git a/bin/bazel-build b/bin/bazel-build index 4c295c0d..de6be237 100755 --- a/bin/bazel-build +++ b/bin/bazel-build @@ -5,3 +5,5 @@ set -e cd "$(dirname "$0")"/.. bazelisk build dd_trace_cpp + +bazelisk "$@" test dd_trace_cpp_test diff --git a/bin/check b/bin/check index 79c4f5b2..c339bbc2 100755 --- a/bin/check +++ b/bin/check @@ -10,5 +10,6 @@ cd "$(dirname "$0")"/.. bin/format --dry-run -Werror bin/test -bin/bazel-build +bin/bazel-build --bazelrc=.bazelrc.absl +bin/bazel-build --bazelrc=.bazelrc.std find bin/ -executable -type f -print0 | xargs -0 shellcheck diff --git a/src/datadog/environment.cpp b/src/datadog/environment.cpp index 780bb42b..825de301 100644 --- a/src/datadog/environment.cpp +++ b/src/datadog/environment.cpp @@ -7,24 +7,30 @@ namespace datadog { namespace tracing { namespace environment { +namespace { + +Optional get_env(const char *name) { + if (const char *value = std::getenv(name)) { + return value; + } + return nullopt; +} + +} // namespace StringView name(Variable variable) { return variable_names[variable]; } -Optional lookup(Variable variable) { +Optional lookup(Variable variable) { const char *name = variable_names[variable]; - const char *value = std::getenv(name); - if (!value) { - return nullopt; - } - return StringView{value}; + return get_env(name); } nlohmann::json to_json() { auto result = nlohmann::json::object({}); for (const char *name : variable_names) { - if (const char *value = std::getenv(name)) { - result[name] = value; + if (auto value = get_env(name)) { + result[name] = std::move(*value); } } diff --git a/src/datadog/environment.h b/src/datadog/environment.h index 4e8a2f48..da1578f1 100644 --- a/src/datadog/environment.h +++ b/src/datadog/environment.h @@ -14,6 +14,8 @@ // // `lookup` retrieves the value of `Variable` in the environment. +#include + #include "json_fwd.hpp" #include "optional.h" #include "string_view.h" @@ -76,7 +78,7 @@ StringView name(Variable variable); // Return the value of the specified environment `variable`, or return // `nullopt` if that variable is not set in the environment. -Optional lookup(Variable variable); +Optional lookup(Variable variable); nlohmann::json to_json(); diff --git a/src/datadog/expected.h b/src/datadog/expected.h index f442fc55..e8abf782 100644 --- a/src/datadog/expected.h +++ b/src/datadog/expected.h @@ -37,6 +37,7 @@ // error then it cannot be "dereferenced" with `operator*`, i.e. it is analogous // to `Optional` (and is implemented as such). +#include #include #include "error.h" @@ -52,16 +53,16 @@ class Expected { public: Expected() = default; Expected(const Expected&) = default; - Expected(Expected&) = default; Expected(Expected&&) = default; + + Expected(const Value&); + Expected(Value&&); + Expected(const Error&); + Expected(Error&&); + Expected& operator=(const Expected&) = default; Expected& operator=(Expected&&) = default; - template - Expected(Other&&); - template - Expected& operator=(Other&&); - // Return whether this object holds a `Value` (as opposed to an `Error`). bool has_value() const noexcept; explicit operator bool() const noexcept; @@ -99,15 +100,16 @@ class Expected { }; template -template -Expected::Expected(Other&& other) : data_(std::forward(other)) {} +Expected::Expected(const Value& value) : data_(value) {} template -template -Expected& Expected::operator=(Other&& other) { - data_ = std::forward(other); - return *this; -} +Expected::Expected(Value&& value) : data_(std::move(value)) {} + +template +Expected::Expected(const Error& error) : data_(error) {} + +template +Expected::Expected(Error&& error) : data_(std::move(error)) {} template bool Expected::has_value() const noexcept { @@ -194,16 +196,17 @@ class Expected { public: Expected() = default; Expected(const Expected&) = default; - Expected(Expected&) = default; Expected(Expected&&) = default; + + Expected(const Error&); + Expected(Error&&); + Expected(decltype(nullopt)); + explicit Expected(const Optional&); + explicit Expected(Optional&&); + Expected& operator=(const Expected&) = default; Expected& operator=(Expected&&) = default; - template - Expected(Other&&); - template - Expected& operator=(Other&&); - void swap(Expected& other); bool has_value() const; @@ -221,14 +224,12 @@ class Expected { const Error* if_error() const&& = delete; }; -template -Expected::Expected(Other&& other) : data_(std::forward(other)) {} - -template -Expected& Expected::operator=(Other&& other) { - data_ = std::forward(other); - return *this; -} +inline Expected::Expected(const Error& error) : data_(error) {} +inline Expected::Expected(Error&& error) : data_(std::move(error)) {} +inline Expected::Expected(decltype(nullopt)) : data_(nullopt) {} +inline Expected::Expected(const Optional& data) : data_(data) {} +inline Expected::Expected(Optional&& data) + : data_(std::move(data)) {} inline void Expected::swap(Expected& other) { data_.swap(other.data_); } diff --git a/src/datadog/extraction_util.cpp b/src/datadog/extraction_util.cpp index 45b84647..5a0ab0d7 100644 --- a/src/datadog/extraction_util.cpp +++ b/src/datadog/extraction_util.cpp @@ -71,7 +71,7 @@ Expected> extract_id_header(const DictReader& headers, int base) { auto found = headers.lookup(header); if (!found) { - return nullopt; + return Optional(); } auto result = parse_uint64(*found, base); if (auto* error = result.if_error()) { @@ -87,7 +87,7 @@ Expected> extract_id_header(const DictReader& headers, prefix += ' '; return error->with_prefix(prefix); } - return *result; + return Optional(*result); } } // namespace diff --git a/src/datadog/msgpack.h b/src/datadog/msgpack.h index 2df96f1c..52b8a919 100644 --- a/src/datadog/msgpack.h +++ b/src/datadog/msgpack.h @@ -160,7 +160,7 @@ inline void pack_integer(std::string& buffer, std::int32_t value) { } inline Expected pack_string(std::string& buffer, StringView value) { - return pack_string(buffer, value.begin(), value.size()); + return pack_string(buffer, value.data(), value.size()); } } // namespace msgpack diff --git a/src/datadog/parse_util.cpp b/src/datadog/parse_util.cpp index 7632419f..ac6aaeac 100644 --- a/src/datadog/parse_util.cpp +++ b/src/datadog/parse_util.cpp @@ -17,14 +17,15 @@ namespace { template Expected parse_integer(StringView input, int base, StringView kind) { Integer value; - const auto status = std::from_chars(input.begin(), input.end(), value, base); + const char *const end = input.data() + input.size(); + const auto status = std::from_chars(input.data(), end, value, base); if (status.ec == std::errc::invalid_argument) { std::string message; message += "Is not a valid integer: \""; append(message, input); message += '\"'; return Error{Error::INVALID_INTEGER, std::move(message)}; - } else if (status.ptr != input.end()) { + } else if (status.ptr != end) { std::string message; message += "Integer has trailing characters in: \""; append(message, input); @@ -47,16 +48,15 @@ StringView strip(StringView input) { const auto not_whitespace = [](unsigned char ch) { return !std::isspace(ch); }; - const char *const begin = - std::find_if(input.begin(), input.end(), not_whitespace); - const char *const end = + const auto begin = std::find_if(input.begin(), input.end(), not_whitespace); + const auto end = std::find_if(input.rbegin(), std::make_reverse_iterator(begin), not_whitespace) .base(); assert(begin <= end); - return StringView{begin, std::size_t(end - begin)}; + return range(begin, end); } Expected parse_uint64(StringView input, int base) { diff --git a/src/datadog/parse_util.h b/src/datadog/parse_util.h index c2104447..6ca17c5d 100644 --- a/src/datadog/parse_util.h +++ b/src/datadog/parse_util.h @@ -14,8 +14,9 @@ namespace datadog { namespace tracing { // Return a `string_view` over the specified range of characters `[begin, end)`. -inline StringView range(const char* begin, const char* end) { - return StringView{begin, std::size_t(end - begin)}; +template +StringView range(CharIter1 begin, CharIter2 end) { + return StringView{&*begin, std::size_t(end - begin)}; } // Remove leading and trailing whitespace (as determined by `std::isspace`) from diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index c7c2c8b1..322c74c6 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -1,8 +1,10 @@ #include "platform_util.h" #ifdef _MSC_VER +// clang-format off +#include #include -#include +// clang-format on #else #include #include @@ -10,8 +12,51 @@ namespace datadog { namespace tracing { +namespace { + +// On Windows, `::gethostname()` requires that the winsock library is runtime +// initialized already. +// +// `void init_winsock_if_not_already()` initializes a static `class +// WinsockLibraryGuard` instance. Because it's `static`, the instance will be +// initialized at most once. +// +// `class WinsockLibraryGuard` initializes winsock in its constructor and +// cleans up winsock in its destructor. +// See +// . +#ifdef _MSC_VER +class WinsockLibraryGuard { + int startup_rc_; + + public: + WinsockLibraryGuard() { + const WORD version_requested = MAKEWORD(2, 2); + WSADATA info; + startup_rc_ = WSAStartup(version_requested, &info); + } + + ~WinsockLibraryGuard() { + // Call cleanup, but only if `WSAStartup` was successful. + if (startup_rc_ == 0) { + WSACleanup(); + } + } +}; + +void init_winsock_if_not_already() { + static WinsockLibraryGuard guard; + (void)guard; +} + +#endif + +} // namespace Optional get_hostname() { +#ifdef _MSC_VER + init_winsock_if_not_already(); +#endif char buffer[256]; if (::gethostname(buffer, sizeof buffer)) { return nullopt; diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp index ac4afcd6..4a2cc229 100644 --- a/src/datadog/trace_sampler_config.cpp +++ b/src/datadog/trace_sampler_config.cpp @@ -226,7 +226,7 @@ Expected finalize_config( message += "Trace sampling max_per_second must be greater than zero, but the " "following value was given: "; - message += std::to_string(*config.max_per_second); + message += std::to_string(max_per_second); return Error{Error::MAX_PER_SECOND_OUT_OF_RANGE, std::move(message)}; } result.max_per_second = max_per_second; diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index eae5cdd5..21fac8c0 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -68,8 +68,7 @@ Optional extract_traceparent(ExtractedData& result, const auto to_string_view = [](const auto& submatch) { assert(submatch.first <= submatch.second); - return StringView{submatch.first, - std::size_t(submatch.second - submatch.first)}; + return range(submatch.first, submatch.second); }; const auto version = to_string_view(match[1]); @@ -110,8 +109,8 @@ struct PartiallyParsedTracestate { Optional parse_tracestate(StringView tracestate) { Optional result; - const char* const begin = tracestate.begin(); - const char* const end = tracestate.end(); + const char* const begin = tracestate.data(); + const char* const end = begin + tracestate.size(); const char* pair_begin = begin; while (pair_begin != end) { const char* const pair_end = std::find(pair_begin, end, ','); @@ -174,8 +173,8 @@ Optional parse_tracestate(StringView tracestate) { // - `sampling_priority` // - `additional_datadog_w3c_tracestate` void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { - const char* const begin = datadog_value.begin(); - const char* const end = datadog_value.end(); + const char* const begin = datadog_value.data(); + const char* const end = begin + datadog_value.size(); const char* pair_begin = begin; while (pair_begin != end) { const char* const pair_end = std::find(pair_begin, end, ';'); diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h index ef48e3a4..3d0495a2 100644 --- a/test/mocks/collectors.h +++ b/test/mocks/collectors.h @@ -7,9 +7,9 @@ #include #include -#include #include #include +#include #include #include #include @@ -39,7 +39,7 @@ struct MockCollector : public Collector { } std::size_t span_count() const { - return std::accumulate(chunks.begin(), chunks.end(), 0, + return std::accumulate(chunks.begin(), chunks.end(), std::size_t(0), [](std::size_t total, const auto& chunk) { return total + chunk.size(); }); diff --git a/test/mocks/http_clients.h b/test/mocks/http_clients.h index ad59006c..3459ce90 100644 --- a/test/mocks/http_clients.h +++ b/test/mocks/http_clients.h @@ -50,7 +50,7 @@ struct MockHTTPClient : public HTTPClient { on_error_ = on_error; set_headers(request_headers); } - return post_error; + return Expected(post_error); } void drain(std::chrono::steady_clock::time_point /*deadline*/) override { diff --git a/test/test_limiter.cpp b/test/test_limiter.cpp index 2ffb42f8..c040eb1d 100644 --- a/test/test_limiter.cpp +++ b/test/test_limiter.cpp @@ -6,6 +6,16 @@ #include "test.h" +// The `timegm` function has a different name on Windows. +// See . +#ifdef _MSC_VER +namespace { + +std::time_t timegm(std::tm *tm) { return _mkgmtime(tm); } + +} // namespace +#endif + using namespace datadog::tracing; TEST_CASE("limiter") { diff --git a/test/test_smoke.cpp b/test/test_smoke.cpp index 924a9312..cd4549d6 100644 --- a/test/test_smoke.cpp +++ b/test/test_smoke.cpp @@ -4,6 +4,7 @@ #include #include +#include "mocks/http_clients.h" #include "mocks/loggers.h" #include "test.h" @@ -12,6 +13,7 @@ using namespace datadog::tracing; TEST_CASE("smoke") { TracerConfig config; config.service = "testsvc"; + config.agent.http_client = std::make_shared(); config.logger = std::make_shared(); config.collector = std::make_shared(); diff --git a/test/test_span.cpp b/test/test_span.cpp index 3cdf599c..3867d5bc 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -24,6 +24,7 @@ #include "mocks/collectors.h" #include "mocks/dict_readers.h" #include "mocks/dict_writers.h" +#include "mocks/http_clients.h" #include "mocks/loggers.h" #include "test.h" @@ -704,6 +705,7 @@ TEST_CASE("injecting W3C tracestate header") { TEST_CASE("128-bit trace ID injection") { TracerConfig config; config.service = "testsvc"; + config.agent.http_client = std::make_shared(); config.logger = std::make_shared(); config.generate_128bit_trace_ids = true; diff --git a/test/test_span_sampler.cpp b/test/test_span_sampler.cpp index 298ee72b..3a9009eb 100644 --- a/test/test_span_sampler.cpp +++ b/test/test_span_sampler.cpp @@ -72,19 +72,19 @@ SpanSamplingTags span_sampling_tags(const SpanData& span) { SpanSamplerConfig::Rule by_service(StringView service) { SpanSamplerConfig::Rule rule; - rule.service = service; + assign(rule.service, service); return rule; } SpanSamplerConfig::Rule by_name(StringView name) { SpanSamplerConfig::Rule rule; - rule.name = name; + assign(rule.name, name); return rule; } SpanSamplerConfig::Rule by_resource(StringView resource) { SpanSamplerConfig::Rule rule; - rule.resource = resource; + assign(rule.resource, resource); return rule; } @@ -98,7 +98,7 @@ SpanSamplerConfig::Rule by_tags( SpanSamplerConfig::Rule by_name_and_tags( StringView name, std::unordered_map tags) { SpanSamplerConfig::Rule rule; - rule.name = name; + assign(rule.name, name); rule.tags = std::move(tags); return rule; } diff --git a/test/test_tracer.cpp b/test/test_tracer.cpp index 987a7ba2..121f1c06 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -29,6 +29,7 @@ #include "mocks/collectors.h" #include "mocks/dict_readers.h" #include "mocks/dict_writers.h" +#include "mocks/http_clients.h" #include "mocks/loggers.h" #include "test.h" @@ -1685,6 +1686,7 @@ TEST_CASE("heterogeneous extraction") { TracerConfig config; config.service = "testsvc"; + config.agent.http_client = std::make_shared(); config.extraction_styles = test_case.extraction_styles; config.injection_styles = test_case.injection_styles; config.logger = std::make_shared(); diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 736dec9c..90cb1d26 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -4,6 +4,7 @@ #include #include #include +#include // setenv (windows/unix), unsetenv (unix), _putenv_s (windows) #include #include @@ -21,13 +22,9 @@ #include "mocks/collectors.h" #include "mocks/event_schedulers.h" +#include "mocks/http_clients.h" #include "mocks/loggers.h" #include "test.h" -#ifdef _MSC_VER -#include // SetEnvironmentVariable -#else -#include // setenv, unsetenv -#endif namespace datadog { namespace tracing { @@ -51,7 +48,7 @@ class EnvGuard { public: EnvGuard(std::string name, std::string value) : name_(std::move(name)) { - const char* current = std::getenv(name_.c_str()); + const char* current = get_value(); if (current) { former_value_ = current; } @@ -66,9 +63,11 @@ class EnvGuard { } } + const char* get_value() { return std::getenv(name_.c_str()); } + void set_value(const std::string& value) { #ifdef _MSC_VER - ::SetEnvironmentVariable(name_.c_str(), value.c_str()); + ::_putenv_s(name_.c_str(), value.c_str()); #else const bool overwrite = true; ::setenv(name_.c_str(), value.c_str(), overwrite); @@ -77,7 +76,7 @@ class EnvGuard { void unset() { #ifdef _MSC_VER - ::SetEnvironmentVariable(name_.c_str(), NULL); + ::_putenv_s(name_.c_str(), ""); #else ::unsetenv(name_.c_str()); #endif @@ -144,6 +143,7 @@ class SomewhatSecureTemporaryFile : public std::fstream { TEST_CASE("TracerConfig::defaults") { TracerConfig config; + config.agent.http_client = std::make_shared(); SECTION("service is required") { SECTION("empty") { @@ -212,7 +212,6 @@ TEST_CASE("TracerConfig::defaults") { }; auto test_case = GENERATE(values({ - {"empty", "", {}, nullopt}, {"missing colon", "foo", {}, Error::TAG_MISSING_SEPARATOR}, {"trailing comma", "foo:bar, baz:123,", @@ -249,6 +248,7 @@ TEST_CASE("TracerConfig::defaults") { TEST_CASE("TracerConfig::log_on_startup") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; const auto logger = std::make_shared(); config.logger = logger; @@ -310,6 +310,7 @@ TEST_CASE("TracerConfig::log_on_startup") { TEST_CASE("TracerConfig::report_traces") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; const auto collector = std::make_shared(); config.collector = collector; @@ -379,6 +380,7 @@ TEST_CASE("TracerConfig::report_traces") { TEST_CASE("TracerConfig::agent") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; SECTION("event_scheduler") { @@ -537,7 +539,6 @@ TEST_CASE("TracerConfig::agent") { // during configuration. For the purposes of configuration, any // value is accepted. {"we don't parse port", x, "bogus", x, "http", "localhost:bogus"}, - {"even empty is ok", x, "", x, "http", "localhost:"}, {"URL", x, x, "http://dd-agent:8080", "http", "dd-agent:8080"}, {"URL overrides scheme", x, x, "https://dd-agent:8080", "https", "dd-agent:8080"}, @@ -576,6 +577,7 @@ TEST_CASE("TracerConfig::agent") { TEST_CASE("TracerConfig::trace_sampler") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; SECTION("default is no rules") { @@ -659,7 +661,6 @@ TEST_CASE("TracerConfig::trace_sampler") { }; auto test_case = GENERATE(values({ - {"empty", "", {Error::INVALID_DOUBLE}}, {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, {"trailing space", "0.23 ", {Error::INVALID_DOUBLE}}, {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, @@ -725,7 +726,6 @@ TEST_CASE("TracerConfig::trace_sampler") { }; auto test_case = GENERATE(values({ - {"empty", "", {Error::INVALID_DOUBLE}}, {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, {"trailing space", "23 ", {Error::INVALID_DOUBLE}}, {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, @@ -841,6 +841,7 @@ TEST_CASE("TracerConfig::trace_sampler") { TEST_CASE("TracerConfig::span_sampler") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; SECTION("default is no rules") { @@ -1036,13 +1037,18 @@ TEST_CASE("TracerConfig::span_sampler") { SECTION("failed usage") { SECTION("unable to open") { - std::filesystem::path defunct; - { - SomewhatSecureTemporaryFile file; - REQUIRE(file.is_open()); - defunct = file.path(); - } - const EnvGuard guard{"DD_SPAN_SAMPLING_RULES_FILE", defunct.string()}; + // It's not elegant, but neither an empty path nor a path to a + // deleted file work for this test on Windows. + // + // On Windows, deleting the file doesn't delete the file, and an + // empty path deletes the environment variable rather than set the + // environment variable empty. + // + // An easy workaround is to choose a path that is very likely not on + // the file system. + const std::string invalid = "ooga/booga/booga/booga"; + + const EnvGuard guard{"DD_SPAN_SAMPLING_RULES_FILE", invalid}; auto finalized = finalize_config(config); REQUIRE(!finalized); REQUIRE(finalized.error().code == Error::SPAN_SAMPLING_RULES_FILE_IO); @@ -1071,6 +1077,7 @@ TEST_CASE("TracerConfig::span_sampler") { TEST_CASE("TracerConfig propagation styles") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; SECTION("default style is [Datadog, W3C]") { @@ -1145,8 +1152,9 @@ TEST_CASE("TracerConfig propagation styles") { }; // brevity - const auto datadog = PropagationStyle::DATADOG, - b3 = PropagationStyle::B3, none = PropagationStyle::NONE; + static const auto datadog = PropagationStyle::DATADOG, + b3 = PropagationStyle::B3, + none = PropagationStyle::NONE; // clang-format off auto test_case = GENERATE(values({ {__LINE__, "Datadog", x, {datadog}}, @@ -1292,6 +1300,7 @@ TEST_CASE("TracerConfig propagation styles") { TEST_CASE("configure 128-bit trace IDs") { TracerConfig config; + config.agent.http_client = std::make_shared(); config.service = "testsvc"; SECTION("defaults to true") { @@ -1322,7 +1331,6 @@ TEST_CASE("configure 128-bit trace IDs") { {__LINE__, "no", false}, {__LINE__, "nein", true}, {__LINE__, "0", false}, - {__LINE__, "", true}, })); // clang-format on diff --git a/test/test_tracer_telemetry.cpp b/test/test_tracer_telemetry.cpp index 534a17d4..9040b96b 100644 --- a/test/test_tracer_telemetry.cpp +++ b/test/test_tracer_telemetry.cpp @@ -16,7 +16,7 @@ using namespace datadog::tracing; TEST_CASE("Tracer telemetry", "[telemetry]") { const std::time_t mock_time = 1672484400; - const Clock clock = []() { + const Clock clock = [mock_time = mock_time]() { TimePoint result; result.wall = std::chrono::system_clock::from_time_t(mock_time); return result;