Bug: Logger Path Duplicated

[entrinix]

Direction
Logger config seems duplicated.

Describe the bug
Setting line 45 in CodeIgniter4/app/Config/Logger.php

public $path = '';

seems to have no effect. Double checked with file vendor/codeigniter4/framework/system/Log/Handlers/FileHander line 75 by checking what $config comes in as. This also uses the value from line 108 and not from 45.

Changing line 108

'path' => WRITEPATH . 'logs/',

does. It would seem that line 45 is a holdover from CI3 and should either be removed or do something.

Furthermore, line 118 defaults to php, but in CI4, this folder is no longer stored in a publicly accessible folder, and should default to log, again, seems like a holdover from CI3.

CodeIgniter 4 version
RC2

Affected module(s)
Logger

[dafriend]

@jim-parry I have been spending a lot of time on a rework of the logger that I believe has merit. It's not quite done but enough is there for you to get the gist. I can push the branch I've been working on (without a PR) if you're interested in seeing it. If not, please let me know so I can put my efforts into something more useful.

I see you have recently merged several commits re: the logger and wonder if I'm wasting my time.

In a nutshell, my approach is to refactor the setup such that Config\Logger\Logger is concerned only with serving up handlers. Each CodeIgniter\Log\Handlers deals with its specific technology - FileHandler with server files, ChromeLoggerHandler with the Chrome Logger, and an imagined EmailHandler with email. I can see, eventually, a TextMessageHandler as a possibility.

Each handler relies on its own config. Having different handlers work with a different set of reporting levels is a trivial config setting. More or less equivalent to setting the threshold level in the current v4 approach on a handler by handler basis.

What I have been doing is nearing completion. Unit tests for the FileHandler and Logger are revised and all passing. I'm trying to sus out the Chrome logger right now. The documentation has not yet been touched. I'd certainly do that before making a PR.

So, what do say? Want to see it or should I move on?

[jim-parry]

I was just trying to fix the bug. I don't know if there are broader concerns about the Logger, and will check with the rest of the team.

[MGatner]

@dafriend I’d be interested in seeing what you have. No need for a PR, you could push it to your fork and link it here.

Your mention of text messages makes me wonder if there isn’t some overlap with Queues. I know we still want to implement Queues (probably 4.1) but you could check out https://github.com/codeigniter4/CodeIgniter4/tree/feature/queue for comparison. I’d want to make sure the two filled their own separate roles.

[lonnieezell]

Would definitely be interested in seeing it as it sounds like it functions much like other handler-based elements in the framework (sessions, cache). As long as it is consistent with how those handle things, and doesn't break psr-3 compatability I would be open to considering it.

[dafriend]

I find the branch but cannot find any code related to message queues. It's possible that a TextMessageHandler might utilize a queue but I don't see any overlap.

[dafriend]

I've pushed my working branch (update_logger) to my fork.