From dff3a9f6734697a7a0799fa82ddaed69cb3c91b7 Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Mon, 21 Oct 2024 12:17:02 -0400 Subject: [PATCH 1/4] Changing what seems to be the default value to 100 --- ext/configuration.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/configuration.h b/ext/configuration.h index 57dd53cc7e..1208433253 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -149,7 +149,7 @@ enum ddtrace_sampling_rules_format { CONFIG(SET, DD_TRACE_RESOURCE_URI_QUERY_PARAM_ALLOWED, "") \ CONFIG(SET, DD_TRACE_HTTP_URL_QUERY_PARAM_ALLOWED, "*") \ CONFIG(SET, DD_TRACE_HTTP_POST_DATA_PARAM_ALLOWED, "") \ - CONFIG(INT, DD_TRACE_RATE_LIMIT, "0", .ini_change = zai_config_system_ini_change) \ + CONFIG(INT, DD_TRACE_RATE_LIMIT, "100", .ini_change = zai_config_system_ini_change) \ CONFIG(DOUBLE, DD_TRACE_SAMPLE_RATE, "-1", .ini_change = ddtrace_alter_DD_TRACE_SAMPLE_RATE, \ .env_config_fallback = ddtrace_conf_otel_sample_rate) \ CONFIG(JSON, DD_TRACE_SAMPLING_RULES, "[]") \ From badc35356860917828ecab56511c73ccb94b5c9f Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Mon, 21 Oct 2024 15:40:46 -0400 Subject: [PATCH 2/4] Updating limit to use depending on SAMPLE_RATE value --- ext/configuration.h | 2 +- ext/limiter/limiter.c | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/ext/configuration.h b/ext/configuration.h index 1208433253..57dd53cc7e 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -149,7 +149,7 @@ enum ddtrace_sampling_rules_format { CONFIG(SET, DD_TRACE_RESOURCE_URI_QUERY_PARAM_ALLOWED, "") \ CONFIG(SET, DD_TRACE_HTTP_URL_QUERY_PARAM_ALLOWED, "*") \ CONFIG(SET, DD_TRACE_HTTP_POST_DATA_PARAM_ALLOWED, "") \ - CONFIG(INT, DD_TRACE_RATE_LIMIT, "100", .ini_change = zai_config_system_ini_change) \ + CONFIG(INT, DD_TRACE_RATE_LIMIT, "0", .ini_change = zai_config_system_ini_change) \ CONFIG(DOUBLE, DD_TRACE_SAMPLE_RATE, "-1", .ini_change = ddtrace_alter_DD_TRACE_SAMPLE_RATE, \ .env_config_fallback = ddtrace_conf_otel_sample_rate) \ CONFIG(JSON, DD_TRACE_SAMPLING_RULES, "[]") \ diff --git a/ext/limiter/limiter.c b/ext/limiter/limiter.c index 389a16bf60..212e14f41c 100644 --- a/ext/limiter/limiter.c +++ b/ext/limiter/limiter.c @@ -27,7 +27,15 @@ static ddtrace_limiter* dd_limiter; void ddtrace_limiter_create() { - uint32_t limit = (uint32_t) get_global_DD_TRACE_RATE_LIMIT(); + double sample_rate = (double)get_global_DD_TRACE_SAMPLE_RATE(); // Assuming this is accessible + uint32_t limit; + + // Set limit based on sample rate + if (sample_rate != -1) { + limit = 100; // Set a fixed limit if sample rate is set + } else { + limit = (uint32_t)get_global_DD_TRACE_RATE_LIMIT(); // Get the global limit + } if (!limit) { return; From 5586415f8f5fe570d57ddff69670cab8c58b215f Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Fri, 25 Oct 2024 15:23:57 -0400 Subject: [PATCH 3/4] Updated default value and limiter with sample rate --- ext/configuration.h | 2 +- ext/limiter/limiter.c | 14 +++----------- tests/ext/limiter/002-limiter-reached.phpt | 1 + 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/ext/configuration.h b/ext/configuration.h index 57dd53cc7e..1208433253 100644 --- a/ext/configuration.h +++ b/ext/configuration.h @@ -149,7 +149,7 @@ enum ddtrace_sampling_rules_format { CONFIG(SET, DD_TRACE_RESOURCE_URI_QUERY_PARAM_ALLOWED, "") \ CONFIG(SET, DD_TRACE_HTTP_URL_QUERY_PARAM_ALLOWED, "*") \ CONFIG(SET, DD_TRACE_HTTP_POST_DATA_PARAM_ALLOWED, "") \ - CONFIG(INT, DD_TRACE_RATE_LIMIT, "0", .ini_change = zai_config_system_ini_change) \ + CONFIG(INT, DD_TRACE_RATE_LIMIT, "100", .ini_change = zai_config_system_ini_change) \ CONFIG(DOUBLE, DD_TRACE_SAMPLE_RATE, "-1", .ini_change = ddtrace_alter_DD_TRACE_SAMPLE_RATE, \ .env_config_fallback = ddtrace_conf_otel_sample_rate) \ CONFIG(JSON, DD_TRACE_SAMPLING_RULES, "[]") \ diff --git a/ext/limiter/limiter.c b/ext/limiter/limiter.c index 212e14f41c..ab9631e578 100644 --- a/ext/limiter/limiter.c +++ b/ext/limiter/limiter.c @@ -27,20 +27,12 @@ static ddtrace_limiter* dd_limiter; void ddtrace_limiter_create() { - double sample_rate = (double)get_global_DD_TRACE_SAMPLE_RATE(); // Assuming this is accessible - uint32_t limit; - - // Set limit based on sample rate - if (sample_rate != -1) { - limit = 100; // Set a fixed limit if sample rate is set - } else { - limit = (uint32_t)get_global_DD_TRACE_RATE_LIMIT(); // Get the global limit - } - - if (!limit) { + if (get_global_DD_TRACE_SAMPLE_RATE() < 0) { return; } + uint32_t limit = (uint32_t) get_global_DD_TRACE_RATE_LIMIT(); + // We share the limiter among forks (ie, forks need to write this memory), this requires that we map the memory as shared ddog_ShmHandle *shm; if (!ddtrace_ffi_try("Failed allocating shared memory", ddog_alloc_anon_shm_handle(limit, &shm))) { diff --git a/tests/ext/limiter/002-limiter-reached.phpt b/tests/ext/limiter/002-limiter-reached.phpt index b3a1803ed7..b26685d403 100644 --- a/tests/ext/limiter/002-limiter-reached.phpt +++ b/tests/ext/limiter/002-limiter-reached.phpt @@ -6,6 +6,7 @@ rate limiter reached DD_TRACE_AUTO_FLUSH_ENABLED=0 DD_TRACE_GENERATE_ROOT_SPAN=0 DD_TRACE_RATE_LIMIT=10 +DD_TRACE_SAMPLE_RATE=1 --FILE-- Date: Fri, 25 Oct 2024 15:52:28 -0400 Subject: [PATCH 4/4] Fixing panic and accepting 0 values --- ext/limiter/limiter.c | 6 +++++- tests/ext/limiter/001-limiter-disabled.phpt | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ext/limiter/limiter.c b/ext/limiter/limiter.c index ab9631e578..cc6db08a1c 100644 --- a/ext/limiter/limiter.c +++ b/ext/limiter/limiter.c @@ -27,12 +27,16 @@ static ddtrace_limiter* dd_limiter; void ddtrace_limiter_create() { - if (get_global_DD_TRACE_SAMPLE_RATE() < 0) { + if (zai_config_memoized_entries[DDTRACE_CONFIG_DD_TRACE_SAMPLE_RATE].name_index < 0) { return; } uint32_t limit = (uint32_t) get_global_DD_TRACE_RATE_LIMIT(); + if (!limit) { + return; + } + // We share the limiter among forks (ie, forks need to write this memory), this requires that we map the memory as shared ddog_ShmHandle *shm; if (!ddtrace_ffi_try("Failed allocating shared memory", ddog_alloc_anon_shm_handle(limit, &shm))) { diff --git a/tests/ext/limiter/001-limiter-disabled.phpt b/tests/ext/limiter/001-limiter-disabled.phpt index bfa5f0d2d6..652e677f98 100644 --- a/tests/ext/limiter/001-limiter-disabled.phpt +++ b/tests/ext/limiter/001-limiter-disabled.phpt @@ -3,6 +3,7 @@ rate limiter disabled --ENV-- DD_TRACE_GENERATE_ROOT_SPAN=1 DD_TRACE_RATE_LIMIT=0 +DD_TRACE_SAMPLE_RATE=1 --FILE--