Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WAF telemetry #2735

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

WAF telemetry #2735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb8b87b
Telemetry in AppSec
cataphract Oct 2, 2024
1c02927
Address PR comments
cataphract Nov 5, 2024
b3f6f9f
appsec helper: abstract tags format
cataphract Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions appsec/src/extension/commands/client_init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ 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)
Expand Down
201 changes: 164 additions & 37 deletions appsec/src/extension/commands_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "request_abort.h"
#include "tags.h"
#include <ext/standard/base64.h>
#include <mpack.h>
#include <stdatomic.h>

typedef struct _dd_omsg {
zend_llist iovecs;
Expand Down Expand Up @@ -437,7 +439,71 @@ static void _command_process_stack_trace_parameters(mpack_node_t root)
}
}

dd_result _command_process_actions(mpack_node_t root, struct req_info *ctx)
static dd_result _command_process_actions(
mpack_node_t root, struct req_info *ctx);

/*
* array(
* 0: [<"ok" / "record" / "block" / "redirect">,
* [if block/redirect parameters: (map)]]
* 1: [if block/redirect/record: appsec span data (array of strings: json
* fragments)],
* 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
#define RESP_INDEX_APPSEC_SPAN_DATA 1
#define RESP_INDEX_FORCE_KEEP 2
#define RESP_INDEX_SPAN_META 3
#define RESP_INDEX_SPAN_METRICS 4
#define RESP_INDEX_TELEMETRY_METRICS 5

dd_result dd_command_proc_resp_verd_span_data(
mpack_node_t root, void *unspecnull _ctx)
{
struct req_info *ctx = _ctx;
assert(ctx != NULL);

mpack_node_t actions = mpack_node_array_at(root, RESP_INDEX_ACTION_PARAMS);
dd_result res = _command_process_actions(actions, ctx);

if (res == dd_should_block || res == dd_should_redirect ||
res == dd_should_record) {
_set_appsec_span_data(
mpack_node_array_at(root, RESP_INDEX_APPSEC_SPAN_DATA));
}

mpack_node_t force_keep = mpack_node_array_at(root, RESP_INDEX_FORCE_KEEP);
if (mpack_node_type(force_keep) == mpack_type_bool &&
mpack_node_bool(force_keep)) {
dd_tags_set_sampling_priority();
}

if (mpack_node_array_length(root) >= RESP_INDEX_SPAN_METRICS + 1 &&
ctx->root_span) {
zend_object *span = ctx->root_span;

mpack_node_t meta = mpack_node_array_at(root, RESP_INDEX_SPAN_META);
dd_command_process_meta(meta, span);
mpack_node_t metrics =
mpack_node_array_at(root, RESP_INDEX_SPAN_METRICS);
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;
}

static dd_result _command_process_actions(
mpack_node_t root, struct req_info *ctx)
{
size_t actions = mpack_node_array_length(root);
dd_result res = dd_success;
Expand Down Expand Up @@ -482,42 +548,6 @@ dd_result _command_process_actions(mpack_node_t root, struct req_info *ctx)
return res;
}

dd_result dd_command_proc_resp_verd_span_data(
mpack_node_t root, void *unspecnull _ctx)
{
struct req_info *ctx = _ctx;
assert(ctx != NULL);

mpack_node_t actions = mpack_node_array_at(root, 0);

dd_result res = _command_process_actions(actions, ctx);

if (res == dd_should_block || res == dd_should_redirect ||
res == dd_should_record) {
_set_appsec_span_data(mpack_node_array_at(root, 1));
}

// NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
mpack_node_t force_keep = mpack_node_array_at(root, 2);
if (mpack_node_type(force_keep) == mpack_type_bool &&
mpack_node_bool(force_keep)) {
dd_tags_set_sampling_priority();
}

// NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
if (mpack_node_array_length(root) >= 5 && ctx->root_span) {
zend_object *span = ctx->root_span;

mpack_node_t meta = mpack_node_array_at(root, 3);
dd_command_process_meta(meta, span);
// NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers)
mpack_node_t metrics = mpack_node_array_at(root, 4);
dd_command_process_metrics(metrics, span);
}

return res;
}

static void _add_appsec_span_data_frag(mpack_node_t node)
{
const char *data = mpack_node_data(node);
Expand Down Expand Up @@ -648,6 +678,103 @@ 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it is a good practice or I made it up but, should this early declaration be at the top?

Copy link
Collaborator

@bwoebi bwoebi Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You put these at the top if you use these multiple times at various places across a file. For a single use like here, there's no reason to.

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 = "";
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;
}

_handle_telemetry_metric(
key_str, key_len, dval, tags_str, tags_len);
}
}

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; \
Anilm3 marked this conversation as resolved.
Show resolved Hide resolved
_init_zstr(&key_zstr, name, LSTRLEN(name)); \
zend_string *tags_zstr = zend_string_init(tags_str, tags_len, 1); \
ddtrace_metric_register_buffer( \
key_zstr, type, DDTRACE_METRIC_NAMESPACE_APPSEC); \
ddtrace_metric_add_point(key_zstr, value, tags_zstr); \
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw you added some integration tests checking this metrics but I am missing some PHPT checking the extension.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also allow you to test unknown telemetry metrics

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);

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)
{
Expand Down
1 change: 1 addition & 0 deletions appsec/src/extension/commands_helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ 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(
Expand Down
54 changes: 54 additions & 0 deletions appsec/src/extension/ddtrace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
#include "zend_object_handlers.h"
#include "zend_types.h"

void (*nullable ddtrace_metric_register_buffer)(
zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns);
bool (*nullable ddtrace_metric_add_point)(
zend_string *nonnull name, double value, zend_string *nonnull tags);

static int (*_orig_ddtrace_shutdown)(SHUTDOWN_FUNC_ARGS);
static int _mod_type;
static int _mod_number;
Expand All @@ -31,6 +36,7 @@ static zend_string *_meta_struct_propname;
static THREAD_LOCAL_ON_ZTS bool _suppress_ddtrace_rshutdown;
static uint8_t *_ddtrace_runtime_id = NULL;

static void _setup_testing_telemetry_functions(void);
static zend_module_entry *_find_ddtrace_module(void);
static int _ddtrace_rshutdown_testing(SHUTDOWN_FUNC_ARGS);
static void _register_testing_objects(void);
Expand All @@ -48,6 +54,11 @@ static zend_string *(*_ddtrace_ip_extraction_find)(zval *server);

static const char *nullable (*_ddtrace_remote_config_get_path)(void);

static void _test_ddtrace_metric_register_buffer(
zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns);
static bool _test_ddtrace_metric_add_point(
zend_string *nonnull name, double value, zend_string *nonnull tags);

static void dd_trace_load_symbols(void)
{
bool testing = get_global_DD_APPSEC_TESTING();
Expand Down Expand Up @@ -109,6 +120,19 @@ static void dd_trace_load_symbols(void)
"Failed to load ddtrace_remote_config_get_path: %s", dlerror());
}

ddtrace_metric_register_buffer =
dlsym(handle, "ddtrace_metric_register_buffer");
if (ddtrace_metric_register_buffer == NULL && !testing) {
mlog(dd_log_error, "Failed to load ddtrace_metric_register_buffer: %s",
dlerror()); // NOLINT(concurrency-mt-unsafe)
}

ddtrace_metric_add_point = dlsym(handle, "ddtrace_metric_add_point");
if (ddtrace_metric_add_point == NULL && !testing) {
mlog(dd_log_error, "Failed to load ddtrace_metric_add_point: %s",
dlerror()); // NOLINT(concurrency-mt-unsafe)
}

dlclose(handle);
}

Expand All @@ -123,6 +147,7 @@ void dd_trace_startup()

if (get_global_DD_APPSEC_TESTING()) {
_register_testing_objects();
_setup_testing_telemetry_functions();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I coudln't get why this is within APPSEC_TESTING. I'm probably missing something

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Makes sense

}

zend_module_entry *mod = _find_ddtrace_module();
Expand All @@ -145,6 +170,16 @@ void dd_trace_startup()
}
}

static void _setup_testing_telemetry_functions()
{
if (ddtrace_metric_register_buffer == NULL) {
ddtrace_metric_register_buffer = _test_ddtrace_metric_register_buffer;
}
if (ddtrace_metric_add_point == NULL) {
ddtrace_metric_add_point = _test_ddtrace_metric_add_point;
}
}

static zend_module_entry *_find_ddtrace_module()
{
zend_string *ddtrace_name =
Expand Down Expand Up @@ -516,3 +551,22 @@ static const zend_function_entry functions[] = {
// clang-format on

static void _register_testing_objects() { dd_phpobj_reg_funcs(functions); }

static void _test_ddtrace_metric_register_buffer(
zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns)
{
php_error_docref(NULL, E_NOTICE,
"Would call ddtrace_metric_register_buffer with name=%.*s "
"type=%d ns=%d",
(int)ZSTR_LEN(name), ZSTR_VAL(name), type, ns);
}
static bool _test_ddtrace_metric_add_point(
zend_string *nonnull name, double value, zend_string *nonnull tags)
{
php_error_docref(NULL, E_NOTICE,
"Would call to ddtrace_metric_add_point with name=%.*s value=%f "
"tags=%.*s",
(int)ZSTR_LEN(name), ZSTR_VAL(name), value, (int)ZSTR_LEN(tags),
ZSTR_VAL(tags));
return true;
}
25 changes: 25 additions & 0 deletions appsec/src/extension/ddtrace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,28 @@ bool dd_trace_user_req_add_listeners(
zend_string *nullable dd_ip_extraction_find(zval *nonnull server);

const char *nullable dd_trace_remote_config_get_path(void);

typedef enum {
DDTRACE_METRIC_TYPE_GAUGE,
DDTRACE_METRIC_TYPE_COUNT,
DDTRACE_METRIC_TYPE_DISTRIBUTION,
Anilm3 marked this conversation as resolved.
Show resolved Hide resolved
} ddtrace_metric_type;

typedef enum {
DDTRACE_METRIC_NAMESPACE_TRACERS,
DDTRACE_METRIC_NAMESPACE_PROFILERS,
DDTRACE_METRIC_NAMESPACE_RUM,
DDTRACE_METRIC_NAMESPACE_APPSEC,
DDTRACE_METRIC_NAMESPACE_IDE_PLUGINS,
DDTRACE_METRIC_NAMESPACE_LIVE_DEBUGGER,
DDTRACE_METRIC_NAMESPACE_IAST,
DDTRACE_METRIC_NAMESPACE_GENERAL,
DDTRACE_METRIC_NAMESPACE_TELEMETRY,
DDTRACE_METRIC_NAMESPACE_APM,
DDTRACE_METRIC_NAMESPACE_SIDECAR,
} ddtrace_metric_ns;

extern void (*nullable ddtrace_metric_register_buffer)(
zend_string *nonnull name, ddtrace_metric_type type, ddtrace_metric_ns ns);
extern bool (*nullable ddtrace_metric_add_point)(zend_string *nonnull name,
double value, zend_string *nonnull tags);
1 change: 0 additions & 1 deletion appsec/src/extension/request_abort.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "compatibility.h"
#include "configuration.h"
#include "ddappsec.h"
#include "dddefs.h"
#include "ddtrace.h"
#include "logging.h"
#include "php_compat.h"
Expand Down
Loading
Loading