Skip to content

Commit bfbcb5b

Browse files
morrisonleviclaude
andauthored
perf(config): cache sys getenv (#3670)
* perf(config): cache sys getenv In rinit, calling system getenv when a SAPI env var isn't found is slow enough it shows up in profiles. glibc's getenv does a linear scan on the environ and does strncmp on the members, so we can definitely do better. We save it in minit so it's available for SYSTEM_INI variables and again on the first request. This is also arguably more correct: system environment variables map to a system INI setting, which shouldn't change at runtime. * perf(config): avoid interim string copies Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9cde556 commit bfbcb5b

16 files changed

Lines changed: 421 additions & 271 deletions

File tree

ext/otel_config.c

Lines changed: 47 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,22 @@ static void report_otel_cfg_telemetry_invalid(const char *otel_cfg, const char *
2020
}
2121
}
2222

23-
static bool get_otel_value(zai_str str, zai_env_buffer buf, bool pre_rinit) {
24-
if (zai_getenv_ex(str, buf, pre_rinit) == ZAI_ENV_SUCCESS) {
25-
return true;
26-
}
23+
static bool get_otel_value(zai_str str, zai_env_buffer *buf, bool pre_rinit) {
24+
if (!pre_rinit && zai_sapi_getenv(str, buf) == ZAI_ENV_SUCCESS) return true;
25+
zai_option_str sys = zai_sys_getenv(str);
26+
if (zai_option_str_is_some(sys)) { buf->ptr = sys.ptr; buf->len = sys.len; return true; }
2727

2828
zval *cfg = cfg_get_entry(str.ptr, str.len);
2929
if (cfg) {
3030
if (Z_TYPE_P(cfg) == IS_ARRAY) {
3131
zval *val;
32-
char *off = buf.ptr;
32+
char *off = buf->ptr;
3333
ZEND_HASH_FOREACH_VAL(Z_ARR_P(cfg), val) {
3434
if (Z_TYPE_P(val) == IS_STRING) {
35-
if (off - buf.ptr + Z_STRLEN_P(val) + 2 >= ZAI_ENV_MAX_BUFSIZ) {
35+
if (off - buf->ptr + Z_STRLEN_P(val) + 2 >= ZAI_ENV_MAX_BUFSIZ) {
3636
return false;
3737
}
38-
if (off != buf.ptr) {
38+
if (off != buf->ptr) {
3939
*off++ = ',';
4040
}
4141
memcpy(off, Z_STRVAL_P(val), Z_STRLEN_P(val));
@@ -46,29 +46,30 @@ static bool get_otel_value(zai_str str, zai_env_buffer buf, bool pre_rinit) {
4646
} else if (Z_STRLEN_P(cfg) == 0 || Z_STRLEN_P(cfg) + 1 >= ZAI_ENV_MAX_BUFSIZ) {
4747
return false;
4848
} else {
49-
memcpy(buf.ptr, Z_STRVAL_P(cfg), Z_STRLEN_P(cfg) + 1);
49+
memcpy(buf->ptr, Z_STRVAL_P(cfg), Z_STRLEN_P(cfg) + 1);
5050
}
5151
return true;
5252
}
5353

5454
return false;
5555
}
5656

57-
static bool ddtrace_conf_otel_resource_attributes_special(const char *tag, int len, zai_env_buffer buf, bool pre_rinit) {
58-
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_RESOURCE_ATTRIBUTES"), buf, pre_rinit)) {
57+
static bool ddtrace_conf_otel_resource_attributes_special(const char *tag, int len, zai_env_buffer *buf, bool pre_rinit) {
58+
ZAI_ENV_BUFFER_INIT(local, ZAI_ENV_MAX_BUFSIZ);
59+
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_RESOURCE_ATTRIBUTES"), &local, pre_rinit)) {
5960
return false;
6061
}
6162

62-
for (char *cur = buf.ptr, *key_start = cur; *cur; ++cur) {
63+
for (char *cur = local.ptr, *key_start = cur; *cur; ++cur) {
6364
if (*cur == '=') {
6465
char *key_end = cur++;
6566
while (*cur && *cur != ',') {
6667
++cur;
6768
}
6869
if (key_end - key_start == len && memcmp(key_start, tag, len) == 0 && key_end[1]) {
6970
size_t vallen = cur - (key_end + 1);
70-
memcpy(buf.ptr, key_end + 1, vallen);
71-
buf.ptr[vallen] = 0;
71+
memcpy(buf->ptr, key_end + 1, vallen);
72+
buf->ptr[vallen] = 0;
7273
return true;
7374
}
7475
key_start = cur-- + 1;
@@ -78,92 +79,95 @@ static bool ddtrace_conf_otel_resource_attributes_special(const char *tag, int l
7879
return false;
7980
}
8081

81-
bool ddtrace_conf_otel_resource_attributes_env(zai_env_buffer buf, bool pre_rinit) {
82+
bool ddtrace_conf_otel_resource_attributes_env(zai_env_buffer *buf, bool pre_rinit) {
8283
return ddtrace_conf_otel_resource_attributes_special(ZEND_STRL("deployment.environment"), buf, pre_rinit);
8384
}
8485

85-
bool ddtrace_conf_otel_resource_attributes_version(zai_env_buffer buf, bool pre_rinit) {
86+
bool ddtrace_conf_otel_resource_attributes_version(zai_env_buffer *buf, bool pre_rinit) {
8687
return ddtrace_conf_otel_resource_attributes_special(ZEND_STRL("service.version"), buf, pre_rinit);
8788
}
8889

89-
bool ddtrace_conf_otel_service_name(zai_env_buffer buf, bool pre_rinit) {
90+
bool ddtrace_conf_otel_service_name(zai_env_buffer *buf, bool pre_rinit) {
9091
return get_otel_value((zai_str)ZAI_STRL("OTEL_SERVICE_NAME"), buf, pre_rinit)
9192
|| ddtrace_conf_otel_resource_attributes_special(ZEND_STRL("service.name"), buf, pre_rinit);
9293
}
9394

94-
bool ddtrace_conf_otel_log_level(zai_env_buffer buf, bool pre_rinit) {
95+
bool ddtrace_conf_otel_log_level(zai_env_buffer *buf, bool pre_rinit) {
9596
return get_otel_value((zai_str)ZAI_STRL("OTEL_LOG_LEVEL"), buf, pre_rinit);
9697
}
9798

98-
bool ddtrace_conf_otel_propagators(zai_env_buffer buf, bool pre_rinit) {
99-
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_PROPAGATORS"), buf, pre_rinit)) {
99+
bool ddtrace_conf_otel_propagators(zai_env_buffer *buf, bool pre_rinit) {
100+
ZAI_ENV_BUFFER_INIT(local, ZAI_ENV_MAX_BUFSIZ);
101+
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_PROPAGATORS"), &local, pre_rinit)) {
100102
return false;
101103
}
102-
char *off = (char *)zend_memnstr(buf.ptr, ZEND_STRL("b3"), buf.ptr + strlen(buf.ptr));
103-
if (off && (!off[strlen("b3")] || off[strlen("b3")] == ',') && strlen(buf.ptr) < buf.len - 100) {
104-
memmove(off + strlen("b3 single header"), off + strlen("b3"), buf.ptr + strlen(buf.ptr) - (off + strlen("b3")) + 1);
104+
memcpy(buf->ptr, local.ptr, strlen(local.ptr) + 1);
105+
char *off = (char *)zend_memnstr(buf->ptr, ZEND_STRL("b3"), buf->ptr + strlen(buf->ptr));
106+
if (off && (!off[strlen("b3")] || off[strlen("b3")] == ',') && strlen(buf->ptr) < buf->len - 100) {
107+
memmove(off + strlen("b3 single header"), off + strlen("b3"), buf->ptr + strlen(buf->ptr) - (off + strlen("b3")) + 1);
105108
memcpy(off, "b3 single header", strlen("b3 single header"));
106109
}
107110
return true;
108111
}
109112

110-
bool ddtrace_conf_otel_sample_rate(zai_env_buffer buf, bool pre_rinit) {
113+
bool ddtrace_conf_otel_sample_rate(zai_env_buffer *buf, bool pre_rinit) {
111114
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_TRACES_SAMPLER"), buf, pre_rinit)) {
112115
return false;
113116
}
114117

115-
if (strcmp(buf.ptr, "always_on") == 0 || strcmp(buf.ptr, "parentbased_always_on") == 0) {
116-
memcpy(buf.ptr, ZEND_STRS("1"));
118+
if (strcmp(buf->ptr, "always_on") == 0 || strcmp(buf->ptr, "parentbased_always_on") == 0) {
119+
buf->ptr = "1"; buf->len = 1;
117120
return true;
118121
}
119-
if (strcmp(buf.ptr, "always_off") == 0 || strcmp(buf.ptr, "parentbased_always_off") == 0) {
120-
memcpy(buf.ptr, ZEND_STRS("0"));
122+
if (strcmp(buf->ptr, "always_off") == 0 || strcmp(buf->ptr, "parentbased_always_off") == 0) {
123+
buf->ptr = "0"; buf->len = 1;
121124
return true;
122125
}
123-
if (strcmp(buf.ptr, "traceidratio") == 0 || strcmp(buf.ptr, "parentbased_traceidratio") == 0) {
126+
if (strcmp(buf->ptr, "traceidratio") == 0 || strcmp(buf->ptr, "parentbased_traceidratio") == 0) {
124127
if (get_otel_value((zai_str)ZAI_STRL("OTEL_TRACES_SAMPLER_ARG"), buf, pre_rinit)) {
125128
return true;
126129
}
127-
LOG_ONCE(WARN, "OTEL_TRACES_SAMPLER is %s, but is missing OTEL_TRACES_SAMPLER_ARG", buf.ptr);
130+
LOG_ONCE(WARN, "OTEL_TRACES_SAMPLER is %s, but is missing OTEL_TRACES_SAMPLER_ARG", buf->ptr);
128131
} else {
129-
LOG_ONCE(WARN, "OTEL_TRACES_SAMPLER has invalid value: %s", buf.ptr);
132+
LOG_ONCE(WARN, "OTEL_TRACES_SAMPLER has invalid value: %s", buf->ptr);
130133
}
131134
report_otel_cfg_telemetry_invalid("otel_traces_sampler", "dd_trace_sample_rate", pre_rinit);
132135
return false;
133136
}
134137

135-
bool ddtrace_conf_otel_traces_exporter(zai_env_buffer buf, bool pre_rinit) {
138+
bool ddtrace_conf_otel_traces_exporter(zai_env_buffer *buf, bool pre_rinit) {
136139
if (get_otel_value((zai_str)ZAI_STRL("OTEL_TRACES_EXPORTER"), buf, pre_rinit)) {
137-
if (strcmp(buf.ptr, "none") == 0) {
138-
memcpy(buf.ptr, ZEND_STRS("0"));
140+
if (strcmp(buf->ptr, "none") == 0) {
141+
buf->ptr = "0"; buf->len = 1;
139142
return true;
140143
}
141-
LOG_ONCE(WARN, "OTEL_TRACES_EXPORTER has invalid value: %s", buf.ptr);
144+
LOG_ONCE(WARN, "OTEL_TRACES_EXPORTER has invalid value: %s", buf->ptr);
142145
report_otel_cfg_telemetry_invalid("otel_traces_exporter", "dd_trace_enabled", pre_rinit);
143146
}
144147
return false;
145148
}
146149

147-
bool ddtrace_conf_otel_metrics_exporter(zai_env_buffer buf, bool pre_rinit) {
150+
bool ddtrace_conf_otel_metrics_exporter(zai_env_buffer *buf, bool pre_rinit) {
148151
if (get_otel_value((zai_str)ZAI_STRL("OTEL_METRICS_EXPORTER"), buf, pre_rinit)) {
149-
if (strcmp(buf.ptr, "none") == 0) {
150-
memcpy(buf.ptr, ZEND_STRS("0"));
152+
if (strcmp(buf->ptr, "none") == 0) {
153+
buf->ptr = "0"; buf->len = 1;
151154
return true;
152155
}
153-
LOG_ONCE(WARN, "OTEL_METRICS_EXPORTER has invalid value: %s", buf.ptr);
156+
LOG_ONCE(WARN, "OTEL_METRICS_EXPORTER has invalid value: %s", buf->ptr);
154157
report_otel_cfg_telemetry_invalid("otel_metrics_exporter", "dd_integration_metrics_enabled", pre_rinit);
155158
}
156159
return false;
157160
}
158161

159-
bool ddtrace_conf_otel_resource_attributes_tags(zai_env_buffer buf, bool pre_rinit) {
160-
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_RESOURCE_ATTRIBUTES"), buf, pre_rinit)) {
162+
bool ddtrace_conf_otel_resource_attributes_tags(zai_env_buffer *buf, bool pre_rinit) {
163+
ZAI_ENV_BUFFER_INIT(local, ZAI_ENV_MAX_BUFSIZ);
164+
if (!get_otel_value((zai_str)ZAI_STRL("OTEL_RESOURCE_ATTRIBUTES"), &local, pre_rinit)) {
161165
return false;
162166
}
163167

164-
char *out = buf.ptr;
168+
char *out = buf->ptr;
165169
int tags = 0;
166-
for (char *cur = buf.ptr, *key_start = cur; *cur; ++cur) {
170+
for (char *cur = local.ptr, *key_start = cur; *cur; ++cur) {
167171
if (*cur == '=') {
168172
char *key = key_start, *key_end = cur++;
169173
while (*cur && *cur != ',') {
@@ -190,7 +194,7 @@ bool ddtrace_conf_otel_resource_attributes_tags(zai_env_buffer buf, bool pre_rin
190194
--cur;
191195
}
192196
}
193-
if (out != buf.ptr) {
197+
if (out != buf->ptr) {
194198
--out;
195199
}
196200
*out = 0;

ext/otel_config.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33

44
#include <env/env.h>
55

6-
bool ddtrace_conf_otel_resource_attributes_env(zai_env_buffer buf, bool pre_rinit);
7-
bool ddtrace_conf_otel_resource_attributes_version(zai_env_buffer buf, bool pre_rinit);
8-
bool ddtrace_conf_otel_service_name(zai_env_buffer buf, bool pre_rinit);
9-
bool ddtrace_conf_otel_log_level(zai_env_buffer buf, bool pre_rinit);
10-
bool ddtrace_conf_otel_propagators(zai_env_buffer buf, bool pre_rinit);
11-
bool ddtrace_conf_otel_sample_rate(zai_env_buffer buf, bool pre_rinit);
12-
bool ddtrace_conf_otel_traces_exporter(zai_env_buffer buf, bool pre_rinit);
13-
bool ddtrace_conf_otel_metrics_exporter(zai_env_buffer buf, bool pre_rinit);
14-
bool ddtrace_conf_otel_resource_attributes_tags(zai_env_buffer buf, bool pre_rinit);
6+
bool ddtrace_conf_otel_resource_attributes_env(zai_env_buffer *buf, bool pre_rinit);
7+
bool ddtrace_conf_otel_resource_attributes_version(zai_env_buffer *buf, bool pre_rinit);
8+
bool ddtrace_conf_otel_service_name(zai_env_buffer *buf, bool pre_rinit);
9+
bool ddtrace_conf_otel_log_level(zai_env_buffer *buf, bool pre_rinit);
10+
bool ddtrace_conf_otel_propagators(zai_env_buffer *buf, bool pre_rinit);
11+
bool ddtrace_conf_otel_sample_rate(zai_env_buffer *buf, bool pre_rinit);
12+
bool ddtrace_conf_otel_traces_exporter(zai_env_buffer *buf, bool pre_rinit);
13+
bool ddtrace_conf_otel_metrics_exporter(zai_env_buffer *buf, bool pre_rinit);
14+
bool ddtrace_conf_otel_resource_attributes_tags(zai_env_buffer *buf, bool pre_rinit);
1515

1616
#endif // DD_OTEL_CONFIG_H

zend_abstract_interface/config/config.c

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <assert.h>
44
#include <json/json.h>
55
#include <main/php.h>
6+
#include <main/SAPI.h>
67
#include <stdbool.h>
78
#include <stdlib.h>
89
#include <string.h>
@@ -12,11 +13,34 @@ HashTable zai_config_name_map = {0};
1213
uint16_t zai_config_memoized_entries_count = 0;
1314
zai_config_memoized_entry zai_config_memoized_entries[ZAI_CONFIG_ENTRIES_COUNT_MAX];
1415

15-
static bool zai_config_get_env_value(zai_str name, zai_env_buffer buf) {
16-
// TODO Handle other return codes
17-
// We want to explicitly allow pre-RINIT access to env vars here. So that callers can have an early view at config.
18-
// But in general allmost all configurations shall only be accessed after first RINIT. (the trivial getter will
19-
return zai_getenv_ex(name, buf, true) == ZAI_ENV_SUCCESS;
16+
// Indexed [config_id][name_index]; NULL means not set.
17+
static char *zai_config_cached_sys_env[ZAI_CONFIG_ENTRIES_COUNT_MAX][ZAI_CONFIG_NAMES_COUNT_MAX];
18+
19+
const char *zai_config_sys_env_cached(zai_config_id id, uint8_t name_index) {
20+
return zai_config_cached_sys_env[id][name_index];
21+
}
22+
23+
static void zai_config_cache_sys_env(void) {
24+
for (zai_config_id i = 0; i < zai_config_memoized_entries_count; i++) {
25+
zai_config_memoized_entry *m = &zai_config_memoized_entries[i];
26+
for (uint8_t n = 0; n < m->names_count; n++) {
27+
zai_str name = ZAI_STR_NEW(m->names[n].ptr, m->names[n].len);
28+
zai_option_str val = zai_sys_getenv(name);
29+
zai_config_cached_sys_env[i][n] = zai_option_str_is_some(val)
30+
? pestrdup(val.ptr, 1) : NULL;
31+
}
32+
}
33+
}
34+
35+
static void zai_config_clear_sys_env_cache(void) {
36+
for (zai_config_id i = 0; i < zai_config_memoized_entries_count; i++) {
37+
for (uint8_t n = 0; n < zai_config_memoized_entries[i].names_count; n++) {
38+
if (zai_config_cached_sys_env[i][n]) {
39+
pefree(zai_config_cached_sys_env[i][n], 1);
40+
zai_config_cached_sys_env[i][n] = NULL;
41+
}
42+
}
43+
}
2044
}
2145

2246
static inline void zai_config_process_env(zai_config_memoized_entry *memoized, zai_env_buffer buf, zai_option_str *value) {
@@ -31,7 +55,7 @@ static inline void zai_config_process_env(zai_config_memoized_entry *memoized, z
3155
}
3256
}
3357

34-
static void zai_config_find_and_set_value(zai_config_memoized_entry *memoized, zai_config_id id) {
58+
static void zai_config_find_and_set_value(zai_config_memoized_entry *memoized, zai_config_id id, bool in_request) {
3559
// TODO Use less buffer space
3660
// TODO Make a more generic zai_string_buffer
3761
ZAI_ENV_BUFFER_INIT(buf, ZAI_ENV_MAX_BUFSIZ);
@@ -48,18 +72,34 @@ static void zai_config_find_and_set_value(zai_config_memoized_entry *memoized, z
4872
name_index = ZAI_CONFIG_ORIGIN_FLEET_STABLE;
4973
memoized->config_id = (zai_str) ZAI_STR_FROM_ZSTR(entry->config_id);
5074
break;
51-
} else if (zai_config_get_env_value(name, buf)) {
52-
zai_config_process_env(memoized, buf, &value);
53-
break;
54-
} else if (entry && entry->source == DDOG_LIBRARY_CONFIG_SOURCE_LOCAL_STABLE_CONFIG) {
75+
} else {
76+
// SAPI env (e.g. Apache SetEnv) takes priority over the sys env
77+
// cache and must be checked here at first RINIT, not only in
78+
// zai_config_ini_rinit. Code that runs between first_time_rinit
79+
// and zai_config_ini_rinit--such as the signal handler setup that
80+
// reads DD_TRACE_HEALTH_METRICS_ENABLED--relies on SAPI-provided
81+
// values being present in the decoded config.
82+
if (in_request && zai_sapi_getenv(name, &buf) == ZAI_ENV_SUCCESS) {
83+
zai_config_process_env(memoized, buf, &value);
84+
break;
85+
}
86+
const char *cached = zai_config_sys_env_cached(id, name_index);
87+
if (cached) {
88+
buf.ptr = (char *)cached;
89+
buf.len = strlen(cached);
90+
zai_config_process_env(memoized, buf, &value);
91+
break;
92+
}
93+
}
94+
if (entry && entry->source == DDOG_LIBRARY_CONFIG_SOURCE_LOCAL_STABLE_CONFIG) {
5595
strcpy(buf.ptr, ZSTR_VAL(entry->value));
5696
zai_config_process_env(memoized, buf, &value);
5797
name_index = ZAI_CONFIG_ORIGIN_LOCAL_STABLE;
5898
memoized->config_id = (zai_str) ZAI_STR_FROM_ZSTR(entry->config_id);
5999
break;
60100
}
61101
}
62-
if (!value.len && memoized->env_config_fallback && memoized->env_config_fallback(buf, true)) {
102+
if (!value.len && memoized->env_config_fallback && memoized->env_config_fallback(&buf, true)) {
63103
zai_config_process_env(memoized, buf, &value);
64104
name_index = ZAI_CONFIG_ORIGIN_MODIFIED;
65105
}
@@ -153,6 +193,7 @@ bool zai_config_minit(zai_config_entry entries[], size_t entries_count, zai_conf
153193
#if PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 70400
154194
zai_persistent_new_interned_string = zend_new_interned_string;
155195
#endif
196+
zai_config_cache_sys_env();
156197
return true;
157198
}
158199

@@ -164,11 +205,13 @@ static void zai_config_dtor_memoized_zvals(void) {
164205

165206
void zai_config_mshutdown(void) {
166207
zai_config_dtor_memoized_zvals();
208+
zai_config_memoized_entries_count = 0;
167209
if (zai_config_name_map.nTableSize) {
168210
zend_hash_destroy(&zai_config_name_map);
169211
}
170212
zai_config_ini_mshutdown();
171213
zai_config_stable_file_mshutdown();
214+
zai_config_clear_sys_env_cache();
172215
}
173216

174217
void zai_config_runtime_config_ctor(void);
@@ -237,9 +280,16 @@ void zai_config_first_time_rinit(bool in_request) {
237280
(void)in_request;
238281
#endif
239282

283+
// Non-CLI SAPIs (CGI/FPM/mod_php) may inject env vars before the first
284+
// request, so refresh the cache to pick them up.
285+
if (in_request && strcmp(sapi_module.name, "cli") != 0) {
286+
zai_config_clear_sys_env_cache();
287+
zai_config_cache_sys_env();
288+
}
289+
240290
for (uint16_t i = 0; i < zai_config_memoized_entries_count; i++) {
241291
zai_config_memoized_entry *memoized = &zai_config_memoized_entries[i];
242-
zai_config_find_and_set_value(memoized, i);
292+
zai_config_find_and_set_value(memoized, i, in_request);
243293
#if PHP_VERSION_ID >= 70300
244294
zai_config_intern_zval(&memoized->decoded_value);
245295
#else

zend_abstract_interface/config/config.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ zval *zai_config_get_value(zai_config_id id);
110110

111111
bool zai_config_get_id_by_name(zai_str name, zai_config_id *id);
112112

113+
// Returns NULL if not set; persistent allocation, must not be freed.
114+
const char *zai_config_sys_env_cached(zai_config_id id, uint8_t name_index);
115+
113116
// Adds name to name<->id mapping. Id may be present multiple times.
114117
void zai_config_register_config_id(zai_config_name *name, zai_config_id id);
115118

0 commit comments

Comments
 (0)