Skip to content

Add "recurse" => true option to hook/trace config array#1677

Merged
bwoebi merged 1 commit into
masterfrom
bob/hook-recursion
Jul 29, 2022
Merged

Add "recurse" => true option to hook/trace config array#1677
bwoebi merged 1 commit into
masterfrom
bob/hook-recursion

Conversation

@bwoebi

@bwoebi bwoebi commented Jul 29, 2022

Copy link
Copy Markdown
Collaborator

Description

Thus, it is now possible to write code like

DDTrace\hook_function('greet', [
    'prehook' => function ($args) {
        echo "greet prehook: {$args[0]}\n";
    },
    'posthook' => function ($args) {
        echo "greet posthook: {$args[0]}\n";
    },
    'recurse' => true
]);

to have traces also trigger upon recursion

Fixes #1105.

Readiness checklist

  • (only for Members) Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the release document. For community contributors the reviewer is in charge of this task.

Thus, it is now possible to write code like

DDTrace\hook_function('greet', [
    'prehook' => function ($args) {
        echo "greet prehook: {$args[0]}\n";
    },
    'posthook' => function ($args) {
        echo "greet posthook: {$args[0]}\n";
    },
    'recurse' => true
]);

to have traces also trigger upon recursion

Signed-off-by: Bob Weinand <[email protected]>
@bwoebi bwoebi added this to the 0.77.0 milestone Jul 29, 2022
@bwoebi
bwoebi requested a review from morrisonlevi July 29, 2022 13:35
@bwoebi
bwoebi requested a review from a team as a code owner July 29, 2022 13:35

@morrisonlevi morrisonlevi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What you have looks fine, but from experience we should also add a test for something like some class extending Pdo::exec that calls parent::exec as part of its exec.

Also, we should update the Symfony integration to allow recursion for the event handler hooks if possible. This part doesn't have to be in this PR, though.

@morrisonlevi morrisonlevi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Chatted with Bob; the test I wanted is irrelevant now due to some other changes I missed. Looks good.

@bwoebi
bwoebi merged commit 70321e3 into master Jul 29, 2022
@bwoebi
bwoebi deleted the bob/hook-recursion branch July 29, 2022 16:42
@morrisonlevi morrisonlevi added the 🏆 enhancement A new feature or improvement label Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏆 enhancement A new feature or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] \DDTrace\trace_method not tracing nested/recursive method invocations

2 participants