@samjf opened this Pull Request on March 24th 2022 Contributor

Ref: DEV-2584

Description:

When viewing the logs of a multi-tenant install it is difficult to determine which Matomo instance was requested when the error (log entry) occurs.

A simple solution is to print the host from the incoming headers. Whilst this is not safe against tampering, it is only printed in logs and will be effective for normal traffic.

Notes

I preferred injecting the error message prefix when providing the prefix in core/testMinimumPhpVersion.php due to the fact that the code seems to be early in the bootstrap and must be compatible with early versions of PHP. The filter_var validation function of the hostname/domain has a minimum php requirement of 5.2.

Review

@github-actions[bot] commented on April 2nd 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@sgiehl commented on April 7th 2022 Member

@samjf This looks good so far. Is it on purpose that only exceptions / errors that might get logged outside of matomo's logging will be prefixed with the host? Messages logged to the matomo log files won't include the hostname. For that guess it would need to be added somewhere in https://github.com/matomo-org/matomo/blob/115527353a9e75e01aa4d263408956ae45403bea/plugins/Monolog/Formatter/LineMessageFormatter.php

@samjf commented on April 10th 2022 Contributor

@sgiehl I don't think I've really had a problem with the matomo file log. Correct me if i'm wrong but It seems that the hostnames are being logged for me (checking locally). Perhaps archiving/commands already adds them on a multitenant?
image

The main problem was trying to distinguish between a large amount of errors on the PHP/FPM error/exception logging.

@sgiehl commented on April 11th 2022 Member

@samjf Guess our cloud plugin has a special log processor, that adds the instanceid. We could consider having that in core and only log it for multitenant maybe.
But as that won't affect many people at all, guess we can save the effort for now.

@samjf commented on April 11th 2022 Contributor

Many thanks @sgiehl
That might be a good contribution from cloud in the future. Hard tracking down exactly where it was added haha.

This Pull Request was closed on April 11th 2022
Powered by GitHub Issue Mirror