Improved logging, richer stack traces

[pgorod]

Description, Motivation and Context

Adds some config_override.php options to get more information in the logs, specifically:

• ability to ask for richer information regarding a single error message, making it easier to focus on a specific problem
• ability to get more highly technical information without being a technical user (useful for Forums troubleshooting and gathering info from clients)
• get IDE-level stack traces, with both argument names and values

Documentation preview

https://pedro--suitedocs.netlify.app/developer/logging/#_advanced_logging_configuration

How To Test This

Please read the documentation linked above, there are many options.

An important part of the testing is confirming that nothing changes until we start putting options in config_override.php. This will make me more confident about putting the code out there :-)

For a simple run, you can test this by adding entries in your config_override.php such as these (all explained in the Docs page):

$sugar_config['show_log_trace'] = 'One2MBeanRelationship'; // use a boolean true for all; or a string as filter
$sugar_config['show_log_trace_with_eol'] = true;
$sugar_config['show_log_trace_overview'] = true;
$sugar_config['show_log_trace_trim'] = 500;
$sugar_config['show_log_trace_source'] = 1;

Answers to some valid concerns

1. This is backwards compatible because it does not assume any config values to be there, always uses sensible defaults, and respects the previous meaning of $sugar_config['show_log_trace'].

2. This code is complex, and some bug can have slipped by, but it only runs in the cases where you select log traces, so it's easy to turn off, and shouldn't affect production systems.

3. Some of this code impacts performance (lots of string operations, more file activity) but it's always opt-in: the impact only occurs when you configure it to happen.

4. This is accompanied with a warning in Documentation: using log traces with argument values might leave sensitive information in the system logs. Use it only temporarily for troubleshooting purposes; obfuscate the information before posting it online; double-check that your logs are only accessible to admins; delete the logs when finished troubleshooting.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Final checklist

• My code follows the code style of this project found here.
• My change requires a change to the documentation.
• I have read the How to Contribute guidelines.

[codecov-io]

Codecov Report

Merging #8640 into hotfix-7.10.x will decrease coverage by 0.00%.
The diff coverage is 9.21%.

@@                Coverage Diff                @@
##           hotfix-7.10.x    #8640      +/-   ##
=================================================
- Coverage          10.70%   10.70%   -0.01%     
=================================================
  Files               3229     3229              
  Lines             240915   240977      +62     
=================================================
+ Hits               25795    25800       +5     
- Misses            215120   215177      +57     

[pgorod]

@Dillon-Brown the Travis issue with serialize and unserialize is not really relevant. In my opinion the tests should not require those methods, it might seem obvious to require them for an object that implements Serializable, but this is just a case where that is not needed. Anyway, fixing it is also harmless, just have the methods there, though nobody will ever call them.

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/why-suitecrm-fetch-all-user-data-from-database-because-its-took-long-time-for-fetching/84549/2

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/bulk-select-delete-500-error/84634/9

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/mysql-error-1054-unknown-column-config-deleted-in-where-clause/86371/12

[chris001]

Nice improvement @pgorod ! Would be great to supplement this feature with whoops (github) and OpenTelemetry.

[pgorod]

Thanks. There are definitely very interesting packages out there!
This one I made, while it's a simpler thing, makes good use of the knowledge of SuiteCRM specificities. The fact that SuiteCRM mostly calls a logging function, instead of throwing exceptions, may not be the coolest thing, but has advantages for retaining more stack information.

My package is also done with our community Forums in mind. I wanted a simple way of putting together suitecrm.log and php_errors.log information; and of telling SuiteCRM to emit a stack trace of a specific error; and to pull in a lot of debugging information, including function argument values and argument names.

This final part, BTW, I don't know of any other PHP logging system out there that is able to do it. It probably exists, because it's possible, so others surely must have done it, but I haven't found one yet. I guess people typically focus either on Reflection or on the Exception object, but never bother to combine both like I did.

I guess I can say this is a cool PR, and, I think it's really uncool that to this day I still unnecessarily lose time on the Forums doing things the hard way because the PR sits here un-merged for 2.5 years...

