From c998bbac7eb5e1dd5b9f67ea2e173928fd7ff5fd Mon Sep 17 00:00:00 2001 From: Anil Mahtani <929854+Anilm3@users.noreply.github.com> Date: Fri, 22 Sep 2023 16:39:28 +0100 Subject: [PATCH] Propagate Runtime IDs and keep a valid pool in RC client (#301) --- src/extension/commands/client_init.c | 9 +- src/extension/ddtrace.c | 54 ++++++++ src/extension/ddtrace.h | 1 + src/helper/client.cpp | 14 ++ src/helper/client.hpp | 1 + src/helper/remote_config/client.cpp | 6 +- src/helper/remote_config/client.hpp | 10 +- src/helper/remote_config/client_handler.cpp | 6 - src/helper/remote_config/client_handler.hpp | 13 ++ src/helper/runtime_id_pool.hpp | 82 +++++++++++ src/helper/service.hpp | 22 ++- src/helper/service_identifier.hpp | 7 +- src/helper/service_manager.cpp | 4 + tests/extension/rinit_rshutdown_basic.phpt | Bin 5330 -> 5557 bytes .../extension/runtime_id_without_ddtrace.phpt | 10 ++ tests/helper/client_test.cpp | 94 +++++++++++++ .../remote_config/client_handler_test.cpp | 68 ++++----- tests/helper/remote_config/client_test.cpp | 51 +++++++ tests/helper/remote_config/mocks.hpp | 15 ++ tests/helper/runtime_id_pool_test.cpp | 130 ++++++++++++++++++ tests/helper/service_manager_test.cpp | 20 +++ tests/helper/service_test.cpp | 15 +- 22 files changed, 561 insertions(+), 71 deletions(-) create mode 100644 src/helper/runtime_id_pool.hpp create mode 100644 tests/extension/runtime_id_without_ddtrace.phpt create mode 100644 tests/helper/runtime_id_pool_test.cpp diff --git a/src/extension/commands/client_init.c b/src/extension/commands/client_init.c index 131a6618ea..d18e490d6f 100644 --- a/src/extension/commands/client_init.c +++ b/src/extension/commands/client_init.c @@ -125,8 +125,13 @@ static dd_result _pack_command( // We send this empty for now. The helper will check for empty and if so it // will generate it dd_mpack_write_lstr(w, "runtime_id"); - dd_mpack_write_nullable_cstr(w, ""); - + zend_string *runtime_id = dd_trace_get_formatted_runtime_id(false); + if (runtime_id == NULL) { + dd_mpack_write_nullable_cstr(w, ""); + } else { + dd_mpack_write_nullable_zstr(w, runtime_id); + zend_string_free(runtime_id); + } mpack_finish_map(w); // Engine settings diff --git a/src/extension/ddtrace.c b/src/extension/ddtrace.c index cb1865e013..61217ca13b 100644 --- a/src/extension/ddtrace.c +++ b/src/extension/ddtrace.c @@ -6,6 +6,7 @@ #include "ddtrace.h" #include #include +#include #include "configuration.h" #include "ddappsec.h" @@ -26,6 +27,7 @@ static zend_string *_metrics_propname; static THREAD_LOCAL_ON_ZTS bool _suppress_ddtrace_rshutdown; static THREAD_LOCAL_ON_ZTS zval *_span_meta; static THREAD_LOCAL_ON_ZTS zval *_span_metrics; +static uint8_t *_ddtrace_runtime_id = NULL; static zend_module_entry *_find_ddtrace_module(void); static int _ddtrace_rshutdown_testing(SHUTDOWN_FUNC_ARGS); @@ -70,6 +72,12 @@ static void dd_trace_load_symbols(void) dlerror()); } + _ddtrace_runtime_id = dlsym(handle, "ddtrace_runtime_id"); + if (_ddtrace_runtime_id == NULL) { + // NOLINTNEXTLINE(concurrency-mt-unsafe) + mlog(dd_log_debug, "Failed to load ddtrace_runtime_id: %s", dlerror()); + } + dlclose(handle); } @@ -245,6 +253,34 @@ zval *nullable dd_trace_root_span_get_metrics() return _ddtrace_root_span_get_metrics(); } +// NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) +zend_string *nullable dd_trace_get_formatted_runtime_id(bool persistent) +{ + if (_ddtrace_runtime_id == NULL) { + return NULL; + } + + zend_string *encoded_id = zend_string_alloc(36, persistent); + + size_t length = sprintf(ZSTR_VAL(encoded_id), + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x", + _ddtrace_runtime_id[0], _ddtrace_runtime_id[1], _ddtrace_runtime_id[2], + _ddtrace_runtime_id[3], _ddtrace_runtime_id[4], _ddtrace_runtime_id[5], + _ddtrace_runtime_id[6], _ddtrace_runtime_id[7], _ddtrace_runtime_id[8], + _ddtrace_runtime_id[9], _ddtrace_runtime_id[10], + _ddtrace_runtime_id[11], _ddtrace_runtime_id[12], + _ddtrace_runtime_id[13], _ddtrace_runtime_id[14], + _ddtrace_runtime_id[15]); + + if (length != 36) { + zend_string_free(encoded_id); + encoded_id = NULL; + } + + return encoded_id; +} +// NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + static PHP_FUNCTION(datadog_appsec_testing_ddtrace_rshutdown) { if (zend_parse_parameters_none() == FAILURE) { @@ -309,6 +345,19 @@ static PHP_FUNCTION(datadog_appsec_testing_root_span_get_metrics) // NOLINT } } +static PHP_FUNCTION(datadog_appsec_testing_get_formatted_runtime_id) // NOLINT +{ + if (zend_parse_parameters_none() == FAILURE) { + RETURN_FALSE; + } + + zend_string *id = dd_trace_get_formatted_runtime_id(false); + if (id != NULL) { + RETURN_STR(id); + } + RETURN_EMPTY_STRING(); +} + // clang-format off ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(void_ret_bool_arginfo, 0, 0, _IS_BOOL, 0) ZEND_END_ARG_INFO() @@ -316,6 +365,10 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(void_ret_nullable_array, 0, 0, IS_ARRAY, 1) ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(void_ret_nullable_string, 0, 0, IS_STRING, 1) +ZEND_END_ARG_INFO() + + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_root_span_add_tag, 0, 0, _IS_BOOL, 2) ZEND_ARG_TYPE_INFO(0, tag, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, value, IS_STRING, 0) @@ -326,6 +379,7 @@ static const zend_function_entry functions[] = { ZEND_RAW_FENTRY(DD_TESTING_NS "root_span_add_tag", PHP_FN(datadog_appsec_testing_root_span_add_tag), arginfo_root_span_add_tag, 0) ZEND_RAW_FENTRY(DD_TESTING_NS "root_span_get_meta", PHP_FN(datadog_appsec_testing_root_span_get_meta), void_ret_nullable_array, 0) ZEND_RAW_FENTRY(DD_TESTING_NS "root_span_get_metrics", PHP_FN(datadog_appsec_testing_root_span_get_metrics), void_ret_nullable_array, 0) + ZEND_RAW_FENTRY(DD_TESTING_NS "get_formatted_runtime_id", PHP_FN(datadog_appsec_testing_get_formatted_runtime_id), void_ret_nullable_string, 0) PHP_FE_END }; // clang-format on diff --git a/src/extension/ddtrace.h b/src/extension/ddtrace.h index 95d185d1a0..9f8f04db7a 100644 --- a/src/extension/ddtrace.h +++ b/src/extension/ddtrace.h @@ -33,3 +33,4 @@ void dd_trace_close_all_spans_and_flush(void); // It is ready for modification, with refcount == 1 zval *nullable dd_trace_root_span_get_meta(void); zval *nullable dd_trace_root_span_get_metrics(void); +zend_string *nullable dd_trace_get_formatted_runtime_id(bool persistent); diff --git a/src/helper/client.cpp b/src/helper/client.cpp index ac5152b81b..e04635db5c 100644 --- a/src/helper/client.cpp +++ b/src/helper/client.cpp @@ -156,11 +156,19 @@ bool client::handle_command(const network::client_init::request &command) bool has_errors = false; client_enabled_conf = command.enabled_configuration; + if (service_id.runtime_id.empty()) { + service_id.runtime_id = generate_random_uuid(); + } + runtime_id_ = service_id.runtime_id; try { service_ = service_manager_->create_service(std::move(service_id), eng_settings, command.rc_settings, meta, metrics, !client_enabled_conf.has_value()); + if (service_) { + // This null check is only needed due to some tests + service_->register_runtime_id(runtime_id_); + } } catch (std::system_error &e) { // TODO: logging should happen at WAF impl DD_STDLOG(DD_STDLOG_RULES_FILE_NOT_FOUND, @@ -499,6 +507,12 @@ bool client::run_request() void client::run(worker::queue_consumer &q) { + const defer on_exit{[this]() { + if (this->service_) { + this->service_->unregister_runtime_id(this->runtime_id_); + } + }}; + if (q.running()) { if (!run_client_init()) { SPDLOG_DEBUG("Finished handling client (client_init failed)"); diff --git a/src/helper/client.hpp b/src/helper/client.hpp index dbeaf55b47..7488c7c4e6 100644 --- a/src/helper/client.hpp +++ b/src/helper/client.hpp @@ -69,6 +69,7 @@ class client { std::optional context_; std::optional client_enabled_conf; bool request_enabled_ = {false}; + std::string runtime_id_; }; } // namespace dds diff --git a/src/helper/remote_config/client.cpp b/src/helper/remote_config/client.cpp index a5abbee8ad..fb0c69aa34 100644 --- a/src/helper/remote_config/client.cpp +++ b/src/helper/remote_config/client.cpp @@ -32,8 +32,8 @@ client::client(std::unique_ptr &&arg_api, service_identifier &&sid, remote_config::settings settings, std::vector listeners) : api_(std::move(arg_api)), id_(dds::generate_random_uuid()), - sid_(std::move(sid)), settings_(std::move(settings)), - listeners_(std::move(listeners)) + ids_(sid.runtime_id), sid_(std::move(sid)), + settings_(std::move(settings)), listeners_(std::move(listeners)) { for (auto const &listener : listeners_) { const auto &supported_products = listener->get_supported_products(); @@ -75,7 +75,7 @@ client::ptr client::from_settings(service_identifier &&sid, } } - const protocol::client_tracer ct{sid_.runtime_id, sid_.tracer_version, + const protocol::client_tracer ct{std::move(ids_.get()), sid_.tracer_version, sid_.service, sid_.extra_services, sid_.env, sid_.app_version}; const protocol::client_state cs{targets_version_, config_states, diff --git a/src/helper/remote_config/client.hpp b/src/helper/remote_config/client.hpp index 9908c3dd66..542d6ff216 100644 --- a/src/helper/remote_config/client.hpp +++ b/src/helper/remote_config/client.hpp @@ -19,6 +19,7 @@ #include "protocol/client.hpp" #include "protocol/tuf/get_configs_request.hpp" #include "protocol/tuf/get_configs_response.hpp" +#include "runtime_id_pool.hpp" #include "service_config.hpp" #include "settings.hpp" #include "utils.hpp" @@ -42,7 +43,7 @@ class client { virtual ~client() = default; client(const client &) = delete; - client(client &&) = default; + client(client &&) = delete; client &operator=(const client &) = delete; client &operator=(client &&) = delete; @@ -63,6 +64,12 @@ class client { return sid_; } + virtual void register_runtime_id(const std::string &id) { ids_.add(id); } + virtual void unregister_runtime_id(const std::string &id) + { + ids_.remove(id); + } + protected: [[nodiscard]] protocol::get_configs_request generate_request() const; bool process_response(const protocol::get_configs_response &response); @@ -70,6 +77,7 @@ class client { std::unique_ptr api_; std::string id_; + runtime_id_pool ids_; const service_identifier sid_; const remote_config::settings settings_; diff --git a/src/helper/remote_config/client_handler.cpp b/src/helper/remote_config/client_handler.cpp index d6c11f41d6..1355d00124 100644 --- a/src/helper/remote_config/client_handler.cpp +++ b/src/helper/remote_config/client_handler.cpp @@ -46,12 +46,6 @@ client_handler::ptr client_handler::from_settings(service_identifier &&id, return {}; } - // TODO runtime_id will be send by the extension when the extension can get - // it from the profiler. When that happen, this wont be needed - if (id.runtime_id.empty()) { - id.runtime_id = generate_random_uuid(); - } - std::vector listeners = {}; if (dynamic_enablement) { listeners.emplace_back( diff --git a/src/helper/remote_config/client_handler.hpp b/src/helper/remote_config/client_handler.hpp index ab0208f39c..4fea0e2a06 100644 --- a/src/helper/remote_config/client_handler.hpp +++ b/src/helper/remote_config/client_handler.hpp @@ -46,6 +46,19 @@ class client_handler { remote_config::client *get_client() { return rc_client_.get(); } + void register_runtime_id(const std::string &id) + { + if (rc_client_) { + rc_client_->register_runtime_id(id); + } + } + void unregister_runtime_id(const std::string &id) + { + if (rc_client_) { + rc_client_->unregister_runtime_id(id); + } + } + protected: void run(std::future &&exit_signal); void handle_error(); diff --git a/src/helper/runtime_id_pool.hpp b/src/helper/runtime_id_pool.hpp new file mode 100644 index 0000000000..4764c03909 --- /dev/null +++ b/src/helper/runtime_id_pool.hpp @@ -0,0 +1,82 @@ +// Unless explicitly stated otherwise all files in this repository are +// dual-licensed under the Apache-2.0 License or BSD-3-Clause License. +// +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. +#pragma once + +#include +#include +#include +#include + +namespace dds { + +/* The runtime ID pool basically provides: + * - A thread-safe mechanism to store and remove runtime IDs. + * - A function to retrieve a valid runtime ID that doesn't change + * for as long as it is valid. + * - An guarantee that there will always be an ID provided, even if + * the process who owned that ID has already been finalised. + */ +class runtime_id_pool { +public: + explicit runtime_id_pool(const std::string &initial_id) + { + if (initial_id.empty()) { + throw std::invalid_argument( + "runtime ID pool initialised with invalid ID"); + } + + std::lock_guard lock{mtx_}; + ids_.emplace(initial_id); + current_ = *ids_.begin(); + } + + void add(std::string id) + { + // Empty IDs aren't valid + if (id.empty()) { + return; + } + + std::lock_guard lock{mtx_}; + ids_.emplace(std::move(id)); + if (ids_.size() == 1 || current_.empty()) { + current_ = *ids_.begin(); + } + } + + void remove(const std::string &id) + { + // Empty IDs aren't valid + if (id.empty()) { + return; + } + + std::lock_guard lock{mtx_}; + auto it = ids_.find(id); + if (it != ids_.end()) { + ids_.erase(it); + + // Don't change the ID if there are no more IDs or if the current ID + // is still valid within the multiset + if (!ids_.empty() && ids_.find(current_) == ids_.end()) { + current_ = *ids_.begin(); + } + } + } + + [[nodiscard]] std::string get() const + { + std::lock_guard lock{mtx_}; + return current_; + } + +protected: + mutable std::mutex mtx_; + std::unordered_multiset ids_; + std::string current_; +}; + +} // namespace dds diff --git a/src/helper/service.hpp b/src/helper/service.hpp index c78a774e67..8998d3796a 100644 --- a/src/helper/service.hpp +++ b/src/helper/service.hpp @@ -34,12 +34,28 @@ class service { service(service &&) = delete; service &operator=(service &&) = delete; + virtual ~service() = default; + static service::ptr from_settings(service_identifier &&id, const dds::engine_settings &eng_settings, const remote_config::settings &rc_settings, std::map &meta, std::map &metrics, bool dynamic_enablement); + virtual void register_runtime_id(const std::string &id) + { + if (client_handler_) { + client_handler_->register_runtime_id(id); + } + } + + virtual void unregister_runtime_id(const std::string &id) + { + if (client_handler_) { + client_handler_->unregister_runtime_id(id); + } + } + [[nodiscard]] std::shared_ptr get_engine() const { // TODO make access atomic? @@ -53,9 +69,9 @@ class service { } protected: - std::shared_ptr engine_; - std::shared_ptr service_config_; - dds::remote_config::client_handler::ptr client_handler_; + std::shared_ptr engine_{}; + std::shared_ptr service_config_{}; + dds::remote_config::client_handler::ptr client_handler_{}; }; } // namespace dds diff --git a/src/helper/service_identifier.hpp b/src/helper/service_identifier.hpp index 6511e24c29..fb967cf820 100644 --- a/src/helper/service_identifier.hpp +++ b/src/helper/service_identifier.hpp @@ -23,9 +23,7 @@ struct service_identifier { bool operator==(const service_identifier &oth) const noexcept { - return service == oth.service && env == oth.env && - tracer_version == oth.tracer_version && - app_version == oth.app_version && runtime_id == oth.runtime_id; + return service == oth.service && env == oth.env; } friend auto &operator<<(std::ostream &os, const service_identifier &id) @@ -50,8 +48,7 @@ struct service_identifier { struct hash { std::size_t operator()(const service_identifier &id) const noexcept { - return dds::hash(id.service, id.env, id.tracer_version, - id.app_version, id.runtime_id); + return dds::hash(id.service, id.env); } }; }; diff --git a/src/helper/service_manager.cpp b/src/helper/service_manager.cpp index e20e7ff58d..f79a0dca8a 100644 --- a/src/helper/service_manager.cpp +++ b/src/helper/service_manager.cpp @@ -19,10 +19,14 @@ std::shared_ptr service_manager::create_service( if (hit != cache_.end()) { auto service_ptr = hit->second.lock(); if (service_ptr) { // not expired + SPDLOG_DEBUG( + "Found an existing service for {}::{}", id.service, id.env); return service_ptr; } } + SPDLOG_DEBUG("Creating a service for {}::{}", id.service, id.env); + auto service_ptr = service::from_settings(service_identifier(id), settings, rc_settings, meta, metrics, dynamic_enablement); cache_.emplace(id, std::move(service_ptr)); diff --git a/tests/extension/rinit_rshutdown_basic.phpt b/tests/extension/rinit_rshutdown_basic.phpt index f2161deb6b1212b72a08210d4150d1787da9bed7..c7956b98e2e790c725778a3d80620248e2062fd2 100644 GIT binary patch delta 263 zcmcblxm9~ZFr%!COMFO>qqA$gyQ`mTkYk8ze2~9?NPKXBqo1w8<{-wcjFLL(sU`7g z`9-;jB_*jT@kOP1C7HRY@tG-;*_p4@ySRiDB_^lF6qh6xmBbepB<5*oT64iflXLQm zQz4=TKw%Y-yn?MlN=keQNLPGuYEfolPG(hVJXmE4SZOiHgtEk<_>|Jz0u8X9XoJ{j zrQFn#L?s{%b%AbXic+kCf~~CrE(dC8YFcl;#e9Ql@-i0j$qB3+o6oZ`@o^{R=jUjY K6qTlGasdE_16&#a delta 36 ucmV+<0NekyE7B>jQvtJa0ki>=J_FRVvIN%xlamP@li(2rv#kgH4FL)gXASiL diff --git a/tests/extension/runtime_id_without_ddtrace.phpt b/tests/extension/runtime_id_without_ddtrace.phpt new file mode 100644 index 0000000000..f248ed4dc1 --- /dev/null +++ b/tests/extension/runtime_id_without_ddtrace.phpt @@ -0,0 +1,10 @@ +--TEST-- +Runtime ID without DDTrace loaded +--FILE-- + +--EXPECTF-- +string(0) "" diff --git a/tests/helper/client_test.cpp b/tests/helper/client_test.cpp index ab1b5550c0..345c4c5d78 100644 --- a/tests/helper/client_test.cpp +++ b/tests/helper/client_test.cpp @@ -33,6 +33,19 @@ class service_manager : public dds::service_manager { bool dynamic_enablement), (override)); }; + +class service : public dds::service { +public: + service(std::shared_ptr engine, + std::shared_ptr service_config) + : dds::service(engine, service_config, {}) + {} + + MOCK_METHOD(void, register_runtime_id, (const std::string &id), (override)); + MOCK_METHOD( + void, unregister_runtime_id, (const std::string &id), (override)); +}; + } // namespace mock void send_client_init( @@ -136,6 +149,87 @@ TEST(ClientTest, ClientInit) EXPECT_EQ(msg_res->metrics[tag::event_rules_failed], 0.0); } +TEST(ClientTest, ClientInitRegisterRuntimeId) +{ + std::shared_ptr engine{engine::create()}; + auto service_config = std::make_shared(); + + auto service = std::make_shared(engine, service_config); + auto smanager = std::make_shared(); + auto broker = new mock::broker(); + + client c(smanager, std::unique_ptr(broker)); + + auto fn = create_sample_rules_ok(); + + network::client_init::request msg; + msg.pid = 1729; + msg.runtime_version = "1.0"; + msg.client_version = "2.0"; + msg.service.runtime_id = "thisisaruntimeid"; + msg.engine_settings.rules_file = fn; + + network::request req(std::move(msg)); + + std::shared_ptr res; + EXPECT_CALL(*broker, recv(_)).WillOnce(Return(req)); + EXPECT_CALL(*broker, + send(testing::An &>())) + .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); + + EXPECT_CALL(*smanager, create_service(_, _, _, _, _, true)) + .Times(1) + .WillOnce(Return(service)); + + std::string runtime_id; + EXPECT_CALL(*service, register_runtime_id(_)) + .Times(1) + .WillOnce(testing::SaveArg<0>(&runtime_id)); + + EXPECT_TRUE(c.run_client_init()); + EXPECT_STREQ(runtime_id.c_str(), "thisisaruntimeid"); +} + +TEST(ClientTest, ClientInitGeneratesRuntimeId) +{ + std::shared_ptr engine{engine::create()}; + auto service_config = std::make_shared(); + + auto service = std::make_shared(engine, service_config); + auto smanager = std::make_shared(); + auto broker = new mock::broker(); + + client c(smanager, std::unique_ptr(broker)); + + auto fn = create_sample_rules_ok(); + + network::client_init::request msg; + msg.pid = 1729; + msg.runtime_version = "1.0"; + msg.client_version = "2.0"; + msg.engine_settings.rules_file = fn; + + network::request req(std::move(msg)); + + std::shared_ptr res; + EXPECT_CALL(*broker, recv(_)).WillOnce(Return(req)); + EXPECT_CALL(*broker, + send(testing::An &>())) + .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); + + EXPECT_CALL(*smanager, create_service(_, _, _, _, _, true)) + .Times(1) + .WillOnce(Return(service)); + + std::string runtime_id; + EXPECT_CALL(*service, register_runtime_id(_)) + .Times(1) + .WillOnce(testing::SaveArg<0>(&runtime_id)); + + EXPECT_TRUE(c.run_client_init()); + EXPECT_STRNE(runtime_id.c_str(), ""); +} + TEST(ClientTest, ClientInitInvalidRules) { auto smanager = std::make_shared(); diff --git a/tests/helper/remote_config/client_handler_test.cpp b/tests/helper/remote_config/client_handler_test.cpp index 78523969bf..aefaf40f41 100644 --- a/tests/helper/remote_config/client_handler_test.cpp +++ b/tests/helper/remote_config/client_handler_test.cpp @@ -5,20 +5,12 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "../common.hpp" +#include "mocks.hpp" #include "remote_config/client_handler.hpp" namespace dds { namespace mock { -class client : public remote_config::client { -public: - client(service_identifier &&sid) - : remote_config::client(nullptr, std::move(sid), {}) - {} - ~client() = default; - MOCK_METHOD0(poll, bool()); - MOCK_METHOD0(is_remote_config_available, bool()); -}; class client_handler : public remote_config::client_handler { public: @@ -97,19 +89,6 @@ TEST_F(ClientHandlerTest, RuntimeIdIsNotGeneratedIfProvided) .runtime_id.c_str()); } -TEST_F(ClientHandlerTest, RuntimeIdIsGeneratedWhenNotProvided) -{ - id.runtime_id.clear(); - - EXPECT_TRUE(id.runtime_id.empty()); - auto client_handler = remote_config::client_handler::from_settings( - dds::service_identifier(id), settings, service_config, rc_settings, - engine, false); - EXPECT_FALSE(client_handler->get_client() - ->get_service_identifier() - .runtime_id.empty()); -} - TEST_F(ClientHandlerTest, AsmFeatureProductIsAddeWhenDynamicEnablement) { auto dynamic_enablement = true; @@ -181,8 +160,7 @@ TEST_F(ClientHandlerTest, ValidateRCThread) std::promise available_call_promise; auto available_call_future = available_call_promise.get_future(); - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique(sid); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(1) .WillOnce(DoAll(SignalCall(&available_call_promise), Return(true))); @@ -202,8 +180,8 @@ TEST_F(ClientHandlerTest, ValidateRCThread) TEST_F(ClientHandlerTest, WhenRcNotAvailableItKeepsDiscovering) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(2) .WillOnce(Return(false)) @@ -219,8 +197,8 @@ TEST_F(ClientHandlerTest, WhenRcNotAvailableItKeepsDiscovering) TEST_F(ClientHandlerTest, WhenPollFailsItGoesBackToDiscovering) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(2) .WillOnce(Return(true)) @@ -238,8 +216,8 @@ TEST_F(ClientHandlerTest, WhenPollFailsItGoesBackToDiscovering) TEST_F(ClientHandlerTest, WhenDiscoverFailsItStaysOnDiscovering) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(3) .WillOnce(Return(false)) @@ -257,8 +235,8 @@ TEST_F(ClientHandlerTest, WhenDiscoverFailsItStaysOnDiscovering) TEST_F(ClientHandlerTest, ItKeepsPollingWhileNoError) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(1) .WillOnce(Return(true)); @@ -286,8 +264,8 @@ TEST_F(ClientHandlerTest, ItDoesNotStartIfNoRcClientGiven) TEST_F(ClientHandlerTest, ItDoesNotGoOverMaxIfGivenInitialIntervalIsLower) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(3) .WillRepeatedly(Return(false)); @@ -307,8 +285,8 @@ TEST_F(ClientHandlerTest, ItDoesNotGoOverMaxIfGivenInitialIntervalIsLower) TEST_F(ClientHandlerTest, IfInitialIntervalIsHigherThanMaxItBecomesNewMax) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); EXPECT_CALL(*rc_client, is_remote_config_available) .Times(3) .WillRepeatedly(Return(false)); @@ -328,12 +306,26 @@ TEST_F(ClientHandlerTest, IfInitialIntervalIsHigherThanMaxItBecomesNewMax) TEST_F(ClientHandlerTest, ByDefaultMaxIntervalisFiveMinutes) { - auto rc_client = - std::make_unique(dds::service_identifier(sid)); + auto rc_client = std::make_unique( + dds::service_identifier(sid)); auto client_handler = mock::client_handler(std::move(rc_client), service_config, 200ms); EXPECT_EQ(5min, client_handler.get_max_interval()); } +TEST_F(ClientHandlerTest, RegisterAndUnregisterRuntimeID) +{ + auto rc_client = std::make_unique( + dds::service_identifier(sid)); + EXPECT_CALL(*rc_client, register_runtime_id).Times(1); + EXPECT_CALL(*rc_client, unregister_runtime_id).Times(1); + + auto client_handler = remote_config::client_handler( + std::move(rc_client), service_config, 200ms); + + client_handler.register_runtime_id("something"); + client_handler.unregister_runtime_id("something"); +} + } // namespace dds diff --git a/tests/helper/remote_config/client_test.cpp b/tests/helper/remote_config/client_test.cpp index bc0365f9f6..cc3fbefb79 100644 --- a/tests/helper/remote_config/client_test.cpp +++ b/tests/helper/remote_config/client_test.cpp @@ -446,6 +446,57 @@ TEST_F(RemoteConfigClient, ItCallsToApiOnPoll) sort_arrays(request_sent)); } +TEST_F(RemoteConfigClient, ReplaceRuntimeID) +{ + auto api = std::make_unique(); + std::string request_sent = ""; + EXPECT_CALL(*api, get_configs(_)) + .Times(AtLeast(1)) + .WillOnce(DoAll(testing::SaveArg<0>(&request_sent), + Return(generate_example_response(paths)))); + + service_identifier sid{ + service, extra_services, env, tracer_version, app_version, runtime_id}; + + dds::test_client api_client( + id, std::move(api), std::move(sid), std::move(settings), listeners_); + + // Unregister the old ID and register a new one + api_client.unregister_runtime_id(runtime_id); + runtime_id = "something else"; + api_client.register_runtime_id(runtime_id); + api_client.register_runtime_id("dummy"); + api_client.register_runtime_id("unused"); + api_client.register_runtime_id("irrelevant"); + + EXPECT_TRUE(api_client.poll()); + EXPECT_EQ(sort_arrays(generate_request_serialized(false, false)), + sort_arrays(request_sent)); +} + +TEST_F(RemoteConfigClient, RemoveRuntimeID) +{ + auto api = std::make_unique(); + std::string request_sent = ""; + EXPECT_CALL(*api, get_configs(_)) + .Times(AtLeast(1)) + .WillOnce(DoAll(testing::SaveArg<0>(&request_sent), + Return(generate_example_response(paths)))); + + service_identifier sid{ + service, extra_services, env, tracer_version, app_version, runtime_id}; + + dds::test_client api_client( + id, std::move(api), std::move(sid), std::move(settings), listeners_); + + // Unregister the ID, it should still be used + api_client.unregister_runtime_id(runtime_id); + + EXPECT_TRUE(api_client.poll()); + EXPECT_EQ(sort_arrays(generate_request_serialized(false, false)), + sort_arrays(request_sent)); +} + TEST_F(RemoteConfigClient, ItReturnErrorWhenApiNotProvided) { service_identifier sid{ diff --git a/tests/helper/remote_config/mocks.hpp b/tests/helper/remote_config/mocks.hpp index 010ebf2096..7d40d84c08 100644 --- a/tests/helper/remote_config/mocks.hpp +++ b/tests/helper/remote_config/mocks.hpp @@ -9,7 +9,9 @@ #include "../common.hpp" #include "base64.h" #include "engine.hpp" +#include "remote_config/client.hpp" #include "remote_config/config.hpp" +#include "service_identifier.hpp" namespace dds::remote_config::mock { @@ -28,6 +30,19 @@ class engine : public dds::engine { static auto create() { return std::shared_ptr(new engine()); } }; +class client : public remote_config::client { +public: + client(service_identifier sid) + : remote_config::client(nullptr, std::move(sid), {}) + {} + ~client() override = default; + MOCK_METHOD0(poll, bool()); + MOCK_METHOD0(is_remote_config_available, bool()); + MOCK_METHOD(void, register_runtime_id, (const std::string &id), (override)); + MOCK_METHOD( + void, unregister_runtime_id, (const std::string &id), (override)); +}; + inline remote_config::config generate_config( const std::string &product, const std::string &content, bool encode = true) { diff --git a/tests/helper/runtime_id_pool_test.cpp b/tests/helper/runtime_id_pool_test.cpp new file mode 100644 index 0000000000..8cfe8765ad --- /dev/null +++ b/tests/helper/runtime_id_pool_test.cpp @@ -0,0 +1,130 @@ +// Unless explicitly stated otherwise all files in this repository are +// dual-licensed under the Apache-2.0 License or BSD-3-Clause License. +// +// This product includes software developed at Datadog +// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. +#include "common.hpp" +#include "utils.hpp" +#include + +namespace dds { + +TEST(RuntimeIDPool, InvalidConstruction) +{ + EXPECT_THROW(runtime_id_pool ids{""}, std::invalid_argument); +} + +TEST(RuntimeIDPool, ConstructionAndGet) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); +} + +TEST(RuntimeIDPool, AddAndGet) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + + ids.add(generate_random_uuid()); + + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); +} + +TEST(RuntimeIDPool, RemoveLast) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + ids.remove(uuid); + + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); +} + +TEST(RuntimeIDPool, RemoveCurrent) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + + auto uuid2 = generate_random_uuid(); + ids.add(uuid2); + + ids.remove(uuid); + + EXPECT_STREQ(ids.get().c_str(), uuid2.c_str()); +} + +TEST(RuntimeIDPool, RemoveLastAndAdd) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + ids.remove(uuid); + + auto uuid2 = generate_random_uuid(); + ids.add(uuid2); + + EXPECT_STREQ(ids.get().c_str(), uuid2.c_str()); +} + +TEST(RuntimeIDPool, RemoveAddEmptyAndGet) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + ids.remove(uuid); + ids.add(""); + + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); +} + +TEST(RuntimeIDPool, RemoveDuplicateAndGet) +{ + auto uuid = generate_random_uuid(); + auto uuid2 = generate_random_uuid(); + runtime_id_pool ids{uuid}; + ids.add(uuid); + ids.add(uuid); + ids.add(uuid2); + + ids.remove(uuid); + + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); +} + +TEST(RuntimeIDPool, RemoveAllDuplicatesAndGet) +{ + auto uuid = generate_random_uuid(); + auto uuid2 = generate_random_uuid(); + runtime_id_pool ids{uuid}; + ids.add(uuid); + ids.add(uuid); + ids.add(uuid2); + + ids.remove(uuid); + ids.remove(uuid); + ids.remove(uuid); + + EXPECT_STREQ(ids.get().c_str(), uuid2.c_str()); +} + +TEST(RuntimeIDPool, AddManyAndRemove) +{ + auto uuid = generate_random_uuid(); + runtime_id_pool ids{uuid}; + + std::array uuid_array; + + for (auto i = 0; i < 20; ++i) { + uuid_array[i] = generate_random_uuid(); + ids.add(uuid_array[i]); + } + + for (auto i = 0; i < 20; ++i) { + ids.remove(uuid_array[i]); + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); + } + + ids.remove(uuid); + EXPECT_STREQ(ids.get().c_str(), uuid.c_str()); +} + +} // namespace dds diff --git a/tests/helper/service_manager_test.cpp b/tests/helper/service_manager_test.cpp index 90ae6c3e2c..565d6dcee2 100644 --- a/tests/helper/service_manager_test.cpp +++ b/tests/helper/service_manager_test.cpp @@ -108,4 +108,24 @@ TEST(ServiceManagerTest, BadRulesFile) }, dds::parsing_error); } + +TEST(ServiceManagerTest, UniqueServices) +{ + std::map meta; + std::map metrics; + + service_manager_exp manager; + auto fn = create_sample_rules_ok(); + dds::engine_settings engine_settings; + engine_settings.rules_file = fn; + + auto service1 = manager.create_service( + {"service", {}, "env", "1.0", "2.0", "runtime ID 0"}, engine_settings, + {}, meta, metrics, {}); + auto service2 = manager.create_service( + {"service", {}, "env", "1.1", "3.0", "runtime ID 1"}, engine_settings, + {}, meta, metrics, {}); + + EXPECT_EQ(service1.get(), service2.get()); +} } // namespace dds diff --git a/tests/helper/service_test.cpp b/tests/helper/service_test.cpp index 445579967f..b801ec6e44 100644 --- a/tests/helper/service_test.cpp +++ b/tests/helper/service_test.cpp @@ -4,30 +4,19 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "common.hpp" +#include "remote_config/mocks.hpp" #include #include #include namespace dds { -namespace mock { -class client : public remote_config::client { -public: - client(service_identifier &sid) - : remote_config::client(nullptr, std::move(sid), {}) - {} - ~client() = default; - MOCK_METHOD0(poll, bool()); - MOCK_METHOD0(is_remote_config_available, bool()); -}; -} // namespace mock - TEST(ServiceTest, NullEngine) { service_identifier sid{"service", {"extra01", "extra02"}, "env", "tracer_version", "app_version", "runtime_id"}; std::shared_ptr engine; - auto client = std::make_unique(sid); + auto client = std::make_unique(sid); EXPECT_CALL(*client, poll).Times(0); auto service_config = std::make_shared();