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

Couple changes to aid debugging #13269

Merged
merged 9 commits into from Sep 11, 2018
Merged

Couple changes to aid debugging #13269

merged 9 commits into from Sep 11, 2018

Conversation

diosmosis
Copy link
Member

Changes:

  • Add fatal error "breadcrumb" to ArchiveProcessor:: aggregateDataTableRecord() which is a memory heavy method (so could cause out of memory error which would be good to track).
  • Add total blob length to analyze archive tables output.

@diosmosis diosmosis added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Aug 10, 2018
@diosmosis diosmosis added this to the 3.7.0 milestone Aug 10, 2018
@diosmosis diosmosis added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Aug 11, 2018
@mattab mattab modified the milestones: 3.7.0, 3.6.1 Sep 1, 2018
{
$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, $limit = 2);
$backtrace[1]['class'] = $className; // knowing the derived class name is far more useful
$backtrace[1]['args'] = empty($importantArgs) ? [] : array_map('strval', $importantArgs);
Copy link
Member

Choose a reason for hiding this comment

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

would you need to json_encode the args or so in case it is an array of an array or so?

} else {
$args .= ', ';
}
$args .= $name . '=' . $value;
Copy link
Member

Choose a reason for hiding this comment

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

just fyi: Was wondering if we need to escape something here but by the looks not needed as it is printed with {{ lastError.message|e('url') }}

@tsteur
Copy link
Member

tsteur commented Sep 8, 2018

Left a few comments but otherwise looks 👍

@diosmosis diosmosis merged commit b8e4d27 into 3.x-dev Sep 11, 2018
@diosmosis diosmosis deleted the debugging-improvements branch September 11, 2018 01:36
diosmosis added a commit that referenced this pull request Sep 20, 2018
* Add ability to print arguments in fatal error trace.

* Add sum of blob content to analyze archive tables command.

* Fixing tests.

* Fixing tests.

* fixing test

* json_encode instead of strval

* fix test
InfinityVoid pushed a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018
* Add ability to print arguments in fatal error trace.

* Add sum of blob content to analyze archive tables command.

* Fixing tests.

* Fixing tests.

* fixing test

* json_encode instead of strval

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants