tempest-framework icon indicating copy to clipboard operation
tempest-framework copied to clipboard

feat(log)!: support multiple loggers

Open innocenzi opened this issue 3 months ago • 13 comments

This pull request dissociates tempest/debug from tempest/log, among other improvements.

  • Logger now support tagged configurations
  • All log files default to being in .tempest/logs instead of ./log
  • The debug package has its own config for defining the debug log file
  • Debug tailing is now more readable
  • Channel properties are now properly commented
  • Channels support minimum log levels
  • The single tail command and the tail:project one have been removed, there's just tail:debug and tail:logs now

~~This is still a draft because I feel like there could be better APIs for the config, but not sure what exactly. The issue right now is that if you don't know about the existing channel classes, you have to look up the docs, because the channels property is an array.~~

~~Maybe I'm overthinking and it's fine as-is? Currently, it looks like that:~~

return new LogConfig(
    channels: [
        new DailyLogChannel(
            path: internal_storage_path('logs', 'tempest.log'),
            maxFiles: env('LOG_MAX_FILES', default: 31),
            prefix:env('APPLICATION_NAME', default: 'tempest'),
        ),
    ],
);

EDIT: ended up using the same pattern as everywhere in Tempest—dedicated config files, with one config allowing for multiple channels.

return new DailyLogConfig(
    path: internal_storage_path('logs', 'tempest.log'),
    prefix: env('APPLICATION_NAME', default: 'tempest'),
    maxFiles: env('LOG_MAX_FILES', default: 31),
);

Due to this change, this PR unfortunately becomes breaking.

As a reminder, to log differently in different environment, we can have logging.<env>.config.php files.

TODO:

  • [x] Update documentation

innocenzi avatar Oct 02 '25 19:10 innocenzi

Looks clean!

How will we handle the breaking change? Is there a way we can keep the old way for now but deprecate it? Just so that we can have this in 2.x already and don't have to wait until 3.x

brendt avatar Oct 03 '25 12:10 brendt

There would be naming conflicts with the new LogConfig interface unfortunately. That would be a log of glue code too...

To be honest, the breaking change is minor: if users have a custom logging config, it will be ignored in favor of the default one

EDIT: actually, it won't be ignored, it will crash because they'd be instantiating an interface

innocenzi avatar Oct 03 '25 12:10 innocenzi

I'm fine merging it in; but we should provide a rector for it then. Are you up for that? Otherwise I'll take care of it

brendt avatar Oct 04 '25 16:10 brendt

I updated the PR to bring some other improvements, notably tagged configurations, in order to have the ability to have multiple loggers for different purposes, which is actually quite important!

I also added docs to the PR, so it's ready to be merged.

@brendt, I tried making a Rector rule for the breaking change, but couldn't manage, so if you still want to, you can do it 😓

innocenzi avatar Oct 04 '25 23:10 innocenzi

This pull request is stale because it has been open for 30 days with no activity.

github-actions[bot] avatar Nov 04 '25 02:11 github-actions[bot]

@brendt can we merge?

innocenzi avatar Nov 10 '25 20:11 innocenzi

I'm on the fence about it if we don't have a rector for it. Gonna ask around on Discord for some real life examples.

brendt avatar Nov 14 '25 07:11 brendt

./tempest tail:debug causes my CPU temperature to rise by 15 degrees :smile: Maybe it's busy waiting? I'm using linux

btw. related XKCD https://xkcd.com/1172/

aazsamir avatar Nov 14 '25 08:11 aazsamir

causes my CPU temperature to rise by 15 degrees

Do you mean with this branch?

brendt avatar Nov 14 '25 10:11 brendt

causes my CPU temperature to rise by 15 degrees

Do you mean with this branch?

Yes, but now I checked and it's the same on main, so not a regression, sorry^^

aazsamir avatar Nov 14 '25 10:11 aazsamir

Gonna ask around on Discord for some real life examples

I believe many people indeed are using custom logging config. I would only be comfortable merging this in 2.x if we have an automated upgrade for it. I'm not sure that'll be trivial to do though 😬

I briefly looked into it, and I'm not sure we can do what we need to do with Rector to automate this upgrade. That being said, maybe my Rector knowledge is lacking.

Maybe we should simply start preparing for 3.0 and include this one in there?

brendt avatar Nov 22 '25 05:11 brendt

Personally I would say the Tempest community is small enough for us to be able to make that breaking change on a minor version 😬

That being said given your question on the Discord server I would say that few are actually customizing it

If you really don't want the breaking change though, we can create a 3.x and merge this

innocenzi avatar Nov 23 '25 23:11 innocenzi

I've given it some more thought and I think I'd like it to be targeted toward 3.x.

I'm ok tagging 3.x soon though. Let's aim for the beginning of January? That way I can finish my FrankenPHP PR, and then we'll merge 8.5 support in 3.x as well

brendt avatar Nov 28 '25 07:11 brendt

@innocenzi feel free to merge in 3.x when you're ready

brendt avatar Dec 03 '25 12:12 brendt