Add line-hooking by bwoebi · Pull Request #4275 · DataDog/dd-trace-php

bwoebi · 2026-10-05T19:53:42Z

DDTrace\install_line_hook(file, line, begin, endLine, end) runs callbacks before a source line, or around a line range, and hands them a LineHookData exposing the instrumented frame's variables. Ranges close on every exit from the range, including exceptional ones. It returns negative ids (accepted by remove_hook()) distinguishing it from function hooks.

interceptor/line_hook.c arms a site by replacing the opline's VM handler with a trampoline jumping to the original afterward. Records for op_arrays in OPcache's shared segments are refcounted in a shared mapping so concurrent workers can arm and restore the same opline, and zai_line_hook_handle_fork() reacquires the child's references.

hook/line_hooks.c has a lot of machinery to find the entry point of hooks. Sadly this requires a full scan for the filenames and a full forward scan through the opcodes to find the begin and end lines, including identifying exit points.

DDTrace\install_line_hook(file, line, begin, endLine, end) runs callbacks before a source line, or around a line range, and hands them a LineHookData exposing the instrumented frame's variables. Ranges close on every exit from the range, including exceptional ones. It returns negative ids (accepted by remove_hook()) distinguishing it from function hooks. interceptor/line_hook.c arms a site by replacing the opline's VM handler with a trampoline jumping to the original afterward. Records for op_arrays in OPcache's shared segments are refcounted in a shared mapping so concurrent workers can arm and restore the same opline, and zai_line_hook_handle_fork() reacquires the child's references. hook/line_hooks.c has a lot of machinery to find the entry point of hooks. Sadly this requires a full scan for the filenames and a full forward scan through the opcodes to find the begin and end lines, including identifying exit points. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

chatgpt-codex-connector · 2026-10-05T19:53:59Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-05T20:00:40.589533Z	`aa92367`	PR opened
🔒 Security Review	✅ Completed 2026-10-05T20:01:01.049382Z	`aa92367`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

datadog-official · 2026-10-05T19:59:34Z

✨ Unblock PR with BitsAI

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 113 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-php | php-app.amd64.DOC: [public.ecr.aws/lts/ubuntu:22.04, linux/amd64, 5.6] — ❌ 1 test failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab
❌ tests.docker_ssi.test_docker_ssi.TestDockerSSIFeatures.test_telemetry_abort[php-app] from system_tests_suite
assert 0 &gt;= 1
 &#43;  where 0 = len([])

