Skip to content

Conversation

@innocenzi
Copy link
Member

@innocenzi innocenzi commented Oct 2, 2025

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:

  • Update documentation

@innocenzi innocenzi changed the title refactor(logging)!: dissociate tempest/debug from tempest/log refactor(logging): dissociate tempest/debug from tempest/log Oct 2, 2025
@innocenzi innocenzi force-pushed the feat/logging-improvements branch from 9a1bb51 to 0757d78 Compare October 2, 2025 19:52
@innocenzi innocenzi changed the title refactor(logging): dissociate tempest/debug from tempest/log refactor(logging)!: dissociate tempest/debug from tempest/log Oct 2, 2025
@innocenzi innocenzi force-pushed the feat/logging-improvements branch from 67d17c2 to 55e95fd Compare October 2, 2025 22:53
@innocenzi innocenzi marked this pull request as ready for review October 3, 2025 12:17
@innocenzi innocenzi requested a review from brendt October 3, 2025 12:17
@brendt
Copy link
Member

brendt commented Oct 3, 2025

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

@innocenzi
Copy link
Member Author

innocenzi commented Oct 3, 2025

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

@brendt
Copy link
Member

brendt commented Oct 4, 2025

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

@innocenzi innocenzi changed the title refactor(logging)!: dissociate tempest/debug from tempest/log feat(log)!: support multiple loggers Oct 4, 2025
@innocenzi
Copy link
Member Author

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 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants