From cc02664aae87b9a987d7118a684a35376af105ca Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Fri, 12 Dec 2025 18:39:40 +0000 Subject: [PATCH 1/5] Update libdatadog --- libdatadog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libdatadog b/libdatadog index 629bce0954..259ca4bdd3 160000 --- a/libdatadog +++ b/libdatadog @@ -1 +1 @@ -Subproject commit 629bce09547abc77d7bbda623921f97eb5611949 +Subproject commit 259ca4bdd3b6ff997acdc648583e97caaff1e35a From 1a96912523e33a749032f759a84bd1c7a7a1b45f Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Fri, 12 Dec 2025 15:45:50 +0000 Subject: [PATCH 2/5] Submit worker count --- appsec/src/helper/client.cpp | 8 +- appsec/src/helper/client.hpp | 54 +++++- appsec/src/helper/service.cpp | 168 ++++++++++++++---- appsec/src/helper/service.hpp | 96 ++++++++-- appsec/src/helper/sidecar_settings.hpp | 5 +- appsec/src/helper/telemetry.hpp | 2 +- .../php/integration/TelemetryTests.groovy | 6 +- components-rs/sidecar.h | 28 +++ ddtrace.sym | 2 + 9 files changed, 315 insertions(+), 54 deletions(-) diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index 86211d2125..a2b558d031 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -195,7 +195,7 @@ bool client::handle_command(const network::client_init::request &command) response->status = has_errors ? "fail" : "ok"; response->errors = std::move(errors); - if (service_) { + if (!service_.is_empty()) { // may be null in testing collect_metrics(*response, *service_, context_); } @@ -218,7 +218,7 @@ bool client::handle_command(const network::client_init::request &command) template bool client::service_guard() { - if (!service_) { + if (service_.is_empty()) { // This implies a failed client_init, we can't continue. SPDLOG_DEBUG("no service available on {}", T::name); send_error_response(*broker_); @@ -349,7 +349,7 @@ bool client::compute_client_status() return request_enabled_; } - if (service_ == nullptr) { + if (service_.is_empty()) { request_enabled_ = false; return request_enabled_; } @@ -503,8 +503,6 @@ void client::update_settings( rc_settings.shmem_path = std::string{rc_path}; } - sidecar_settings const current_sc_settings = - service_->get_sidecar_settings(); std::shared_ptr new_service = service_manager_->get_or_create_service( *engine_settings_, rc_settings, telemetry_settings); diff --git a/appsec/src/helper/client.hpp b/appsec/src/helper/client.hpp index 2e3e22d855..c4473949d9 100644 --- a/appsec/src/helper/client.hpp +++ b/appsec/src/helper/client.hpp @@ -7,7 +7,6 @@ #include -#include "config.hpp" #include "engine.hpp" #include "network/broker.hpp" #include "network/proto.hpp" @@ -18,6 +17,55 @@ namespace dds { +class service_holder { +public: + service_holder() = default; + service_holder(const service_holder &) = delete; + service_holder &operator=(const service_holder &) = delete; + service_holder(service_holder &&) = delete; + service_holder &operator=(service_holder &&) = delete; + ~service_holder() + { + if (service_) { + service_->decrement_num_workers(); + } + } + auto operator->() const { return service_.get(); } + auto &operator*() const { return *service_; } + service_holder &operator=(std::shared_ptr new_service) + { + if (service_ == new_service) { + return *this; + } + if (service_) { + service_->decrement_num_workers(); + } + service_ = std::move(new_service); + if (service_) { + service_->increment_num_workers(); + } + return *this; + } + + void reset() + { + if (service_) { + service_->decrement_num_workers(); + } + service_.reset(); + } + + [[nodiscard]] bool is_empty() const { return service_ == nullptr; } + + [[nodiscard]] std::shared_ptr get_shared_ptr() const + { + return service_; + } + +private: + std::shared_ptr service_; +}; + class client { public: // Below this limit the encoding+compression might result on a longer string @@ -54,7 +102,7 @@ class client { [[nodiscard]] std::shared_ptr get_service() const { - return service_; + return service_.get_shared_ptr(); } // NOLINTNEXTLINE(google-runtime-references) @@ -93,7 +141,7 @@ class client { std::shared_ptr service_manager_; std::optional engine_settings_; - std::shared_ptr service_; + service_holder service_; std::unique_ptr sample_acc_; // depends on service_ std::optional context_; diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index 277332a82d..86aa064d70 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -5,6 +5,7 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "service.hpp" +#include "common.h" #include "sidecar_settings.hpp" #include @@ -25,8 +26,6 @@ extern "C" { using CharSlice = ddog_Slice_CChar; -namespace dds { - namespace { inline CharSlice to_ffi_string(std::string_view sv) { @@ -37,6 +36,12 @@ using ddog_sidecar_enqueue_telemetry_log_t = decltype(&ddog_sidecar_enqueue_telemetry_log); ddog_sidecar_enqueue_telemetry_log_t fn_ddog_sidecar_enqueue_telemetry_log; +using ddog_sidecard_enqueue_telemetry_point_t = decltype(&ddog_sidecar_enqueue_telemetry_point); +ddog_sidecard_enqueue_telemetry_point_t fn_ddog_sidecar_enqueue_telemetry_point; + +using ddog_sidecar_enqueue_telemetry_metric_t = decltype(&ddog_sidecar_enqueue_telemetry_metric); +ddog_sidecar_enqueue_telemetry_metric_t fn_ddog_sidecar_enqueue_telemetry_metric; + using ddog_Error_message_t = decltype(&ddog_Error_message); ddog_Error_message_t fn_ddog_Error_message; @@ -44,6 +49,7 @@ using ddog_Error_drop_t = decltype(&ddog_Error_drop); ddog_Error_drop_t fn_ddog_Error_drop; } // namespace +namespace dds { service::service(std::shared_ptr engine, std::shared_ptr service_config, std::unique_ptr client_handler, @@ -156,38 +162,136 @@ void service::metrics_impl::submit_log(const sidecar_settings &sc_settings, } } -void service::resolve_symbols() +void service::metrics_impl::submit_metric_ffi( + const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings, + std::string_view name, ddog_MetricType type) { - if (fn_ddog_sidecar_enqueue_telemetry_log == nullptr) { - fn_ddog_sidecar_enqueue_telemetry_log = - // NOLINTNEXTLINE - reinterpret_cast( - dlsym(RTLD_DEFAULT, "ddog_sidecar_enqueue_telemetry_log")); - if (fn_ddog_sidecar_enqueue_telemetry_log == nullptr) { - throw std::runtime_error{ - "Failed to resolve ddog_sidecar_enqueue_telemetry_log"}; - } - } - - if (fn_ddog_Error_message == nullptr) { - fn_ddog_Error_message = - // NOLINTNEXTLINE - reinterpret_cast( - dlsym(RTLD_DEFAULT, "ddog_Error_message")); - if (fn_ddog_Error_message == nullptr) { - throw std::runtime_error{"Failed to resolve ddog_Error_message"}; - } - } - - if (fn_ddog_Error_drop == nullptr) { - fn_ddog_Error_drop = - // NOLINTNEXTLINE - reinterpret_cast( - dlsym(RTLD_DEFAULT, "ddog_Error_drop")); - if (fn_ddog_Error_drop == nullptr) { - throw std::runtime_error{"Failed to resolve ddog_Error_drop"}; - } + SPDLOG_DEBUG("register_metric_ffi (ffi): name: {}, type: {}", name, type); + + if (fn_ddog_sidecar_enqueue_telemetry_metric == nullptr) { + throw std::runtime_error("Failed to resolve ddog_sidecar_enqueue_telemetry_metric"); } + ddog_MaybeError result = fn_ddog_sidecar_enqueue_telemetry_metric( + to_ffi_string(sc_settings.session_id), + to_ffi_string(sc_settings.runtime_id), + to_ffi_string(telemetry_settings.service_name), + to_ffi_string(telemetry_settings.env_name), + to_ffi_string(name), + type, + DDOG_METRIC_NAMESPACE_APPSEC + ); + + if (result.tag == DDOG_OPTION_ERROR_SOME_ERROR) { + ddog_CharSlice const error_msg = fn_ddog_Error_message(&result.some); + SPDLOG_INFO("Failed to register telemetry metric, error: {}", + std::string_view{error_msg.ptr, error_msg.len}); + fn_ddog_Error_drop(&result.some); + } +} + +void service::metrics_impl::submit_metric_point_ffi( + const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings, + std::string_view name, + double value, + std::optional tags) +{ + SPDLOG_DEBUG( + "enqueue_metric_point_ffi (ffi): name: {}, value: {}, tags: {}", name, + value, tags.has_value() ? tags.value() : "(none)"sv); + + if (fn_ddog_sidecar_enqueue_telemetry_point == nullptr) { + throw std::runtime_error("Failed to resolve ddog_sidecar_enqueue_telemetry_point"); + } + CharSlice tags_ffi; + CharSlice *tags_ffi_ptr = nullptr; + if (tags.has_value()) { + tags_ffi = to_ffi_string(*tags); + tags_ffi_ptr = &tags_ffi; + } + ddog_MaybeError result = fn_ddog_sidecar_enqueue_telemetry_point( + to_ffi_string(sc_settings.session_id), + to_ffi_string(sc_settings.runtime_id), + to_ffi_string(telemetry_settings.service_name), + to_ffi_string(telemetry_settings.env_name), + to_ffi_string(name), + value, + tags_ffi_ptr + ); + + if (result.tag == DDOG_OPTION_ERROR_SOME_ERROR) { + ddog_CharSlice const error_msg = fn_ddog_Error_message(&result.some); + SPDLOG_INFO("Failed to enqueue telemetry point, error: {}", + std::string_view{error_msg.ptr, error_msg.len}); + fn_ddog_Error_drop(&result.some); + } +} + +void service::handle_worker_count_metrics(const sidecar_settings &sc_settings) +{ + static constexpr std::string_view METRIC_NAME = + "helper.service_worker_count"; + + auto cur_st = num_workers_.load(std::memory_order_relaxed); + if (cur_st.latest_count_sent) { + return; + } + + auto new_st = cur_st; + if (!cur_st.metrics_registered) { + // it's not an error to register the metrics more than once + msubmitter_->submit_metric_ffi( + sc_settings, + telemetry_settings_, + METRIC_NAME, + DDOG_METRIC_TYPE_GAUGE + ); + new_st.metrics_registered = true; + } + + if (!cur_st.latest_count_sent) { + msubmitter_->submit_metric_point_ffi(sc_settings, telemetry_settings_, + METRIC_NAME, static_cast(cur_st.count), std::nullopt); + + new_st.latest_count_sent = true; + } + + if (cur_st == new_st) { + return; + } + + bool success = num_workers_.compare_exchange_weak( + cur_st, new_st, std::memory_order_relaxed); + + // if the CAS failed, it was because the count changed, so the metric + // point will need to be submitted again: don't mark latest_count_sent + if (!success) { + SPDLOG_DEBUG("Worker count changed while submitting metric point; " + "latest metric is not up to date"); + } +} + +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) +#define RESOLVE_FFI_SYMBOL(symbol_name) \ + do { \ + if (fn_##symbol_name == nullptr) { \ + fn_##symbol_name = \ + reinterpret_cast(/* NOLINT */ \ + dlsym(RTLD_DEFAULT, #symbol_name)); \ + if (fn_##symbol_name == nullptr) { \ + throw std::runtime_error{"Failed to resolve " #symbol_name}; \ + } \ + } \ + } while (0) + +void service::resolve_symbols() +{ + RESOLVE_FFI_SYMBOL(ddog_sidecar_enqueue_telemetry_log); + RESOLVE_FFI_SYMBOL(ddog_sidecar_enqueue_telemetry_point); + RESOLVE_FFI_SYMBOL(ddog_sidecar_enqueue_telemetry_metric); + RESOLVE_FFI_SYMBOL(ddog_Error_message); + RESOLVE_FFI_SYMBOL(ddog_Error_drop); } } // namespace dds diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index 21c5d831dd..43a3253970 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -5,6 +5,7 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #pragma once +#include "common.h" // components-rs/common.h #include "engine.hpp" #include "remote_config/client_handler.hpp" #include "sampler.hpp" @@ -123,7 +124,7 @@ class service { logs.swap(pending_logs_); } for (auto &log : logs) { - submit_log(sc_settings, telemetry_settings, std::move(log)); + submit_log(sc_settings, telemetry_settings, log); } } @@ -140,9 +141,19 @@ class service { } private: + friend class service; + static void submit_log(const sidecar_settings &sc_settings, const telemetry_settings &telemetry_settings, const tel_log &log); + static void submit_metric_ffi(const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings, std::string_view name, + ddog_MetricType type); + + static void submit_metric_point_ffi(const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings, std::string_view name, + double value, std::optional tags); + std::vector pending_metrics_; std::mutex pending_metrics_mutex_; std::vector pending_logs_; @@ -204,12 +215,7 @@ class service { return service_config_; } - [[nodiscard]] const sidecar_settings &get_sidecar_settings() const - { - return sidecar_settings_; - } - - [[nodiscard]] bool schema_extraction_enabled() + [[nodiscard]] bool schema_extraction_enabled() const { return schema_extraction_enabled_; } @@ -250,6 +256,10 @@ class service { void drain_logs(const sidecar_settings &sc_settings) { msubmitter_->drain_logs(sc_settings, telemetry_settings_); + + // take this opportunity to submit internal worker count metrics too + // this could be done at any other time when sc_settings is available + handle_worker_count_metrics(sc_settings); } [[nodiscard]] std::map drain_legacy_metrics() @@ -271,17 +281,81 @@ class service { "remote_config.requests_before_running"sv, 1, {}); } } + + void increment_num_workers() { + auto cur = num_workers_.load(std::memory_order_relaxed); + while (true) { + auto new_v = num_workers_t{ + .metrics_registered = cur.metrics_registered, + .latest_count_sent = false, + .count = cur.count + 1}; + if (num_workers_.compare_exchange_weak(cur, new_v, std::memory_order_relaxed)) { + break; + } + } + } + void decrement_num_workers() { + auto cur = num_workers_.load(std::memory_order_relaxed); + if (cur.count == 0) { + SPDLOG_WARN("Attempted to decrement num_workers_ below 0"); + return; + } + while (true) { + auto new_v = num_workers_t{ + .metrics_registered = cur.metrics_registered, + .latest_count_sent = cur.latest_count_sent, + .count = cur.count - 1}; + if (num_workers_.compare_exchange_weak(cur, new_v, std::memory_order_relaxed)) { + break; + } + } + } protected: - std::shared_ptr engine_{}; - std::shared_ptr service_config_{}; - std::unique_ptr client_handler_{}; + std::shared_ptr engine_; + std::shared_ptr service_config_; + std::unique_ptr client_handler_; bool schema_extraction_enabled_; std::optional schema_sampler_; std::string rc_path_; telemetry_settings telemetry_settings_; std::shared_ptr msubmitter_; - sidecar_settings sidecar_settings_; + + struct num_workers_t { + bool metrics_registered: 1; + bool latest_count_sent: 1; + uint64_t count: 62; + constexpr bool operator==(const num_workers_t &other) const + { + return count == other.count && + metrics_registered == other.metrics_registered && + latest_count_sent == other.latest_count_sent; + } + }; + // required for value-initialization in default constructor of + // std::atomic in C++20 + static_assert(std::is_default_constructible_v); + std::atomic num_workers_; + void handle_worker_count_metrics(const sidecar_settings &sc_settings); }; } // namespace dds + +template <> struct fmt::formatter { + // NOLINTNEXTLINE + constexpr auto parse(fmt::format_parse_context &ctx) { return ctx.begin(); } + template + auto format( + ddog_MetricType type, FormatContext &ctx) const -> decltype(ctx.out()) + { + switch (type) { + case DDOG_METRIC_TYPE_GAUGE: + return fmt::format_to(ctx.out(), "GAUGE"); + case DDOG_METRIC_TYPE_COUNT: + return fmt::format_to(ctx.out(), "COUNT"); + case DDOG_METRIC_TYPE_DISTRIBUTION: + return fmt::format_to(ctx.out(), "DISTRIBUTION"); + } + return fmt::format_to(ctx.out(), "UNKNOWN"); + } +}; diff --git a/appsec/src/helper/sidecar_settings.hpp b/appsec/src/helper/sidecar_settings.hpp index 36e33dd15e..5f6f0ab363 100644 --- a/appsec/src/helper/sidecar_settings.hpp +++ b/appsec/src/helper/sidecar_settings.hpp @@ -16,7 +16,10 @@ struct sidecar_settings { std::string session_id; std::string runtime_id; - bool is_valid() const { return !session_id.empty() && !runtime_id.empty(); } + [[nodiscard]] bool is_valid() const + { + return !session_id.empty() && !runtime_id.empty(); + } bool operator==(const sidecar_settings &other) const { diff --git a/appsec/src/helper/telemetry.hpp b/appsec/src/helper/telemetry.hpp index 02b01c8998..1d240e7656 100644 --- a/appsec/src/helper/telemetry.hpp +++ b/appsec/src/helper/telemetry.hpp @@ -83,7 +83,7 @@ struct fmt::formatter : fmt::formatter { auto format( - const dds::telemetry::telemetry_tags tags, format_context &ctx) const + const dds::telemetry::telemetry_tags& tags, format_context &ctx) const { return formatter::format( std::string_view{tags.data_}, ctx); diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy index 67da553a11..10fed9aaff 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy @@ -95,16 +95,20 @@ class TelemetryTests { TelemetryHelpers.Metric wafInit TelemetryHelpers.Metric wafReq1 TelemetryHelpers.Metric wafReq2 + TelemetryHelpers.Metric workerCount waitForMetrics(30) { List messages -> def allSeries = messages.collectMany { it.series } wafInit = allSeries.find { it.name == 'waf.init' } wafReq1 = allSeries.find { it.name == 'waf.requests' && it.tags.size() == 2 } wafReq2 = allSeries.find { it.name == 'waf.requests' && it.tags.size() == 3 } + workerCount = allSeries.find { it.name == 'helper.service_worker_count' } - wafInit && wafReq1 && wafReq2 + wafInit && wafReq1 && wafReq2 && workerCount } + assert workerCount != null + assert wafInit != null assert wafInit.namespace == 'appsec' assert wafInit.points[0][1] == 1.0 diff --git a/components-rs/sidecar.h b/components-rs/sidecar.h index 4746e0d216..26bfa8df6b 100644 --- a/components-rs/sidecar.h +++ b/components-rs/sidecar.h @@ -208,6 +208,34 @@ ddog_MaybeError ddog_sidecar_enqueue_telemetry_log(ddog_CharSlice session_id_ffi ddog_CharSlice *tags_ffi, bool is_sensitive); +/** + * Enqueues a telemetry point to be processed internally. + * + * # Safety + * Pointers must be valid, strings must be null-terminated if not null. + */ +ddog_MaybeError ddog_sidecar_enqueue_telemetry_point(ddog_CharSlice session_id_ffi, + ddog_CharSlice runtime_id_ffi, + ddog_CharSlice service_name_ffi, + ddog_CharSlice env_name_ffi, + ddog_CharSlice metric_name_ffi, + double value, + ddog_CharSlice *tags_ffi); + +/** + * Registers a telemetry metric to be processed internally. + * + * # Safety + * Pointers must be valid, strings must be null-terminated if not null. + */ +ddog_MaybeError ddog_sidecar_enqueue_telemetry_metric(ddog_CharSlice session_id_ffi, + ddog_CharSlice runtime_id_ffi, + ddog_CharSlice service_name_ffi, + ddog_CharSlice env_name_ffi, + ddog_CharSlice metric_name_ffi, + enum ddog_MetricType metric_type, + enum ddog_MetricNamespace metric_namespace); + /** * Sends a trace to the sidecar via shared memory. */ diff --git a/ddtrace.sym b/ddtrace.sym index 4e24ed6465..f38647c79b 100644 --- a/ddtrace.sym +++ b/ddtrace.sym @@ -36,3 +36,5 @@ ddog_Error_message ddog_Error_drop ddog_library_configurator_drop ddog_sidecar_enqueue_telemetry_log +ddog_sidecar_enqueue_telemetry_point +ddog_sidecar_enqueue_telemetry_metric From c0cee56ae035363a9e06ccc55f82f63380d733c2 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Fri, 12 Dec 2025 17:15:02 +0000 Subject: [PATCH 3/5] appsec: avoid sending metrics to the extension and back --- appsec/src/extension/commands_helpers.c | 15 +---- appsec/src/helper/client.cpp | 31 +++++---- appsec/src/helper/metrics.hpp | 52 ++++++++++++++- appsec/src/helper/service.cpp | 55 +++++++--------- appsec/src/helper/service.hpp | 64 ++++++++++++++----- appsec/src/helper/telemetry.hpp | 1 - .../php/integration/TelemetryTests.groovy | 6 +- 7 files changed, 147 insertions(+), 77 deletions(-) diff --git a/appsec/src/extension/commands_helpers.c b/appsec/src/extension/commands_helpers.c index a2e1b56ba4..d9166cc2c8 100644 --- a/appsec/src/extension/commands_helpers.c +++ b/appsec/src/extension/commands_helpers.c @@ -837,6 +837,9 @@ bool dd_command_process_telemetry_metrics(mpack_node_t metrics) if (mpack_node_error(metrics) != mpack_ok) { break; } + + // enrich tags for waf.requests with input_truncated=true if the + // data is truncated if (dd_msgpack_helpers_is_data_truncated() && WAF_REQUEST_METRIC_LEN == key_len && memcmp(WAF_REQUEST_METRIC, key_str, WAF_REQUEST_METRIC_LEN) == @@ -905,18 +908,6 @@ void _handle_telemetry_metric(const char *nonnull key_str, size_t key_len, } while (0) HANDLE_METRIC("waf.requests", DDTRACE_METRIC_TYPE_COUNT); - HANDLE_METRIC("waf.updates", DDTRACE_METRIC_TYPE_COUNT); - HANDLE_METRIC("waf.init", DDTRACE_METRIC_TYPE_COUNT); - HANDLE_METRIC("waf.config_errors", DDTRACE_METRIC_TYPE_COUNT); - - HANDLE_METRIC("remote_config.first_pull", DDTRACE_METRIC_TYPE_GAUGE); - HANDLE_METRIC("remote_config.last_success", DDTRACE_METRIC_TYPE_GAUGE); - - // Rasp - HANDLE_METRIC("rasp.timeout", DDTRACE_METRIC_TYPE_COUNT); - HANDLE_METRIC("rasp.rule.match", DDTRACE_METRIC_TYPE_COUNT); - HANDLE_METRIC("rasp.rule.eval", DDTRACE_METRIC_TYPE_COUNT); - HANDLE_METRIC("rasp.error", DDTRACE_METRIC_TYPE_COUNT); mlog_g(dd_log_info, "Unknown telemetry metric %.*s", (int)key_len, key_str); } diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index a2b558d031..84e6ebb0d6 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -13,12 +13,12 @@ #include "action.hpp" #include "client.hpp" #include "exception.hpp" -#include "metrics.hpp" #include "network/broker.hpp" #include "network/proto.hpp" #include "service.hpp" #include "service_config.hpp" #include "std_logging.hpp" +#include "telemetry.hpp" using namespace std::chrono_literals; @@ -27,9 +27,9 @@ namespace dds { namespace { void collect_metrics(network::request_shutdown::response &response, - service &service, std::optional &context); + service &service, std::optional &context, const sidecar_settings &sc_settings); void collect_metrics(network::client_init::response &response, service &service, - std::optional &context); + std::optional &context, const sidecar_settings &sc_settings); template // NOLINTNEXTLINE(google-runtime-references) @@ -197,7 +197,7 @@ bool client::handle_command(const network::client_init::request &command) if (!service_.is_empty()) { // may be null in testing - collect_metrics(*response, *service_, context_); + collect_metrics(*response, *service_, context_, sc_settings_); } try { @@ -470,7 +470,7 @@ bool client::handle_command(network::request_shutdown::request &command) return false; } - collect_metrics(*response, *service_, context_); + collect_metrics(*response, *service_, context_, sc_settings_); service_->drain_logs(sc_settings_); return send_message(response); @@ -602,13 +602,22 @@ struct request_metrics_submitter : public telemetry::telemetry_submitter { template void collect_metrics_impl(Response &response, service &service, - std::optional &context) + std::optional &context, const sidecar_settings &sc_settings) { request_metrics_submitter msubmitter{}; if (context) { context->get_metrics(msubmitter); } - service.drain_metrics( + + auto request_metrics = std::move(msubmitter.tel_metrics); + for (auto &metric : request_metrics) { + for (auto &[value, tags] : metric.second) { + service.submit_request_metric(metric.first, value, + telemetry::telemetry_tags::from_string(std::move(tags))); + } + } + service.drain_metrics(sc_settings, + // metrics for the extension are put back into the msubmitter [&msubmitter](std::string_view name, double value, auto tags) { msubmitter.submit_metric(name, value, std::move(tags)); }); @@ -619,14 +628,14 @@ void collect_metrics_impl(Response &response, service &service, response.metrics = std::move(msubmitter.metrics); } void collect_metrics(network::request_shutdown::response &response, - service &service, std::optional &context) + service &service, std::optional &context, const sidecar_settings &sc_settings) { - collect_metrics_impl(response, service, context); + collect_metrics_impl(response, service, context, sc_settings); } void collect_metrics(network::client_init::response &response, service &service, - std::optional &context) + std::optional &context, const sidecar_settings &sc_settings) { - collect_metrics_impl(response, service, context); + collect_metrics_impl(response, service, context, sc_settings); } } // namespace diff --git a/appsec/src/helper/metrics.hpp b/appsec/src/helper/metrics.hpp index 08ccf9d347..3d3558291f 100644 --- a/appsec/src/helper/metrics.hpp +++ b/appsec/src/helper/metrics.hpp @@ -6,6 +6,7 @@ #pragma once +#include // components-rs/common.h #include #include @@ -17,11 +18,14 @@ constexpr std::string_view waf_updates = "waf.updates"; constexpr std::string_view waf_requests = "waf.requests"; constexpr std::string_view waf_config_errors = "waf.config_errors"; -// not implemented: +// implemented in the extension constexpr std::string_view waf_input_truncated = "waf.input_truncated"; +// not implemented constexpr std::string_view waf_truncated_value_size = "waf.truncated_value_size"; +// not implemented constexpr std::string_view waf_duration_tel = "waf.duration"; +// implemented in the extension constexpr std::string_view waf_duration_ext = "waf.duration_ext"; // not implemented (difficult to count requests on the helper) @@ -48,4 +52,50 @@ constexpr std::string_view telemetry_rasp_error = "rasp.error"; constexpr std::string_view telemetry_rasp_rule_match = "rasp.rule.match"; constexpr std::string_view telemetry_rasp_timeout = "rasp.timeout"; +// telemetry +constexpr std::string_view helper_worker_count = "helper.service_worker_count"; + +struct known_tel_metrics { + std::string_view name; + ddog_MetricType type; +}; + +static constexpr std::array known_metrics = { + known_tel_metrics{ + .name = waf_requests, + .type = DDOG_METRIC_TYPE_COUNT, + }, + known_tel_metrics{ + .name = waf_updates, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = waf_init, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = waf_config_errors, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = telemetry_rasp_timeout, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = telemetry_rasp_rule_match, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = telemetry_rasp_rule_eval, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = telemetry_rasp_error, + .type = DDOG_METRIC_TYPE_COUNT, + }, + { + .name = helper_worker_count, + .type = DDOG_METRIC_TYPE_GAUGE, + }, +}; } // namespace dds::metrics diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index 86aa064d70..7a702f875e 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -5,8 +5,9 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "service.hpp" -#include "common.h" +#include "metrics.hpp" #include "sidecar_settings.hpp" +#include #include @@ -162,12 +163,12 @@ void service::metrics_impl::submit_log(const sidecar_settings &sc_settings, } } -void service::metrics_impl::submit_metric_ffi( +void service::metrics_impl::register_metric_ffi( const sidecar_settings &sc_settings, const telemetry_settings &telemetry_settings, std::string_view name, ddog_MetricType type) { - SPDLOG_DEBUG("register_metric_ffi (ffi): name: {}, type: {}", name, type); + SPDLOG_TRACE("register_metric_ffi: name: {}, type: {}", name, type); if (fn_ddog_sidecar_enqueue_telemetry_metric == nullptr) { throw std::runtime_error("Failed to resolve ddog_sidecar_enqueue_telemetry_metric"); @@ -190,15 +191,15 @@ void service::metrics_impl::submit_metric_ffi( } } -void service::metrics_impl::submit_metric_point_ffi( +void service::metrics_impl::submit_metric_ffi( const sidecar_settings &sc_settings, const telemetry_settings &telemetry_settings, std::string_view name, double value, std::optional tags) { - SPDLOG_DEBUG( - "enqueue_metric_point_ffi (ffi): name: {}, value: {}, tags: {}", name, + SPDLOG_TRACE( + "submit_metric_ffi: name: {}, value: {}, tags: {}", name, value, tags.has_value() ? tags.value() : "(none)"sv); if (fn_ddog_sidecar_enqueue_telemetry_point == nullptr) { @@ -228,44 +229,32 @@ void service::metrics_impl::submit_metric_point_ffi( } } -void service::handle_worker_count_metrics(const sidecar_settings &sc_settings) +void service::register_known_metrics(const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings) { - static constexpr std::string_view METRIC_NAME = - "helper.service_worker_count"; + for (auto &&metric : metrics::known_metrics) { + msubmitter_->register_metric_ffi( + sc_settings, telemetry_settings, metric.name, metric.type); + } +} +void service::handle_worker_count_metrics(const sidecar_settings &sc_settings) +{ auto cur_st = num_workers_.load(std::memory_order_relaxed); if (cur_st.latest_count_sent) { return; } - auto new_st = cur_st; - if (!cur_st.metrics_registered) { - // it's not an error to register the metrics more than once - msubmitter_->submit_metric_ffi( - sc_settings, - telemetry_settings_, - METRIC_NAME, - DDOG_METRIC_TYPE_GAUGE - ); - new_st.metrics_registered = true; - } - - if (!cur_st.latest_count_sent) { - msubmitter_->submit_metric_point_ffi(sc_settings, telemetry_settings_, - METRIC_NAME, static_cast(cur_st.count), std::nullopt); - - new_st.latest_count_sent = true; - } + msubmitter_->submit_metric_ffi(sc_settings, telemetry_settings_, + metrics::helper_worker_count, static_cast(cur_st.count), + std::nullopt); - if (cur_st == new_st) { - return; - } + auto new_st = cur_st; + new_st.latest_count_sent = true; - bool success = num_workers_.compare_exchange_weak( + bool success = num_workers_.compare_exchange_strong( cur_st, new_st, std::memory_order_relaxed); - // if the CAS failed, it was because the count changed, so the metric - // point will need to be submitted again: don't mark latest_count_sent if (!success) { SPDLOG_DEBUG("Worker count changed while submitting metric point; " "latest metric is not up to date"); diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index 43a3253970..de26e8bd60 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -5,13 +5,14 @@ // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #pragma once -#include "common.h" // components-rs/common.h #include "engine.hpp" +#include "metrics.hpp" #include "remote_config/client_handler.hpp" #include "sampler.hpp" #include "service_config.hpp" #include "sidecar_settings.hpp" #include "telemetry_settings.hpp" +#include // components-rs/common.h #include #include #include @@ -20,7 +21,7 @@ namespace dds { using namespace std::chrono_literals; -using sampler = timed_set<4096, 8192>; +using sampler = timed_set<4096, 8192>; // NOLINT class service { protected: @@ -102,7 +103,14 @@ class service { std::move(stack_trace), std::move(tags), is_sensitive}); } - template void drain_metrics(Func &&func) + + private: + friend class service; + + template + void drain_metrics(const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings, + Func &&handle_metric_for_extension) { std::vector metrics; { @@ -110,8 +118,17 @@ class service { metrics.swap(pending_metrics_); } for (auto &metric : metrics) { - std::invoke(std::forward(func), metric.name, metric.value, - std::move(metric.tags)); + // waf.requests must be handled by extension; modifies the tags + // TODO: the extension could send the information in the + // message so we could add the tag ourselves. That would allow + // some code to be removed from the extension + if (metric.name == dds::metrics::waf_requests) { + std::invoke(std::forward(handle_metric_for_extension), + metric.name, metric.value, std::move(metric.tags)); + } else { + submit_metric_ffi(sc_settings, telemetry_settings, + metric.name, metric.value, metric.tags.consume()); + } } } @@ -140,17 +157,14 @@ class service { return std::move(meta_); } - private: - friend class service; - static void submit_log(const sidecar_settings &sc_settings, const telemetry_settings &telemetry_settings, const tel_log &log); - static void submit_metric_ffi(const sidecar_settings &sc_settings, + static void register_metric_ffi(const sidecar_settings &sc_settings, const telemetry_settings &telemetry_settings, std::string_view name, ddog_MetricType type); - static void submit_metric_point_ffi(const sidecar_settings &sc_settings, + static void submit_metric_ffi(const sidecar_settings &sc_settings, const telemetry_settings &telemetry_settings, std::string_view name, double value, std::optional tags); @@ -184,6 +198,9 @@ class service { new service(std::forward(args)...)); } + void register_known_metrics(const sidecar_settings &sc_settings, + const telemetry_settings &telemetry_settings); + public: service(const service &) = delete; service &operator=(const service &) = delete; @@ -248,9 +265,25 @@ class service { void notify_of_rc_updates() { client_handler_->poll(); } - template void drain_metrics(Func &&func) + void submit_request_metric( + std::string_view metric_name, double value, telemetry::telemetry_tags tags + ) { + msubmitter_->submit_metric(metric_name, value, std::move(tags)); + } + + template + void drain_metrics( + const sidecar_settings &sc_settings, + Func &&handle_metric_for_extension) { - msubmitter_->drain_metrics(std::forward(func)); + auto registered = metrics_registered_.load(std::memory_order_relaxed); + if (!registered && metrics_registered_.compare_exchange_strong( + registered, true, std::memory_order_relaxed)) { + register_known_metrics(sc_settings, telemetry_settings_); + } + + msubmitter_->drain_metrics(sc_settings, telemetry_settings_, + std::forward(handle_metric_for_extension)); } void drain_logs(const sidecar_settings &sc_settings) @@ -286,7 +319,6 @@ class service { auto cur = num_workers_.load(std::memory_order_relaxed); while (true) { auto new_v = num_workers_t{ - .metrics_registered = cur.metrics_registered, .latest_count_sent = false, .count = cur.count + 1}; if (num_workers_.compare_exchange_weak(cur, new_v, std::memory_order_relaxed)) { @@ -302,7 +334,6 @@ class service { } while (true) { auto new_v = num_workers_t{ - .metrics_registered = cur.metrics_registered, .latest_count_sent = cur.latest_count_sent, .count = cur.count - 1}; if (num_workers_.compare_exchange_weak(cur, new_v, std::memory_order_relaxed)) { @@ -319,16 +350,15 @@ class service { std::optional schema_sampler_; std::string rc_path_; telemetry_settings telemetry_settings_; + std::atomic metrics_registered_; std::shared_ptr msubmitter_; struct num_workers_t { - bool metrics_registered: 1; bool latest_count_sent: 1; - uint64_t count: 62; + uint64_t count: 63; constexpr bool operator==(const num_workers_t &other) const { return count == other.count && - metrics_registered == other.metrics_registered && latest_count_sent == other.latest_count_sent; } }; diff --git a/appsec/src/helper/telemetry.hpp b/appsec/src/helper/telemetry.hpp index 1d240e7656..a3d1f94b12 100644 --- a/appsec/src/helper/telemetry.hpp +++ b/appsec/src/helper/telemetry.hpp @@ -6,7 +6,6 @@ #include namespace dds::telemetry { - class telemetry_tags { public: telemetry_tags &add(std::string_view key, std::string_view value) diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy index 10fed9aaff..84b5fb7302 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy @@ -107,8 +107,6 @@ class TelemetryTests { wafInit && wafReq1 && wafReq2 && workerCount } - assert workerCount != null - assert wafInit != null assert wafInit.namespace == 'appsec' assert wafInit.points[0][1] == 1.0 @@ -127,6 +125,10 @@ class TelemetryTests { assert wafReq2 != null assert 'rule_triggered:true' in wafReq2.tags assert wafReq2.points[0][1] >= 1.0 + + assert workerCount != null + assert workerCount.namespace == 'appsec' + assert workerCount.points[0][1] >= 1.0 } @Test From 5a8f0d9cd2cf54fbabdb43cf33e96915b22b7837 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Fri, 12 Dec 2025 17:48:12 +0000 Subject: [PATCH 4/5] Remove extension submission of tel metrics gotten from helper --- appsec/src/extension/commands/client_init.c | 7 - .../src/extension/commands/request_shutdown.c | 5 +- appsec/src/extension/commands_helpers.c | 132 ------------------ appsec/src/extension/commands_helpers.h | 1 - appsec/src/helper/client.cpp | 47 +++---- appsec/src/helper/engine.hpp | 3 + appsec/src/helper/network/proto.hpp | 13 +- appsec/src/helper/service.hpp | 33 ++--- 8 files changed, 42 insertions(+), 199 deletions(-) diff --git a/appsec/src/extension/commands/client_init.c b/appsec/src/extension/commands/client_init.c index c17fbbe199..7ef8be6fc5 100644 --- a/appsec/src/extension/commands/client_init.c +++ b/appsec/src/extension/commands/client_init.c @@ -208,13 +208,6 @@ static void _process_meta_and_metrics( mpack_node_t metrics = mpack_node_array_at(root, 4); dd_command_process_metrics(metrics, span); - - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - if (mpack_node_array_length(root) >= 6) { - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - mpack_node_t tel_metrics = mpack_node_array_at(root, 5); - dd_command_process_telemetry_metrics(tel_metrics); - } } static dd_result _check_helper_version(mpack_node_t root) diff --git a/appsec/src/extension/commands/request_shutdown.c b/appsec/src/extension/commands/request_shutdown.c index 96ee82315e..c28c379ebf 100644 --- a/appsec/src/extension/commands/request_shutdown.c +++ b/appsec/src/extension/commands/request_shutdown.c @@ -28,7 +28,7 @@ static const char *nullable _header_content_type_zend_array( static const dd_command_spec _spec = { .name = "request_shutdown", .name_len = sizeof("request_shutdown") - 1, - .num_args = 3, // a map, api sec sampling key, and sidecar queue id + .num_args = 4, // a map, api sec sampling key, sidecar queue id, and input_truncated .outgoing_cb = _request_pack, .incoming_cb = dd_command_proc_resp_verd_span_data, .config_features_cb = dd_command_process_config_features_unexpected, @@ -101,6 +101,9 @@ static dd_result _request_pack(mpack_writer_t *nonnull w, void *nonnull ctx) // 3. mpack_write(w, dd_trace_get_sidecar_queue_id()); + // 4. + mpack_write_bool(w, dd_msgpack_helpers_is_data_truncated()); + return dd_success; } diff --git a/appsec/src/extension/commands_helpers.c b/appsec/src/extension/commands_helpers.c index d9166cc2c8..ad892108b1 100644 --- a/appsec/src/extension/commands_helpers.c +++ b/appsec/src/extension/commands_helpers.c @@ -18,13 +18,6 @@ #include #include -static const char WAF_REQUEST_METRIC[] = "waf.requests"; -static const size_t WAF_REQUEST_METRIC_LEN = sizeof(WAF_REQUEST_METRIC) - 1; -static const char TRUNCATED_TAG[] = "input_truncated=true"; -static const size_t TRUNCATED_TAG_LEN = sizeof(TRUNCATED_TAG); -static const char TAG_SEPARATOR = ','; -static const size_t TAG_SEPARATOR_LEN = sizeof(TAG_SEPARATOR); - typedef struct _dd_omsg { zend_llist iovecs; mpack_writer_t writer; @@ -508,8 +501,6 @@ static void dd_command_process_settings(mpack_node_t root); * 2: [force keep: bool] * 3: [meta: map] * 4: [metrics: map] - * 5: [telemetry metrics: map string -> - * array(array(value: double, tags: string)]) * ) */ #define RESP_INDEX_ACTION_PARAMS 0 @@ -518,7 +509,6 @@ static void dd_command_process_settings(mpack_node_t root); #define RESP_INDEX_SETTINGS 3 #define RESP_INDEX_SPAN_META 4 #define RESP_INDEX_SPAN_METRICS 5 -#define RESP_INDEX_TELEMETRY_METRICS 6 dd_result dd_command_proc_resp_verd_span_data( mpack_node_t root, void *unspecnull _ctx) @@ -557,11 +547,6 @@ dd_result dd_command_proc_resp_verd_span_data( dd_command_process_metrics(metrics, span); } - if (mpack_node_array_length(root) >= RESP_INDEX_TELEMETRY_METRICS + 1) { - dd_command_process_telemetry_metrics( - mpack_node_array_at(root, RESP_INDEX_TELEMETRY_METRICS)); - } - return res; } @@ -795,123 +780,6 @@ bool dd_command_process_metrics(mpack_node_t root, zend_object *nonnull span) return true; } -static void _handle_telemetry_metric(const char *nonnull key_str, - size_t key_len, double value, const char *nonnull tags_str, - size_t tags_len); - -bool dd_command_process_telemetry_metrics(mpack_node_t metrics) -{ - if (mpack_node_type(metrics) != mpack_type_map) { - return false; - } - - if (!ddtrace_metric_register_buffer) { - mlog_g(dd_log_debug, "ddtrace_metric_register_buffer unavailable"); - return true; - } - - for (size_t i = 0; i < mpack_node_map_count(metrics); i++) { - mpack_node_t key = mpack_node_map_key_at(metrics, i); - - const char *key_str = mpack_node_str(key); - if (!key_str) { - continue; - } - - size_t key_len = mpack_node_strlen(key); - mpack_node_t arr_value = mpack_node_map_value_at(metrics, i); - - for (size_t j = 0; j < mpack_node_array_length(arr_value); j++) { - mpack_node_t value = mpack_node_array_at(arr_value, j); - mpack_node_t dval_node = mpack_node_array_at(value, 0); - double dval = mpack_node_double(dval_node); - - const char *tags_str = ""; - char *modified_tags_str = NULL; - size_t tags_len = 0; - if (mpack_node_array_length(value) >= 2) { - mpack_node_t tags = mpack_node_array_at(value, 1); - tags_str = mpack_node_str(tags); - tags_len = mpack_node_strlen(tags); - } - if (mpack_node_error(metrics) != mpack_ok) { - break; - } - - // enrich tags for waf.requests with input_truncated=true if the - // data is truncated - if (dd_msgpack_helpers_is_data_truncated() && - WAF_REQUEST_METRIC_LEN == key_len && - memcmp(WAF_REQUEST_METRIC, key_str, WAF_REQUEST_METRIC_LEN) == - 0) { - size_t separator = 0; - if (tags_len > 0) { - separator = TAG_SEPARATOR_LEN; - } - modified_tags_str = - emalloc(tags_len + TRUNCATED_TAG_LEN + 1 + separator); - if (modified_tags_str) { - memcpy(modified_tags_str, tags_str, tags_len); - if (separator > 0) { - modified_tags_str[tags_len] = TAG_SEPARATOR; - } - memcpy(modified_tags_str + tags_len + separator, - TRUNCATED_TAG, TRUNCATED_TAG_LEN); - tags_len += TRUNCATED_TAG_LEN + separator; - tags_str = modified_tags_str; - } - } - - _handle_telemetry_metric( - key_str, key_len, dval, tags_str, tags_len); - if (modified_tags_str) { - efree(modified_tags_str); - } - } - } - - return true; -} - -static void _init_zstr( - zend_string *_Atomic *nonnull zstr, const char *nonnull str, size_t len) -{ - zend_string *zstr_cur = atomic_load_explicit(zstr, memory_order_acquire); - if (zstr_cur != NULL) { - return; - } - zend_string *zstr_new = zend_string_init(str, len, 1); - if (atomic_compare_exchange_strong_explicit(zstr, &(zend_string *){NULL}, - zstr_new, memory_order_release, memory_order_relaxed)) { - return; - } - zend_string_release(zstr_new); -} - -// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -void _handle_telemetry_metric(const char *nonnull key_str, size_t key_len, - double value, const char *nonnull tags_str, size_t tags_len) -{ -#define HANDLE_METRIC(name, type) \ - do { \ - if (key_len == LSTRLEN(name) && memcmp(key_str, name, key_len) == 0) { \ - static zend_string *_Atomic key_zstr; \ - _init_zstr(&key_zstr, name, LSTRLEN(name)); \ - zend_string *tags_zstr = zend_string_init(tags_str, tags_len, 0); \ - dd_telemetry_add_metric(key_zstr, value, tags_zstr, type); \ - zend_string_release(tags_zstr); \ - mlog_g(dd_log_debug, \ - "Telemetry metric %.*s added with tags %.*s and value %f", \ - (int)key_len, key_str, (int)tags_len, tags_str, value); \ - return; \ - } \ - } while (0) - - HANDLE_METRIC("waf.requests", DDTRACE_METRIC_TYPE_COUNT); - - mlog_g(dd_log_info, "Unknown telemetry metric %.*s", (int)key_len, key_str); -} - static void _dump_in_msg( dd_log_level_t lvl, const char *nonnull data, size_t data_len) { diff --git a/appsec/src/extension/commands_helpers.h b/appsec/src/extension/commands_helpers.h index 3ebe26c140..459fa0ac1d 100644 --- a/appsec/src/extension/commands_helpers.h +++ b/appsec/src/extension/commands_helpers.h @@ -38,7 +38,6 @@ dd_result dd_command_proc_resp_verd_span_data(mpack_node_t root, /* Common helpers */ void dd_command_process_meta(mpack_node_t root, zend_object *nonnull span); bool dd_command_process_metrics(mpack_node_t root, zend_object *nonnull span); -bool dd_command_process_telemetry_metrics(mpack_node_t root); dd_result dd_command_process_config_features( mpack_node_t root, ATTR_UNUSED void *nullable ctx); dd_result dd_command_process_config_features_unexpected( diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index 84e6ebb0d6..3300680688 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -469,6 +469,8 @@ bool client::handle_command(network::request_shutdown::request &command) if (!response) { return false; } + + context_->set_input_truncated(command.input_truncated); collect_metrics(*response, *service_, context_, sc_settings_); service_->drain_logs(sc_settings_); @@ -550,7 +552,9 @@ void client::run(worker::queue_consumer &q) namespace { struct request_metrics_submitter : public telemetry::telemetry_submitter { - request_metrics_submitter() = default; + request_metrics_submitter(service &svc, bool input_truncated) + : service_{svc}, input_truncated_{input_truncated} + {} ~request_metrics_submitter() override = default; request_metrics_submitter(const request_metrics_submitter &) = delete; request_metrics_submitter &operator=( @@ -561,16 +565,18 @@ struct request_metrics_submitter : public telemetry::telemetry_submitter { void submit_metric(std::string_view name, double value, telemetry::telemetry_tags tags) override { - std::string tags_s = tags.consume(); + if (input_truncated_ && name == metrics::waf_requests) { + tags.add("input_truncated", "true"); + } SPDLOG_TRACE("submit_metric [req]: name={}, value={}, tags={}", name, - value, tags_s); - tel_metrics[name].emplace_back(value, std::move(tags_s)); + value, tags); + service_.submit_request_metric(name, value, std::move(tags)); }; void submit_span_metric(std::string_view name, double value) override { SPDLOG_TRACE( "submit_span_metric [req]: name={}, value={}", name, value); - metrics[name] = value; + span_metrics[name] = value; }; void submit_span_meta(std::string_view name, std::string value) override { @@ -594,38 +600,29 @@ struct request_metrics_submitter : public telemetry::telemetry_submitter { } std::map meta; - std::map metrics; - std::unordered_map>> - tel_metrics; + std::map span_metrics; + service &service_; // NOLINT + bool input_truncated_; }; template void collect_metrics_impl(Response &response, service &service, std::optional &context, const sidecar_settings &sc_settings) { - request_metrics_submitter msubmitter{}; + request_metrics_submitter msubmitter{ + service, context ? context->get_input_truncated() : false}; if (context) { + // span metrics/meta go to msubmitter.span_metrics/meta and are sent to + // the extension; request telemetry metrics are routed to the service + // and sent to sidecar context->get_metrics(msubmitter); } - auto request_metrics = std::move(msubmitter.tel_metrics); - for (auto &metric : request_metrics) { - for (auto &[value, tags] : metric.second) { - service.submit_request_metric(metric.first, value, - telemetry::telemetry_tags::from_string(std::move(tags))); - } - } - service.drain_metrics(sc_settings, - // metrics for the extension are put back into the msubmitter - [&msubmitter](std::string_view name, double value, auto tags) { - msubmitter.submit_metric(name, value, std::move(tags)); - }); - msubmitter.metrics.merge(service.drain_legacy_metrics()); + service.drain_metrics(sc_settings); + msubmitter.span_metrics.merge(service.drain_legacy_metrics()); msubmitter.meta.merge(service.drain_legacy_meta()); - response.tel_metrics = std::move(msubmitter.tel_metrics); response.meta = std::move(msubmitter.meta); - response.metrics = std::move(msubmitter.metrics); + response.metrics = std::move(msubmitter.span_metrics); } void collect_metrics(network::request_shutdown::response &response, service &service, std::optional &context, const sidecar_settings &sc_settings) diff --git a/appsec/src/helper/engine.hpp b/appsec/src/helper/engine.hpp index d94a4284e9..74f7ae8a1b 100644 --- a/appsec/src/helper/engine.hpp +++ b/appsec/src/helper/engine.hpp @@ -68,6 +68,8 @@ class engine { parameter &¶m, const std::string &rasp_rule = ""); // NOLINTNEXTLINE(google-runtime-references) void get_metrics(telemetry::telemetry_submitter &msubmitter); + [[nodiscard]] bool get_input_truncated() const { return input_truncated_; } + void set_input_truncated(bool input_truncated) { input_truncated_ = input_truncated; } protected: std::shared_ptr common_; @@ -76,6 +78,7 @@ class engine { std::vector prev_published_params_; rate_limiter & limiter_; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members) + bool input_truncated_{false}; }; engine(const engine &) = delete; diff --git a/appsec/src/helper/network/proto.hpp b/appsec/src/helper/network/proto.hpp index 74c0b9b4e5..3291dc263d 100644 --- a/appsec/src/helper/network/proto.hpp +++ b/appsec/src/helper/network/proto.hpp @@ -137,11 +137,8 @@ struct client_init { std::map meta; std::map metrics; - std::unordered_map>> - tel_metrics; - MSGPACK_DEFINE(status, version, errors, meta, metrics, tel_metrics) + MSGPACK_DEFINE(status, version, errors, meta, metrics) }; }; @@ -280,6 +277,7 @@ struct request_shutdown { dds::parameter data; std::uint64_t api_sec_samp_key{0}; std::uint64_t queue_id{0}; + bool input_truncated{false}; request() = default; request(const request &) = delete; @@ -288,7 +286,7 @@ struct request_shutdown { request &operator=(request &&) = default; ~request() override = default; - MSGPACK_DEFINE(data, api_sec_samp_key, queue_id) + MSGPACK_DEFINE(data, api_sec_samp_key, queue_id, input_truncated) }; struct response : base_response_generic { @@ -306,12 +304,9 @@ struct request_shutdown { std::map meta; std::map metrics; - std::unordered_map>> - tel_metrics; MSGPACK_DEFINE( - actions, triggers, force_keep, settings, meta, metrics, tel_metrics) + actions, triggers, force_keep, settings, meta, metrics) }; }; diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index de26e8bd60..cec88e4dcb 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -107,10 +107,8 @@ class service { private: friend class service; - template void drain_metrics(const sidecar_settings &sc_settings, - const telemetry_settings &telemetry_settings, - Func &&handle_metric_for_extension) + const telemetry_settings &telemetry_settings) { std::vector metrics; { @@ -118,17 +116,8 @@ class service { metrics.swap(pending_metrics_); } for (auto &metric : metrics) { - // waf.requests must be handled by extension; modifies the tags - // TODO: the extension could send the information in the - // message so we could add the tag ourselves. That would allow - // some code to be removed from the extension - if (metric.name == dds::metrics::waf_requests) { - std::invoke(std::forward(handle_metric_for_extension), - metric.name, metric.value, std::move(metric.tags)); - } else { - submit_metric_ffi(sc_settings, telemetry_settings, - metric.name, metric.value, metric.tags.consume()); - } + submit_metric_ffi(sc_settings, telemetry_settings, metric.name, + metric.value, metric.tags.consume()); } } @@ -265,16 +254,13 @@ class service { void notify_of_rc_updates() { client_handler_->poll(); } - void submit_request_metric( - std::string_view metric_name, double value, telemetry::telemetry_tags tags - ) { - msubmitter_->submit_metric(metric_name, value, std::move(tags)); + void submit_request_metric(std::string_view metric_name, double value, + telemetry::telemetry_tags tags) + { + msubmitter_->submit_metric(metric_name, value, std::move(tags)); } - template - void drain_metrics( - const sidecar_settings &sc_settings, - Func &&handle_metric_for_extension) + void drain_metrics(const sidecar_settings &sc_settings) { auto registered = metrics_registered_.load(std::memory_order_relaxed); if (!registered && metrics_registered_.compare_exchange_strong( @@ -282,8 +268,7 @@ class service { register_known_metrics(sc_settings, telemetry_settings_); } - msubmitter_->drain_metrics(sc_settings, telemetry_settings_, - std::forward(handle_metric_for_extension)); + msubmitter_->drain_metrics(sc_settings, telemetry_settings_); } void drain_logs(const sidecar_settings &sc_settings) From 1d00e7365c67d67aeedeb632bfd0661ceb3618f9 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Fri, 12 Dec 2025 18:39:10 +0000 Subject: [PATCH 5/5] Fix tests --- .../src/extension/commands/request_shutdown.c | 3 +- appsec/src/helper/client.cpp | 17 ++++--- appsec/src/helper/engine.hpp | 10 +++- appsec/src/helper/network/proto.hpp | 3 +- appsec/src/helper/service.cpp | 44 ++++++++-------- appsec/src/helper/service.hpp | 32 ++++++------ appsec/src/helper/telemetry.hpp | 2 +- appsec/tests/extension/inc/mock_helper.php | 1 - .../tests/extension/input_truncated_01.phpt | 17 +++---- .../tests/extension/input_truncated_04.phpt | 10 +--- .../tests/extension/input_truncated_05.phpt | 6 +-- appsec/tests/extension/rshutdown_command.phpt | 1 + .../rshutdown_command_body_user_request.phpt | 1 + ...down_command_body_user_request_stream.phpt | 1 + .../tests/extension/rshutdown_telemetry.phpt | 37 -------------- appsec/tests/extension/user_req_basic_01.phpt | 1 + appsec/tests/extension/user_req_basic_02.phpt | 1 + appsec/tests/helper/broker_test.cpp | 6 +-- appsec/tests/helper/client_test.cpp | 35 +++++++++++-- appsec/tests/helper/service_test.cpp | 20 ++++++++ .../php/integration/TelemetryTests.groovy | 51 +++++++++++++++++++ 21 files changed, 176 insertions(+), 123 deletions(-) delete mode 100644 appsec/tests/extension/rshutdown_telemetry.phpt diff --git a/appsec/src/extension/commands/request_shutdown.c b/appsec/src/extension/commands/request_shutdown.c index c28c379ebf..0ec3148c95 100644 --- a/appsec/src/extension/commands/request_shutdown.c +++ b/appsec/src/extension/commands/request_shutdown.c @@ -28,7 +28,8 @@ static const char *nullable _header_content_type_zend_array( static const dd_command_spec _spec = { .name = "request_shutdown", .name_len = sizeof("request_shutdown") - 1, - .num_args = 4, // a map, api sec sampling key, sidecar queue id, and input_truncated + .num_args = + 4, // a map, api sec sampling key, sidecar queue id, and input_truncated .outgoing_cb = _request_pack, .incoming_cb = dd_command_proc_resp_verd_span_data, .config_features_cb = dd_command_process_config_features_unexpected, diff --git a/appsec/src/helper/client.cpp b/appsec/src/helper/client.cpp index 3300680688..785736d650 100644 --- a/appsec/src/helper/client.cpp +++ b/appsec/src/helper/client.cpp @@ -27,9 +27,11 @@ namespace dds { namespace { void collect_metrics(network::request_shutdown::response &response, - service &service, std::optional &context, const sidecar_settings &sc_settings); + service &service, std::optional &context, + const sidecar_settings &sc_settings); void collect_metrics(network::client_init::response &response, service &service, - std::optional &context, const sidecar_settings &sc_settings); + std::optional &context, + const sidecar_settings &sc_settings); template // NOLINTNEXTLINE(google-runtime-references) @@ -469,7 +471,7 @@ bool client::handle_command(network::request_shutdown::request &command) if (!response) { return false; } - + context_->set_input_truncated(command.input_truncated); collect_metrics(*response, *service_, context_, sc_settings_); @@ -607,7 +609,8 @@ struct request_metrics_submitter : public telemetry::telemetry_submitter { template void collect_metrics_impl(Response &response, service &service, - std::optional &context, const sidecar_settings &sc_settings) + std::optional &context, + const sidecar_settings &sc_settings) { request_metrics_submitter msubmitter{ service, context ? context->get_input_truncated() : false}; @@ -625,12 +628,14 @@ void collect_metrics_impl(Response &response, service &service, response.metrics = std::move(msubmitter.span_metrics); } void collect_metrics(network::request_shutdown::response &response, - service &service, std::optional &context, const sidecar_settings &sc_settings) + service &service, std::optional &context, + const sidecar_settings &sc_settings) { collect_metrics_impl(response, service, context, sc_settings); } void collect_metrics(network::client_init::response &response, service &service, - std::optional &context, const sidecar_settings &sc_settings) + std::optional &context, + const sidecar_settings &sc_settings) { collect_metrics_impl(response, service, context, sc_settings); } diff --git a/appsec/src/helper/engine.hpp b/appsec/src/helper/engine.hpp index 74f7ae8a1b..4ed11ba7a5 100644 --- a/appsec/src/helper/engine.hpp +++ b/appsec/src/helper/engine.hpp @@ -68,8 +68,14 @@ class engine { parameter &¶m, const std::string &rasp_rule = ""); // NOLINTNEXTLINE(google-runtime-references) void get_metrics(telemetry::telemetry_submitter &msubmitter); - [[nodiscard]] bool get_input_truncated() const { return input_truncated_; } - void set_input_truncated(bool input_truncated) { input_truncated_ = input_truncated; } + [[nodiscard]] bool get_input_truncated() const + { + return input_truncated_; + } + void set_input_truncated(bool input_truncated) + { + input_truncated_ = input_truncated; + } protected: std::shared_ptr common_; diff --git a/appsec/src/helper/network/proto.hpp b/appsec/src/helper/network/proto.hpp index 3291dc263d..b5a4dd4d2f 100644 --- a/appsec/src/helper/network/proto.hpp +++ b/appsec/src/helper/network/proto.hpp @@ -305,8 +305,7 @@ struct request_shutdown { std::map meta; std::map metrics; - MSGPACK_DEFINE( - actions, triggers, force_keep, settings, meta, metrics) + MSGPACK_DEFINE(actions, triggers, force_keep, settings, meta, metrics) }; }; diff --git a/appsec/src/helper/service.cpp b/appsec/src/helper/service.cpp index 7a702f875e..efbee76ff6 100644 --- a/appsec/src/helper/service.cpp +++ b/appsec/src/helper/service.cpp @@ -37,11 +37,14 @@ using ddog_sidecar_enqueue_telemetry_log_t = decltype(&ddog_sidecar_enqueue_telemetry_log); ddog_sidecar_enqueue_telemetry_log_t fn_ddog_sidecar_enqueue_telemetry_log; -using ddog_sidecard_enqueue_telemetry_point_t = decltype(&ddog_sidecar_enqueue_telemetry_point); +using ddog_sidecard_enqueue_telemetry_point_t = + decltype(&ddog_sidecar_enqueue_telemetry_point); ddog_sidecard_enqueue_telemetry_point_t fn_ddog_sidecar_enqueue_telemetry_point; -using ddog_sidecar_enqueue_telemetry_metric_t = decltype(&ddog_sidecar_enqueue_telemetry_metric); -ddog_sidecar_enqueue_telemetry_metric_t fn_ddog_sidecar_enqueue_telemetry_metric; +using ddog_sidecar_enqueue_telemetry_metric_t = + decltype(&ddog_sidecar_enqueue_telemetry_metric); +ddog_sidecar_enqueue_telemetry_metric_t + fn_ddog_sidecar_enqueue_telemetry_metric; using ddog_Error_message_t = decltype(&ddog_Error_message); ddog_Error_message_t fn_ddog_Error_message; @@ -165,23 +168,21 @@ void service::metrics_impl::submit_log(const sidecar_settings &sc_settings, void service::metrics_impl::register_metric_ffi( const sidecar_settings &sc_settings, - const telemetry_settings &telemetry_settings, - std::string_view name, ddog_MetricType type) + const telemetry_settings &telemetry_settings, std::string_view name, + ddog_MetricType type) { SPDLOG_TRACE("register_metric_ffi: name: {}, type: {}", name, type); if (fn_ddog_sidecar_enqueue_telemetry_metric == nullptr) { - throw std::runtime_error("Failed to resolve ddog_sidecar_enqueue_telemetry_metric"); + throw std::runtime_error( + "Failed to resolve ddog_sidecar_enqueue_telemetry_metric"); } ddog_MaybeError result = fn_ddog_sidecar_enqueue_telemetry_metric( to_ffi_string(sc_settings.session_id), to_ffi_string(sc_settings.runtime_id), to_ffi_string(telemetry_settings.service_name), - to_ffi_string(telemetry_settings.env_name), - to_ffi_string(name), - type, - DDOG_METRIC_NAMESPACE_APPSEC - ); + to_ffi_string(telemetry_settings.env_name), to_ffi_string(name), type, + DDOG_METRIC_NAMESPACE_APPSEC); if (result.tag == DDOG_OPTION_ERROR_SOME_ERROR) { ddog_CharSlice const error_msg = fn_ddog_Error_message(&result.some); @@ -193,17 +194,15 @@ void service::metrics_impl::register_metric_ffi( void service::metrics_impl::submit_metric_ffi( const sidecar_settings &sc_settings, - const telemetry_settings &telemetry_settings, - std::string_view name, - double value, - std::optional tags) + const telemetry_settings &telemetry_settings, std::string_view name, + double value, std::optional tags) { - SPDLOG_TRACE( - "submit_metric_ffi: name: {}, value: {}, tags: {}", name, + SPDLOG_TRACE("submit_metric_ffi: name: {}, value: {}, tags: {}", name, value, tags.has_value() ? tags.value() : "(none)"sv); if (fn_ddog_sidecar_enqueue_telemetry_point == nullptr) { - throw std::runtime_error("Failed to resolve ddog_sidecar_enqueue_telemetry_point"); + throw std::runtime_error( + "Failed to resolve ddog_sidecar_enqueue_telemetry_point"); } CharSlice tags_ffi; CharSlice *tags_ffi_ptr = nullptr; @@ -215,11 +214,8 @@ void service::metrics_impl::submit_metric_ffi( to_ffi_string(sc_settings.session_id), to_ffi_string(sc_settings.runtime_id), to_ffi_string(telemetry_settings.service_name), - to_ffi_string(telemetry_settings.env_name), - to_ffi_string(name), - value, - tags_ffi_ptr - ); + to_ffi_string(telemetry_settings.env_name), to_ffi_string(name), value, + tags_ffi_ptr); if (result.tag == DDOG_OPTION_ERROR_SOME_ERROR) { ddog_CharSlice const error_msg = fn_ddog_Error_message(&result.some); @@ -252,7 +248,7 @@ void service::handle_worker_count_metrics(const sidecar_settings &sc_settings) auto new_st = cur_st; new_st.latest_count_sent = true; - bool success = num_workers_.compare_exchange_strong( + bool const success = num_workers_.compare_exchange_strong( cur_st, new_st, std::memory_order_relaxed); if (!success) { diff --git a/appsec/src/helper/service.hpp b/appsec/src/helper/service.hpp index cec88e4dcb..05add764f9 100644 --- a/appsec/src/helper/service.hpp +++ b/appsec/src/helper/service.hpp @@ -103,7 +103,6 @@ class service { std::move(stack_trace), std::move(tags), is_sensitive}); } - private: friend class service; @@ -299,29 +298,32 @@ class service { "remote_config.requests_before_running"sv, 1, {}); } } - - void increment_num_workers() { + + void increment_num_workers() + { auto cur = num_workers_.load(std::memory_order_relaxed); while (true) { auto new_v = num_workers_t{ - .latest_count_sent = false, - .count = cur.count + 1}; - if (num_workers_.compare_exchange_weak(cur, new_v, std::memory_order_relaxed)) { + .latest_count_sent = false, .count = cur.count + 1}; + if (num_workers_.compare_exchange_weak( + cur, new_v, std::memory_order_relaxed)) { break; } } } - void decrement_num_workers() { + void decrement_num_workers() + { auto cur = num_workers_.load(std::memory_order_relaxed); if (cur.count == 0) { SPDLOG_WARN("Attempted to decrement num_workers_ below 0"); return; } while (true) { - auto new_v = num_workers_t{ - .latest_count_sent = cur.latest_count_sent, - .count = cur.count - 1}; - if (num_workers_.compare_exchange_weak(cur, new_v, std::memory_order_relaxed)) { + auto new_v = + num_workers_t{.latest_count_sent = cur.latest_count_sent, + .count = cur.count - 1}; + if (num_workers_.compare_exchange_weak( + cur, new_v, std::memory_order_relaxed)) { break; } } @@ -339,8 +341,8 @@ class service { std::shared_ptr msubmitter_; struct num_workers_t { - bool latest_count_sent: 1; - uint64_t count: 63; + bool latest_count_sent : 1; + uint64_t count : 63; constexpr bool operator==(const num_workers_t &other) const { return count == other.count && @@ -360,8 +362,8 @@ template <> struct fmt::formatter { // NOLINTNEXTLINE constexpr auto parse(fmt::format_parse_context &ctx) { return ctx.begin(); } template - auto format( - ddog_MetricType type, FormatContext &ctx) const -> decltype(ctx.out()) + auto format(ddog_MetricType type, FormatContext &ctx) const + -> decltype(ctx.out()) { switch (type) { case DDOG_METRIC_TYPE_GAUGE: diff --git a/appsec/src/helper/telemetry.hpp b/appsec/src/helper/telemetry.hpp index a3d1f94b12..3678cbea3e 100644 --- a/appsec/src/helper/telemetry.hpp +++ b/appsec/src/helper/telemetry.hpp @@ -82,7 +82,7 @@ struct fmt::formatter : fmt::formatter { auto format( - const dds::telemetry::telemetry_tags& tags, format_context &ctx) const + const dds::telemetry::telemetry_tags &tags, format_context &ctx) const { return formatter::format( std::string_view{tags.data_}, ctx); diff --git a/appsec/tests/extension/inc/mock_helper.php b/appsec/tests/extension/inc/mock_helper.php index d874f4b526..d7abd2fe07 100644 --- a/appsec/tests/extension/inc/mock_helper.php +++ b/appsec/tests/extension/inc/mock_helper.php @@ -277,7 +277,6 @@ function response_request_shutdown($message, $mergeWithEmpty = true) { false, //force_keep [], [], - [], [] ], $message); } diff --git a/appsec/tests/extension/input_truncated_01.phpt b/appsec/tests/extension/input_truncated_01.phpt index d9eb467c6b..fd6d53e0e2 100644 --- a/appsec/tests/extension/input_truncated_01.phpt +++ b/appsec/tests/extension/input_truncated_01.phpt @@ -15,25 +15,20 @@ include __DIR__ . '/inc/mock_helper.php'; $helper = Helper::createInitedRun([ response_list(response_request_init([[['ok', []]]])), - response_list(response_request_shutdown([[['ok', []]], [], false, [], - [], [], ["waf.requests" => [[2.0, ""], [1.0, "a=b"]]]])) + response_list(response_request_shutdown([[['ok', []]]])) ]); var_dump(rinit()); $helper->get_commands(); // ignore var_dump(rshutdown()); -$helper->get_commands(); + +$commands = $helper->get_commands(); +// Verify request_shutdown receives input_truncated=true +var_dump($commands[0][1][3]); ?> --EXPECTF-- bool(true) - -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=2.000000 tags=input_truncated=true in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=1.000000 tags=a=b,input_truncated=true in %s on line %d +bool(true) bool(true) diff --git a/appsec/tests/extension/input_truncated_04.phpt b/appsec/tests/extension/input_truncated_04.phpt index 7f6008ef92..8cb1bbe35d 100644 --- a/appsec/tests/extension/input_truncated_04.phpt +++ b/appsec/tests/extension/input_truncated_04.phpt @@ -16,8 +16,7 @@ include __DIR__ . '/inc/mock_helper.php'; $helper = Helper::createInitedRun([ response_list(response_request_init([[['ok', []]]])), - response_list(response_request_shutdown([[['ok', []]], [], false, [], - [], [], ["waf.requests" => [[2.0, ""], [1.0, "a=b"]]]])) + response_list(response_request_shutdown([[['ok', []]]])) ]); rinit(); @@ -28,13 +27,6 @@ var_dump($commands[1][1][0]['server.request.cookies']['d']); var_dump($commands[1][1][0]['server.request.cookies']['truncated']); ?> --EXPECTF-- -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=2.000000 tags=input_truncated=true in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=1.000000 tags=a=b,input_truncated=true in %s on line %d array(1) { ["'1'"]=> array(1) { diff --git a/appsec/tests/extension/input_truncated_05.phpt b/appsec/tests/extension/input_truncated_05.phpt index 971ca6afa7..d814bd9e57 100644 --- a/appsec/tests/extension/input_truncated_05.phpt +++ b/appsec/tests/extension/input_truncated_05.phpt @@ -17,8 +17,7 @@ $helper = Helper::createInitedRun([ response_list(response_request_exec([[['ok', []]]])), // Test 1 response_list(response_request_exec([[['ok', []]]])), // Test 2 response_list(response_request_exec([[['ok', []]]])), // Test 3 - response_list(response_request_shutdown([[['ok', []]], [], false, [], - [], [], ["waf.requests" => [[3.0, ""]]]])) + response_list(response_request_shutdown([[['ok', []]]])) ]); rinit(); @@ -60,9 +59,6 @@ echo "test3: number of nulls: ", $num_nils, "\n"; ?> --EXPECTF-- -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=3.000000 tags=input_truncated=true in %s on line %d test1: number of elements inside: 2046 test2: number of elements inside 1st array: 2000 test2: number of elements inside 2st array: 44 diff --git a/appsec/tests/extension/rshutdown_command.phpt b/appsec/tests/extension/rshutdown_command.phpt index 1b8ae544be..b5bc91c26e 100644 --- a/appsec/tests/extension/rshutdown_command.phpt +++ b/appsec/tests/extension/rshutdown_command.phpt @@ -64,6 +64,7 @@ Array [1] => 0 [2] => 0 + [3] => ) ) diff --git a/appsec/tests/extension/rshutdown_command_body_user_request.phpt b/appsec/tests/extension/rshutdown_command_body_user_request.phpt index c424ea6522..85a401c5f0 100644 --- a/appsec/tests/extension/rshutdown_command_body_user_request.phpt +++ b/appsec/tests/extension/rshutdown_command_body_user_request.phpt @@ -107,6 +107,7 @@ test [1] => 0 [2] => %s + [3] => ) ) diff --git a/appsec/tests/extension/rshutdown_command_body_user_request_stream.phpt b/appsec/tests/extension/rshutdown_command_body_user_request_stream.phpt index 57a2d5545e..1a2709b916 100644 --- a/appsec/tests/extension/rshutdown_command_body_user_request_stream.phpt +++ b/appsec/tests/extension/rshutdown_command_body_user_request_stream.phpt @@ -116,6 +116,7 @@ test [1] => 0 [2] => %s + [3] => ) ) diff --git a/appsec/tests/extension/rshutdown_telemetry.phpt b/appsec/tests/extension/rshutdown_telemetry.phpt deleted file mode 100644 index 98ab2129a1..0000000000 --- a/appsec/tests/extension/rshutdown_telemetry.phpt +++ /dev/null @@ -1,37 +0,0 @@ ---TEST-- -request_shutdown relays telemetry metrics from the daemon ---INI-- -datadog.appsec.enabled=1 -display_errors=1 ---GET-- -a=b ---FILE-- - [[2.0, "foo=bar"], [1.0, "a=b"]]]])) -]); - -var_dump(rinit()); -$helper->get_commands(); // ignore - -var_dump(rshutdown()); -$helper->get_commands(); - -?> ---EXPECTF-- -bool(true) - -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=2.000000 tags=foo=bar in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call ddtrace_metric_register_buffer with name=waf.requests type=1 ns=3 in %s on line %d - -Notice: datadog\appsec\testing\rshutdown(): Would call to ddtrace_metric_add_point with name=waf.requests value=1.000000 tags=a=b in %s on line %d -bool(true) diff --git a/appsec/tests/extension/user_req_basic_01.phpt b/appsec/tests/extension/user_req_basic_01.phpt index 066c589ada..9ce730ef5a 100644 --- a/appsec/tests/extension/user_req_basic_01.phpt +++ b/appsec/tests/extension/user_req_basic_01.phpt @@ -185,6 +185,7 @@ Array [1] => 0 [2] => %s + [3] => ) ) diff --git a/appsec/tests/extension/user_req_basic_02.phpt b/appsec/tests/extension/user_req_basic_02.phpt index 7d768da77c..87feb3f96e 100644 --- a/appsec/tests/extension/user_req_basic_02.phpt +++ b/appsec/tests/extension/user_req_basic_02.phpt @@ -185,6 +185,7 @@ Array [1] => 0 [2] => %s + [3] => ) ) diff --git a/appsec/tests/helper/broker_test.cpp b/appsec/tests/helper/broker_test.cpp index 2819975daf..3d193c2170 100644 --- a/appsec/tests/helper/broker_test.cpp +++ b/appsec/tests/helper/broker_test.cpp @@ -80,7 +80,7 @@ TEST(BrokerTest, SendClientInit) packer.pack_array(1); // Array of messages packer.pack_array(2); // First message pack_str(packer, "client_init"); // Type - packer.pack_array(6); + packer.pack_array(5); pack_str(packer, "ok"); pack_str(packer, dds::php_ddappsec_version); packer.pack_array(2); @@ -88,7 +88,6 @@ TEST(BrokerTest, SendClientInit) pack_str(packer, "two"); packer.pack_map(0); packer.pack_map(0); - packer.pack_map(0); const auto &expected_data = ss.str(); network::header_t h; @@ -173,7 +172,7 @@ TEST(BrokerTest, SendRequestShutdown) packer.pack_array(1); // Array of messages packer.pack_array(2); // First message pack_str(packer, "request_shutdown"); // Type - packer.pack_array(7); + packer.pack_array(6); packer.pack_array(1); packer.pack_array(2); pack_str(packer, "block"); @@ -189,7 +188,6 @@ TEST(BrokerTest, SendRequestShutdown) packer.pack_map(0); // Settings packer.pack_map(0); // Meta packer.pack_map(0); // Metrics - packer.pack_map(0); // Tel_metrics const auto &expected_data = ss.str(); network::header_t h; diff --git a/appsec/tests/helper/client_test.cpp b/appsec/tests/helper/client_test.cpp index a58d65d140..d51382be42 100644 --- a/appsec/tests/helper/client_test.cpp +++ b/appsec/tests/helper/client_test.cpp @@ -4,10 +4,12 @@ // This product includes software developed at Datadog // (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. #include "common.hpp" +#include "engine_settings.hpp" #include "parameter.hpp" #include "service_config.hpp" #include "sidecar_settings.hpp" #include "subscriber/waf.hpp" +#include "tel_subm_mock.hpp" #include #include #include @@ -47,12 +49,24 @@ class service_manager : public dds::service_manager { class service : public dds::service { public: + static std::shared_ptr create_metrics() + { + return dds::service::create_shared_metrics(); + } + service(std::shared_ptr engine, std::shared_ptr service_config, dds::sidecar_settings sc_settings) : dds::service{std::move(engine), std::move(service_config), {}, dds::service::create_shared_metrics(), "/rc_path", {}} {} + + service(std::shared_ptr engine, + std::shared_ptr service_config, + std::shared_ptr msubmitter) + : dds::service{std::move(engine), std::move(service_config), {}, + std::move(msubmitter), "/rc_path", {}} + {} }; } // namespace mock @@ -132,13 +146,13 @@ constexpr bool EXTENSION_CONFIGURATION_DISABLED = false; TEST(ClientTest, ClientInit) { - auto smanager = std::make_shared(); + auto fn = create_sample_rules_ok(); + + 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"; @@ -153,6 +167,19 @@ TEST(ClientTest, ClientInit) send(testing::An &>())) .WillOnce(DoAll(testing::SaveArg<0>(&res), Return(true))); + // create service and return it + engine_settings eng_settings; + eng_settings.rules_file = fn; + auto msubmitter = mock::service::create_metrics(); + std::shared_ptr engine{ + engine::from_settings(eng_settings, *msubmitter)}; + auto service_config = std::make_shared(); + auto service = + std::make_shared(engine, service_config, msubmitter); + EXPECT_CALL(*smanager, get_or_create_service(_, _, _)) + .Times(1) + .WillOnce(Return(service)); + EXPECT_TRUE(c.run_client_init()); auto msg_res = dynamic_cast(res.get()); @@ -165,8 +192,6 @@ TEST(ClientTest, ClientInit) msg_res->meta[std::string(metrics::event_rules_errors)].c_str(), "{}"); EXPECT_EQ(msg_res->metrics.size(), 2); - // For small enough integers this comparison should work, otherwise replace - // with EXPECT_NEAR. EXPECT_EQ(msg_res->metrics[metrics::event_rules_loaded], 5.0); EXPECT_EQ(msg_res->metrics[metrics::event_rules_failed], 0.0); } diff --git a/appsec/tests/helper/service_test.cpp b/appsec/tests/helper/service_test.cpp index 7059a40507..6b5e6e78e1 100644 --- a/appsec/tests/helper/service_test.cpp +++ b/appsec/tests/helper/service_test.cpp @@ -61,8 +61,28 @@ __attribute__((visibility("default"))) ddog_CharSlice ddog_Error_message( return ddog_CharSlice{nullptr, 0}; } +__attribute__((visibility("default"))) ddog_MaybeError +ddog_sidecar_enqueue_telemetry_point(ddog_CharSlice /*session_id_ffi*/, + ddog_CharSlice /*runtime_id_ffi*/, ddog_CharSlice /*service_name_ffi*/, + ddog_CharSlice /*env_name_ffi*/, ddog_CharSlice /*metric_name_ffi*/, + double /*value*/, ddog_CharSlice * /*tags_ffi*/) +{ + return ddog_MaybeError{.tag = DDOG_OPTION_ERROR_NONE_ERROR}; +} + +__attribute__((visibility("default"))) ddog_MaybeError +ddog_sidecar_enqueue_telemetry_metric(ddog_CharSlice /*session_id_ffi*/, + ddog_CharSlice /*runtime_id_ffi*/, ddog_CharSlice /*service_name_ffi*/, + ddog_CharSlice /*env_name_ffi*/, ddog_CharSlice /*metric_name_ffi*/, + enum ddog_MetricType /*metric_type*/, + enum ddog_MetricNamespace /*metric_namespace*/) +{ + return ddog_MaybeError{.tag = DDOG_OPTION_ERROR_NONE_ERROR}; +} + __attribute__((constructor)) void resolve_symbols() { dds::remote_config::resolve_symbols(); + dds::service::resolve_symbols(); } } diff --git a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy index 84b5fb7302..adb9017e34 100644 --- a/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy +++ b/appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/TelemetryTests.groovy @@ -541,4 +541,55 @@ class TelemetryTests { assert loginFailure.tags.find { it.startsWith('sdk_version:v2') } != null assert loginFailure.type == 'count' } + + /** + * This test verifies that when input is truncated (strings > 4096 chars), + * the waf.requests metric includes the input_truncated:true tag. + */ + @Test + @Order(6) + void 'Input truncation telemetry is generated'() { + Supplier requestSup = CONTAINER.applyRemoteConfig(RC_TARGET, [ + 'datadog/2/ASM_FEATURES/asm_features_activation/config': [ + asm: [enabled: true] + ] + ]) + + // first request to start helper + Trace trace = CONTAINER.traceFromRequest('/hello.php') { HttpResponse resp -> + assert resp.statusCode() == 200 + } + assert trace.traceId != null + + RemoteConfigRequest rcReq = requestSup.get() + assert rcReq != null, 'No RC request received' + + // request with a very long query string (> 4096 chars) to trigger truncation + def longString = 'A' * 5000 + HttpRequest req = CONTAINER.buildReq("/hello.php?long_param=${longString}") + .GET().build() + trace = CONTAINER.traceFromRequest(req, ofString()) { HttpResponse resp -> + assert resp.body().size() > 0 + } + assert trace.traceId != null + + TelemetryHelpers.Metric wafReqTruncated + + waitForMetrics(30) { List messages -> + def allSeries = messages.collectMany { it.series } + wafReqTruncated = allSeries.find { + it.name == 'waf.requests' && 'input_truncated:true' in it.tags + } + + wafReqTruncated != null + } + + assert wafReqTruncated != null + assert wafReqTruncated.namespace == 'appsec' + assert wafReqTruncated.points[0][1] >= 1.0 + assert 'input_truncated:true' in wafReqTruncated.tags + assert wafReqTruncated.tags.find { it.startsWith('event_rules_version:') } != null + assert wafReqTruncated.tags.find { it.startsWith('waf_version:') } != null + assert wafReqTruncated.type == 'count' + } }