self = &lt;tests.docker_ssi.test_docker_ssi.TestDockerSSIFeatures object at 0x7f500296ed20&gt;

    @features.ssi_guardrails
    @irrelevant(context.library == &#34;java&#34; and context.installed_language_runtime &gt;= &#34;1.8.0_0&#34;)
    @irrelevant(context.library == &#34;php&#34; and context.installed_language_runtime &gt;= &#34;7.0&#34;)
    @bug(context.library == &#34;nodejs&#34; and context.installed_language_runtime &lt; &#34;12.17.0&#34;, reason=&#34;INPLAT-252&#34;)
    @bug(context.library == &#34;java&#34; and context.installed_language_runtime == &#34;1.7.0-201&#34;, reason=&#34;INPLAT-427&#34;)
...
DataDog/apm-reliability/dd-trace-php | php-app.amd64.DOC: [public.ecr.aws/lts/ubuntu:22.04, linux/amd64, 7.2] — ❄️ 2 known flaky tests

View more details · View in GitLab

↳ ❄️ Known flaky: tests.docker_ssi.test_docker_ssi.TestDockerSSIFeatures.test_install_supported_runtime[php-app] from system_tests_suite

↳ ❄️ Known flaky: tests.docker_ssi.test_docker_ssi.TestDockerSSIFeatures.test_telemetry[php-app] from system_tests_suite

DataDog/apm-reliability/dd-trace-php | Ubuntu_22_arm64.SI94: [test-app-php] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab
View all 113 failed jobs.

⚠️ Warnings

❄️ 130 New flaky tests detected

tmp/build_extension/tests/ext/hook/line_hook_abstract_decl_line.phpt (A request landing on an abstract method declaration arms that method, whose synthetic r... from PHP.tmp.build_extension.tests.ext.hook
002&#43; 
003&#43; Fatal error: Uncaught Error: Call to undefined function DDTrace in tmp/build_extension/tests/ext/hook/line_hook_abstract_decl_line.php:18
004&#43; Stack trace:
001- bool(true)
002- from 4: (never fired)
003- from 5: 6
004- Done.
005&#43; #0 {main}
006&#43;   thrown in tmp/build_extension/tests/ext/hook/line_hook_abstract_decl_line.php on line 18
tmp/build_extension/tests/ext/hook/line_hook_anon_class.phpt (Line hooks resolve into anonymous class methods, including one sharing the declaring line) from PHP.tmp.build_extension.tests.ext.hook
002&#43; 
003&#43; Fatal error: Uncaught Error: Call to undefined function DDTrace in tmp/build_extension/tests/ext/hook/line_hook_anon_class.php:15
004&#43; Stack trace:
001- string(4) &#34;hi:a&#34;
002- string(4) &#34;hi:b&#34;
003- 9,9
004- Done.
005&#43; #0 {main}
006&#43;   thrown in tmp/build_extension/tests/ext/hook/line_hook_anon_class.php on line 15
View in Flaky Test Management

ℹ️ Info

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 68.38% (+0.00%)

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 46307f4 | Docs | View more details | Give us feedback!}

pr-commenter · 2026-10-05T23:14:41Z

Benchmarks [ tracer ]

Benchmark execution time: 2026-10-06 18:50:37

Comparing candidate commit 46307f4 in PR branch bob/line-hooks with baseline commit f9226fe in branch master.

📊 Benchmarking dashboard

Found 1 performance improvements and 24 performance regressions! Performance is the same for 167 metrics, 2 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

🟥 mem_peak [+56.992KB; +56.992KB] or [+2.559%; +2.559%]

scenario:EmptyFileBench/benchEmptyFileDdprof-opcache

🟥 mem_peak [+56.992KB; +56.992KB] or [+2.559%; +2.559%]

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

🟥 mem_peak [+56.992KB; +56.992KB] or [+2.559%; +2.559%]

scenario:HookBench/benchHookOverheadInstallHookOnFunction-opcache

🟥 execution_time [+26.700µs; +39.786µs] or [+5.630%; +8.389%]

scenario:HookBench/benchHookOverheadInstallHookOnMethod

🟥 execution_time [+25.977µs; +56.635µs] or [+5.013%; +10.930%]

scenario:HookBench/benchHookOverheadInstallHookOnMethod-opcache

🟥 execution_time [+30.252µs; +47.651µs] or [+6.331%; +9.973%]

scenario:HookBench/benchHookOverheadTraceFunction

🟥 execution_time [+5.411µs; +9.749µs] or [+2.673%; +4.815%]

scenario:HookBench/benchHookOverheadTraceMethod-opcache

🟥 mem_peak [+72.700KB; +72.704KB] or [+3.397%; +3.397%]

scenario:LaravelBench/benchLaravelBaseline

🟥 execution_time [+633.508µs; +787.272µs] or [+5.126%; +6.371%]

scenario:LaravelBench/benchLaravelBaseline-opcache

🟥 execution_time [+751.244µs; +911.016µs] or [+6.039%; +7.323%]
🟥 mem_peak [+57.024KB; +57.024KB] or [+2.561%; +2.561%]

scenario:LaravelBench/benchLaravelDdprof

🟥 execution_time [+795.384µs; +897.936µs] or [+5.619%; +6.344%]

scenario:LaravelBench/benchLaravelDdprof-opcache

🟥 mem_peak [+57.024KB; +57.024KB] or [+2.561%; +2.561%]

scenario:LaravelBench/benchLaravelOverhead

🟥 execution_time [+782.715µs; +1128.185µs] or [+6.000%; +8.649%]

scenario:LaravelBench/benchLaravelOverhead-opcache

🟥 execution_time [+322.178µs; +1312.982µs] or [+2.383%; +9.711%]
🟥 mem_peak [+57.024KB; +57.024KB] or [+2.561%; +2.561%]

scenario:LogsInjectionBench/benchLogsInfoInjection-opcache

🟩 execution_time [-1.359µs; -1.038µs] or [-12.232%; -9.338%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

🟥 execution_time [+4.071µs; +6.229µs] or [+3.605%; +5.516%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

🟥 execution_time [+3.340µs; +4.760µs] or [+2.941%; +4.192%]

scenario:PHPRedisBench/benchRedisOverhead-opcache

🟥 mem_peak [+72.728KB; +72.728KB] or [+2.929%; +2.929%]

scenario:SymfonyBench/benchSymfonyBaseline-opcache

🟥 mem_peak [+56.992KB; +56.992KB] or [+2.559%; +2.559%]

scenario:SymfonyBench/benchSymfonyDdprof-opcache

🟥 mem_peak [+56.992KB; +56.992KB] or [+2.559%; +2.559%]

scenario:SymfonyBench/benchSymfonyOverhead-opcache

🟥 mem_peak [+56.992KB; +56.992KB] or [+2.559%; +2.559%]

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead

🟥 execution_time [+6.521µs; +11.909µs] or [+3.046%; +5.563%]

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead-opcache

🟥 mem_peak [+72.704KB; +72.704KB] or [+3.374%; +3.374%]

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:ContextPropagationBench/benchInject64Bit

unstable execution_time [+16.713µs; +17.027µs] or [+719.168%; +732.639%]

scenario:LaravelBench/benchLaravelDdprof-opcache

unstable execution_time [-1096.770µs; +920.770µs] or [-7.258%; +6.093%]

And add some accidentally not staged tests. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

bwoebi requested review from a team as code owners October 5, 2026 19:53

bwoebi added 2 commits October 5, 2026 23:37

Fix line-hook portability and CI regressions

68424e3

Allow serial Valgrind suites more time

e28d438

bwoebi added 2 commits October 6, 2026 18:27

Simplify end line hooking, instead of forward matching.

48ab64d

And add some accidentally not staged tests. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

Add custom result exception cleanup on PHP 7.0/7.1

46307f4

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

Navigation Menu

Repository navigation

Add line-hooking - #4275

Add line-hooking#4275
bwoebi wants to merge 5 commits into
masterfrom
bob/line-hooks

bwoebi commented Oct 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

datadog-official Bot commented Oct 5, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

Uh oh!

pr-commenter Bot commented Oct 5, 2026 •

edited

Loading

Explanation

More details about the CI and significant changes

Unstable benchmarks

scenario:ContextPropagationBench/benchInject64Bit

scenario:LaravelBench/benchLaravelDdprof-opcache

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bwoebi commented Oct 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

datadog-official Bot commented Oct 5, 2026 • edited by datadog-datadog-prod-us1 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Errors

⚠️ Warnings

ℹ️ Info

Uh oh!

pr-commenter Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ tracer ]

Explanation

More details about the CI and significant changes

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

scenario:EmptyFileBench/benchEmptyFileDdprof-opcache

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

scenario:HookBench/benchHookOverheadInstallHookOnFunction-opcache

scenario:HookBench/benchHookOverheadInstallHookOnMethod

scenario:HookBench/benchHookOverheadInstallHookOnMethod-opcache

scenario:HookBench/benchHookOverheadTraceFunction

scenario:HookBench/benchHookOverheadTraceMethod-opcache

scenario:LaravelBench/benchLaravelBaseline

scenario:LaravelBench/benchLaravelBaseline-opcache

scenario:LaravelBench/benchLaravelDdprof

scenario:LaravelBench/benchLaravelDdprof-opcache

scenario:LaravelBench/benchLaravelOverhead

scenario:LaravelBench/benchLaravelOverhead-opcache

scenario:LogsInjectionBench/benchLogsInfoInjection-opcache

scenario:MessagePackSerializationBench/benchMessagePackSerialization

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

scenario:PHPRedisBench/benchRedisOverhead-opcache

scenario:SymfonyBench/benchSymfonyBaseline-opcache

scenario:SymfonyBench/benchSymfonyDdprof-opcache

scenario:SymfonyBench/benchSymfonyOverhead-opcache

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead

scenario:TraceAnnotationsBench/benchTraceAnnotationOverhead-opcache

Unstable benchmarks

scenario:ContextPropagationBench/benchInject64Bit

scenario:LaravelBench/benchLaravelDdprof-opcache

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chatgpt-codex-connector Bot commented Oct 5, 2026 •

edited

Loading

datadog-official Bot commented Oct 5, 2026 •

edited by datadog-datadog-prod-us1 Bot

Loading

pr-commenter Bot commented Oct 5, 2026 •

edited

Loading