Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sending arguments with stacktraces #632

Closed
wants to merge 1 commit into from
Closed

Support sending arguments with stacktraces #632

wants to merge 1 commit into from

Conversation

GrahamCampbell
Copy link
Contributor

@GrahamCampbell GrahamCampbell commented Oct 19, 2021

Goal

As a developer, I'd like to see the arguments passed to functions in stack trace frames to enable me to better understand what happened and how to reproduce the issue. Closes #519.

Design

The Configuration class has a mechanism for toggling if code should be sent. We can copy this design to add support for toggling if argument values should be included. Objects, arrays and resources are represented by noting their type but not their value. Quite a rabbit hole otherwise...

Changeset

New setSendArguments and shouldSendArguments functions. Disabled by default, so no change in functionality for existing users.

Testing

Unit tests.

@GrahamCampbell GrahamCampbell marked this pull request as draft October 19, 2021 23:30
@GrahamCampbell GrahamCampbell changed the title Support sending arguments with stack traces Support sending arguments with stacktraces Oct 20, 2021
@GrahamCampbell GrahamCampbell marked this pull request as ready for review October 20, 2021 00:32
$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS & ~DEBUG_BACKTRACE_PROVIDE_OBJECT);
// Reduce memory usage by omitting args and objects from backtrace by default
$backtrace = debug_backtrace(
$config->shouldSendArguments() ? 0 : DEBUG_BACKTRACE_IGNORE_ARGS & ~DEBUG_BACKTRACE_PROVIDE_OBJECT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Omitting objects seems to not show any args at all in my testing, so I left it out.

@@ -89,7 +91,8 @@ public static function fromBacktrace(Configuration $config, array $backtrace, $t
$topFile,
$topLine,
isset($frame['function']) ? $frame['function'] : null,
isset($frame['class']) ? $frame['class'] : null
isset($frame['class']) ? $frame['class'] : null,
$config->shouldSendArguments() && isset($frame['args']) ? $frame['args'] : null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

required to check shouldSendArguments again for xdebug compatibility

@luke-belton
Copy link
Member

hey @GrahamCampbell thanks for submitting this! We're going to take a look and review as soon as priorities allow it 👍

@luke-belton luke-belton added the backlog We hope to fix this feature/bug in the future label Oct 27, 2021
@mattdyoung
Copy link

mattdyoung commented Nov 16, 2021

Hi @GrahamCampbell

Thanks for the PR. Unfortunately we've decided that we're not going to move forward with this feature for the time being.

There are a few wider issues that would need to be addressed such as dashboard updates to ensure long values wrap sensibly, and providing a new API with finer control over redacting data as the arguments are highly likely to contain sensitive data in some applications. We'd need to invest some effort in designing and implementing these changes to fully support this in an acceptable way.

We're continuing to monitor interest on the original github issue and may pick this up again in the future when priorities allow.

@GrahamCampbell GrahamCampbell closed this by deleting the head repository Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Argument in stacktrace
3 participants