Skip to content

Commit 2b95de2

Browse files
iamlucbwoebi
iamluc
and
bwoebi
authored
fix(loader): surface SSI incompatible runtime (#4021)
* fix(loader): surface SSI incompatible runtime Report tracer disablement from incompatible PHP extensions as incompatible runtime telemetry so SSI diagnostics explain why traces are missing while profiling still works. * test(loader): assert JIT incompat telemetry Cover the OPcache JIT path so the loader reports the same incompatible runtime telemetry as extension-based tracer disablement. * Fix compat Co-authored-by: Bob Weinand <bob.weinand@datadoghq.com> * test(loader): cover xdebug telemetry on PHP 7 Accept the additional profiling incompatible-runtime telemetry emitted on PHP 7.0 when the Xdebug loader scenario runs. --------- Co-authored-by: Bob Weinand <bob.weinand@datadoghq.com>
1 parent 18ca946 commit 2b95de2

3 files changed

Lines changed: 138 additions & 8 deletions

File tree

‎loader/dd_library_loader.c‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ static void *libdatadog_php_handle = NULL;
4040
static unsigned int php_api_no = 0;
4141
static const char *runtime_version = "unknown";
4242
static bool injection_forced = false;
43+
static bool ddtrace_disabled_by_incompatible_runtime = false;
44+
static char ddtrace_disabled_result_reason[384] = {0};
4345

4446
static bool already_done = false;
4547

@@ -73,6 +75,19 @@ PHP_INI_END()
7375

7476
static void ddloader_telemetryf(telemetry_reason reason, injected_ext *config, const char *error, const char *format, ...);
7577

78+
static void ddloader_set_ddtrace_disabled_by_incompatible_runtime(const char *format, ...) {
79+
if (ddtrace_disabled_by_incompatible_runtime) {
80+
return;
81+
}
82+
83+
ddtrace_disabled_by_incompatible_runtime = true;
84+
85+
va_list va;
86+
va_start(va, format);
87+
vsnprintf(ddtrace_disabled_result_reason, sizeof(ddtrace_disabled_result_reason), format, va);
88+
va_end(va);
89+
}
90+
7691
static char *ddtrace_pre_load_hook(injected_ext *config) {
7792
// Load libdatadog_php.so, on which ddtrace.so implicitly depends. Implicit
7893
// because there's no DT_NEEDED(libdatadog_php.so) entry in ddtrace.so.
@@ -225,16 +240,17 @@ static void ddtrace_pre_minit_hook(injected_ext *config, zend_module_entry *modu
225240
}
226241

227242
// Load, but disable the tracer if runtime configuration is not safe for auto-injection
228-
bool disable_tracer = false;
229-
230243
char *incompatible_exts[] = {"Xdebug", "the ionCube PHP Loader", "ionCube Loader", "the ionCube PHP Loader + ionCube24", "newrelic", "blackfire", "pcov"};
231244
for (size_t i = 0; i < sizeof(incompatible_exts) / sizeof(incompatible_exts[0]); ++i) {
232245
if (ddloader_is_ext_loaded(incompatible_exts[i])) {
233246
if (force_load) {
234247
LOG(config, WARN, "Potentially incompatible extension detected: %s. Ignoring as DD_INJECT_FORCE is enabled", incompatible_exts[i]);
235248
} else {
236249
LOG(config, WARN, "Potentially incompatible extension detected: %s. ddtrace will be disabled unless the environment DD_INJECT_FORCE is set to '1', 'true', 'yes' or 'on'", incompatible_exts[i]);
237-
disable_tracer = true;
250+
ddloader_set_ddtrace_disabled_by_incompatible_runtime(
251+
"The PHP tracer was disabled because potentially incompatible extension '%s' is loaded. Set DD_INJECT_FORCE to force tracing.",
252+
incompatible_exts[i]
253+
);
238254
}
239255
}
240256
}
@@ -244,11 +260,13 @@ static void ddtrace_pre_minit_hook(injected_ext *config, zend_module_entry *modu
244260
LOG(config, WARN, "OPcache JIT is enabled and may cause instability. Ignoring as DD_INJECT_FORCE is enabled");
245261
} else {
246262
LOG(config, WARN, "OPcache JIT is enabled and may cause instability. ddtrace will be disabled unless the environment DD_INJECT_FORCE is set to '1', 'true', 'yes' or 'on'");
247-
disable_tracer = true;
263+
ddloader_set_ddtrace_disabled_by_incompatible_runtime(
264+
"The PHP tracer was disabled because OPcache JIT is enabled. Set DD_INJECT_FORCE to force tracing."
265+
);
248266
}
249267
}
250268

251-
if (disable_tracer) {
269+
if (ddtrace_disabled_by_incompatible_runtime) {
252270
ddloader_ini_set_configuration(config, ZEND_STRL("ddtrace.disable"), ZEND_STRL("1"));
253271
}
254272

@@ -728,6 +746,8 @@ static PHP_MINIT_FUNCTION(ddloader_injected_extension_minit) {
728746
zend_result ret = module->module_startup_func(INIT_FUNC_ARGS_PASSTHRU);
729747
if (ret == FAILURE) {
730748
TELEMETRY(REASON_ERROR, config, "error_minit", "'%s' MINIT function failed", config->ext_name);
749+
} else if (strcmp(config->ext_name, "ddtrace") == 0 && ddtrace_disabled_by_incompatible_runtime) {
750+
TELEMETRY(REASON_INCOMPATIBLE_RUNTIME, config, NULL, "%s", ddtrace_disabled_result_reason)
731751
} else {
732752
TELEMETRY(REASON_COMPLETE, config, NULL, "Application instrumentation bootstrapping complete ('%s')", config->ext_name)
733753
}
@@ -1028,6 +1048,8 @@ static void ddloader_zend_extension_shutdown(zend_extension *ext) {
10281048
}
10291049

10301050
injection_forced = false;
1051+
ddtrace_disabled_by_incompatible_runtime = false;
1052+
*ddtrace_disabled_result_reason = 0;
10311053
already_done = false;
10321054
}
10331055

‎loader/tests/functional/test_incompatibility_jit.php‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
$msg_disabled = "OPcache JIT is enabled and may cause instability. ddtrace will be disabled unless the environment DD_INJECT_FORCE is set to '1', 'true', 'yes' or 'on'";
1313
$msg_forced = "OPcache JIT is enabled and may cause instability. Ignoring as DD_INJECT_FORCE is enabled";
14+
$jitTelemetryLogPath = tempnam(sys_get_temp_dir(), 'test_loader_');
1415

1516
$tests = [
1617
// OPcache disabled in CLI
@@ -73,6 +74,10 @@
7374
// JIT enabled
7475
[
7576
"config" => "-dzend_extension=opcache -dopcache.enable_cli=1 -ddatadog.trace.cli_enabled=1 -dopcache.jit_buffer_size=32M -dopcache.jit=tracing",
77+
"env" => [
78+
'FAKE_FORWARDER_LOG_PATH='.$jitTelemetryLogPath,
79+
'DD_TELEMETRY_FORWARDER_PATH='.__DIR__.'/../../bin/fake_forwarder.sh',
80+
],
7681
"must_not_contain" => [],
7782
"must_contain" => [
7883
$msg_disabled,
@@ -94,8 +99,8 @@
9499
95100
=> ddtrace
96101
Version => %s
97-
Injection success => true
98-
Injection error =>
102+
Injection success => false
103+
Injection error => Incompatible runtime
99104
Extra config => datadog.trace.sources_path=%s/trace/src
100105
ddtrace.disable=1
101106
@@ -104,6 +109,34 @@
104109
%A
105110
EOT
106111
],
112+
"telemetry_log_path" => $jitTelemetryLogPath,
113+
"telemetry" => <<<EOS
114+
{
115+
"metadata": {
116+
"runtime_name": "php",
117+
"runtime_version": "%d.%d.%d%S",
118+
"language_name": "php",
119+
"language_version": "%d.%d.%d%S",
120+
"tracer_version": "%s",
121+
"pid": %d,
122+
"result": "abort",
123+
"result_reason": "The PHP tracer was disabled because OPcache JIT is enabled. Set DD_INJECT_FORCE to force tracing.",
124+
"result_class": "incompatible_runtime"
125+
},
126+
"points": [
127+
{
128+
"name": "library_entrypoint.abort",
129+
"tags": [
130+
"reason:incompatible_runtime",
131+
"product:ddtrace"
132+
]
133+
},
134+
{
135+
"name": "library_entrypoint.abort.runtime"
136+
}
137+
]
138+
}
139+
EOS
107140
],
108141
// JIT enabled + force injection via ENV
109142
[
@@ -198,4 +231,9 @@
198231
foreach ($data['must_match'] as $pattern) {
199232
assertMatchesFormat($output, $pattern);
200233
}
234+
if (isset($data['telemetry'])) {
235+
// Let time to the fork to write the telemetry log
236+
usleep(5000);
237+
assertTelemetry($data['telemetry_log_path'], $data['telemetry']);
238+
}
201239
}

‎loader/tests/functional/test_incompatibility_xdebug.php‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,84 @@
1111
$msg_disabled = "Potentially incompatible extension detected: Xdebug. ddtrace will be disabled unless the environment DD_INJECT_FORCE is set to '1', 'true', 'yes' or 'on'";
1212
$msg_forced = "Potentially incompatible extension detected: Xdebug. Ignoring as DD_INJECT_FORCE is enabled";
1313

14-
$output = runCLI('-dzend_extension='.getenv("XDEBUG_SO_NAME").' -v', true, ['DD_TRACE_DEBUG=1']);
14+
$telemetryLogPath = tempnam(sys_get_temp_dir(), 'test_loader_');
15+
16+
$output = runCLI('-dzend_extension='.getenv("XDEBUG_SO_NAME").' -v', true, [
17+
'DD_TRACE_DEBUG=1',
18+
'FAKE_FORWARDER_LOG_PATH='.$telemetryLogPath,
19+
'DD_TELEMETRY_FORWARDER_PATH='.__DIR__.'/../../bin/fake_forwarder.sh',
20+
]);
1521
assertContains($output, 'Found extension file');
1622
assertContains($output, $msg_disabled);
1723
assertNotContains($output, $msg_forced);
1824
assertContains($output, 'with dd_library_loader v');
1925
assertContains($output, 'with Xdebug v');
2026
assertContains($output, 'with ddtrace v');
2127

28+
// Let time to the fork to write the telemetry log
29+
usleep(5000);
30+
31+
$metrics = [<<<EOS
32+
{
33+
"metadata": {
34+
"runtime_name": "php",
35+
"runtime_version": "%d.%d.%d%S",
36+
"language_name": "php",
37+
"language_version": "%d.%d.%d%S",
38+
"tracer_version": "%s",
39+
"pid": %d,
40+
"result": "abort",
41+
"result_reason": "The PHP tracer was disabled because potentially incompatible extension 'Xdebug' is loaded. Set DD_INJECT_FORCE to force tracing.",
42+
"result_class": "incompatible_runtime"
43+
},
44+
"points": [
45+
{
46+
"name": "library_entrypoint.abort",
47+
"tags": [
48+
"reason:incompatible_runtime",
49+
"product:ddtrace"
50+
]
51+
},
52+
{
53+
"name": "library_entrypoint.abort.runtime"
54+
}
55+
]
56+
}
57+
EOS
58+
];
59+
60+
if ('7.0' === php_minor_version()) {
61+
$metrics[] = <<<EOS
62+
{
63+
"metadata": {
64+
"runtime_name": "php",
65+
"runtime_version": "%d.%d.%d%S",
66+
"language_name": "php",
67+
"language_version": "%d.%d.%d%S",
68+
"tracer_version": "%s",
69+
"pid": %d,
70+
"result": "abort",
71+
"result_reason": "%s",
72+
"result_class": "incompatible_runtime"
73+
},
74+
"points": [
75+
{
76+
"name": "library_entrypoint.abort",
77+
"tags": [
78+
"reason:incompatible_runtime",
79+
"product:datadog-profiling"
80+
]
81+
},
82+
{
83+
"name": "library_entrypoint.abort.runtime"
84+
}
85+
]
86+
}
87+
EOS;
88+
}
89+
90+
assertTelemetry($telemetryLogPath, $metrics);
91+
2292
$output = runCLI('-dzend_extension='.getenv("XDEBUG_SO_NAME").' -v', true, ['DD_TRACE_DEBUG=1', 'DD_INJECT_FORCE=1']);
2393
assertContains($output, 'Found extension file');
2494
assertNotContains($output, $msg_disabled);

0 commit comments

Comments
 (0)