[chris001]

I'd fix the Codacy coding standard issue, then comment with an "at" to 2 code reviewers who are committers.

[pgorod]

I don't understand what the Codacy issue is, exactly...

[chris001]

Codacy error might be this:
"It seems like the branch hotfix-7.10.x is not being analyzed. To review this pull request enable the branch in the repository settings."
Probably have to ask an admin to enable checking that branch hotfix-7.10.x in the Codacy settings.

[pgorod]

Ah, ok. I guess since the hotfix-7.10.x branch is no longer in use, what I really need to do is to update the PR to target the hotfix branch.

I will do this when someone with commit rights signals that they are ready to work on merging the PR. Thanks.

[pgorod]

I just added a couple of very minor tweaks.

And I'd need to add this to the documentation also:

since PHP 7.4, args will only be available if php.ini includes: zend.exception_ignore_args = Off

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/var-dump-in-suitecrm-7/88650/4

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/enabling-stack-trace-errors-on-v8-3-0-and-8-2-4-crashes-server/89093/11

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/suitecrm-8-4-beta-release/89872/4

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/actually-using-v8-4-now/90524/9

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/unable-to-login-after-8-4-2-upgrade/90970/14

[SuiteBot]

This pull request has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/suitecrm-8-5-bulk-action-export-selecting-all-records-unexpected-error-when-calling-action-error/91646/2

[chris001]

@pgorod With VS Code and the GitLens extension, it's a few clicks to change this PR's target branch to hotfix, and then, rebase your 2 commits in this PR, to the head of hotfix. Demonstration here:
https://www.youtube.com/watch?v=3o_01F04bZ4

[pgorod]

I think I can handle the rebase, I use PhpStorm which has very nice interactive git tools. The only problem is that my v7 local git repo is a bit messed up and tangled right now, and I don't have much time to go fix it.

Any way, I think this is still my best strategy:

I will do this when someone with commit rights signals that they are ready to work on merging the PR. Thanks.

Because if I fix it now, and then years pass, I will have to fix it again, and that is just pointless work if there is no interest in merging.

[chris001]

There's about 20 commits made by others, showing up in this PR now!
I'd take a backup of your 2 commits, then, I think you click on "Sync Fork", and that should quickly make those 20 commits disappear, and leave only your 2 commits.
Then, users would be free to run a couple of commands, to download and apply your PR to their local test servers, test, and give valuable feedback.

[pgorod]

Yesterday I edited the PR and changed the destination branch to be salesagility/hotfix instead of salesagility/hotfix-7.10 and that caused the other commits to get included. I also explored the "Sync fork" options from my repo, but they weren't helpful.

What I really would need to is to start my branch from my synced hotfix instead of from my hotfix-7.10. In other words, I would need to rebase my two commits on top of hotfix. But I can't find a way to do that from the Github website, I would need to do it from command-line git... which is where my messed up repo comes in. :-)

This would also imply that I re-do the PR, and we would lose these comments and discussion here, which is a pity.

It's not too much work, it's about 30m of untangling git, I do it all the time. It's just that it's not a good time for me to invest that effort. I promise to do it as soon as someone from SalesAgility gives me the sign that there is interest in merging, though.

[chris001]

If you'd rather not untangle your local messed up fork and hotfix branch, then there is this alternative. Download a free IDE such as PHPstorm EAP or VScode, point it to a different local directory than your local fork. Clone a new local fork of the SA v7 repo, rebase your 2 commits on top of that local fork's hotfix branch, then from the UI do a Commit and Push, and if necessary a Force Push, to get the two commits up here, they should arrive here without the other 20 irrelevant commits. Worth a try, up to you.

[pgorod]

@chris001 your comment reminded me I had done this once:
https://pgorod.github.io/Git-rebase-atomic/

I am closing this PR in favor of the updated version: #10342

@johnM2401 or @serhiisamko091184 maybe you want to do some house keeping and move these labels over to the new one.