-
-
Notifications
You must be signed in to change notification settings - Fork 485
[4.x] Add LogTenancyBootstrapper #1381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
lukinovec
wants to merge
26
commits into
master
Choose a base branch
from
add-log-bootstrapper
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
01a06c9
Add LogTenancyBootstrapper
lukinovec 96a05cd
Fix code style (php-cs-fixer)
github-actions[bot] 43cf6d2
Merge branch 'master' into add-log-bootstrapper
lukinovec 50853a3
Test LogTenancyBootstrapper logic (low-level tests)
lukinovec b80d7b3
Test real usage with storage path-based channels
lukinovec a13110c
Test real usage with slack channel (the bootstrapper updates the webh…
lukinovec 718afd3
Simplify the slack channel usage test
lukinovec a806df0
Stop using real domains in the tests
lukinovec ec47528
Refactor bootstrapper, make comments more concise
lukinovec 8cd35d3
Add @see to bootstrapper docblock
lukinovec 62a0e39
Delete redundant test, test the same logic in the one larger test
lukinovec 582243c
Clarify test name
lukinovec bd44036
By default, only override the config if the override tenant property …
lukinovec 63bf4bf
Clarify bootstrapper comments
lukinovec 81daa9d
Simplify test
lukinovec 42c837d
Refactor bootstrapper, provide more info in comments
lukinovec 7bdbe9d
Improve checking if tenant attribute is set
lukinovec c180c2c
Use more accurate terminology
lukinovec 412c1d0
Merge branch 'master' into add-log-bootstrapper
stancl 0b3f698
Merge branch 'master' into add-log-bootstrapper
lukinovec f878aaf
Improve closure overrides
lukinovec b36f3ce
Fix typo
lukinovec 108e0d1
Swap closure param order, add/update comments
lukinovec e133c87
Make test priovide sufficient context for understanding the default b…
lukinovec 58a2447
Use more direct assertions in the tests that assert the actual behavi…
lukinovec ae39e4d
Clarify behavior in log bootstrapper comments
lukinovec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Stancl\Tenancy\Bootstrappers; | ||
|
|
||
| use Closure; | ||
| use Illuminate\Contracts\Config\Repository as Config; | ||
| use Illuminate\Log\LogManager; | ||
| use Stancl\Tenancy\Contracts\TenancyBootstrapper; | ||
| use Stancl\Tenancy\Contracts\Tenant; | ||
|
|
||
| /** | ||
| * This bootstrapper makes it possible to configure tenant-specific logging. | ||
| * | ||
| * By default, the storage path channels ('single' and 'daily' by default, | ||
| * but feel free to customize that using the $storagePathChannels property) | ||
| * are configured to use tenant storage directories. | ||
| * For this to work correctly, this bootstrapper must run *after* FilesystemTenancyBootstrapper. | ||
| * FilesystemTenancyBootstrapper alters how storage_path() works in the tenant context. | ||
| * | ||
| * The bootstrapper also supports custom channel overrides via the $channelOverrides property (see the property's docblock). | ||
| * | ||
| * @see Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper | ||
| */ | ||
| class LogTenancyBootstrapper implements TenancyBootstrapper | ||
| { | ||
| protected array $defaultConfig = []; | ||
|
|
||
| /** | ||
| * Log channels that use the storage_path() helper for storing the logs. Requires FilesystemTenancyBootstrapper to run before this bootstrapper. | ||
| * Or you can bypass this default behavior by using overrides, since they take precedence over the default behavior. | ||
| */ | ||
| public static array $storagePathChannels = ['single', 'daily']; | ||
|
|
||
| /** | ||
| * Custom channel configuration overrides. | ||
| * | ||
| * Examples: | ||
| * - Array mapping (the default approach): ['slack' => ['url' => 'webhookUrl']] maps $tenant->webhookUrl to slack.url (if $tenant->webhookUrl is not null, otherwise, the override is ignored) | ||
| * - Closure: ['slack' => fn (Tenant $tenant, array $channel) => array_merge($channel, ['url' => $tenant->slackUrl])] | ||
| */ | ||
| public static array $channelOverrides = []; | ||
|
|
||
| public function __construct( | ||
| protected Config $config, | ||
| protected LogManager $logManager, | ||
| ) {} | ||
|
|
||
| public function bootstrap(Tenant $tenant): void | ||
| { | ||
| $this->defaultConfig = $this->config->get('logging.channels'); | ||
| $channels = $this->getChannels(); | ||
|
|
||
| $this->configureChannels($channels, $tenant); | ||
| $this->forgetChannels($channels); | ||
| } | ||
|
|
||
| public function revert(): void | ||
| { | ||
| $this->config->set('logging.channels', $this->defaultConfig); | ||
|
|
||
| $this->forgetChannels($this->getChannels()); | ||
| } | ||
|
|
||
| /** | ||
| * Channels to configure and re-resolve afterwards (including the channels in the log stack). | ||
| */ | ||
| protected function getChannels(): array | ||
| { | ||
| // Get the currently used (default) logging channel | ||
| $defaultChannel = $this->config->get('logging.default'); | ||
| $channelIsStack = $this->config->get("logging.channels.{$defaultChannel}.driver") === 'stack'; | ||
|
|
||
| // If the default channel is stack, also get all the channels it contains. | ||
| // The stack channel also has to be included in the list of channels | ||
| // since the channel will be resolved and saved in the log manager, | ||
| // and its config could accidentally be used instead of the underlying channels. | ||
| // | ||
| // For example, when you use 'stack' with the 'slack' channel and you want to configure the webhook URL, | ||
| // both the 'stack' and the 'slack' must be re-resolved after updating the config for the channels to use the correct webhook URLs. | ||
| // If only one of the mentioned channels would be re-resolved, the other's webhook URL would be used for logging. | ||
| $channels = $channelIsStack | ||
| ? [$defaultChannel, ...$this->config->get("logging.channels.{$defaultChannel}.channels")] | ||
| : [$defaultChannel]; | ||
|
|
||
| return $channels; | ||
| } | ||
|
|
||
| /** | ||
| * Configure channels for the tenant context. | ||
| * | ||
| * Only the channels that are in the $storagePathChannels array | ||
| * or have custom overrides in the $channelOverrides property | ||
| * will be configured. | ||
| */ | ||
| protected function configureChannels(array $channels, Tenant $tenant): void | ||
| { | ||
| foreach ($channels as $channel) { | ||
| if (isset(static::$channelOverrides[$channel])) { | ||
| $this->overrideChannelConfig($channel, static::$channelOverrides[$channel], $tenant); | ||
| } elseif (in_array($channel, static::$storagePathChannels)) { | ||
| // Set storage path channels to use tenant-specific directory (default behavior) | ||
| // The tenant log will be located at e.g. "storage/tenant{$tenantKey}/logs/laravel.log" (assuming FilesystemTenancyBootstrapper is used before this bootstrapper) | ||
| $this->config->set("logging.channels.{$channel}.path", storage_path('logs/laravel.log')); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| protected function overrideChannelConfig(string $channel, array|Closure $override, Tenant $tenant): void | ||
| { | ||
| if (is_array($override)) { | ||
| // Map tenant attributes to channel config keys. | ||
| // If the tenant attribute is null, | ||
| // the override is ignored and the channel config key's value remains unchanged. | ||
| foreach ($override as $configKey => $tenantAttributeName) { | ||
| $tenantAttribute = $tenant->getAttribute($tenantAttributeName); | ||
|
|
||
| if ($tenantAttribute !== null) { | ||
| $this->config->set("logging.channels.{$channel}.{$configKey}", $tenantAttribute); | ||
| } | ||
| } | ||
| } elseif ($override instanceof Closure) { | ||
| $channelConfigKey = "logging.channels.{$channel}"; | ||
|
|
||
| $this->config->set($channelConfigKey, $override($tenant, $this->config->get($channelConfigKey))); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Forget all passed channels so they can be re-resolved | ||
| * with updated config on the next logging attempt. | ||
| */ | ||
| protected function forgetChannels(array $channels): void | ||
| { | ||
| foreach ($channels as $channel) { | ||
| $this->logManager->forgetChannel($channel); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded filename 'laravel.log' should be made configurable or use the original filename from the channel configuration. This prevents customization of log filenames and could overwrite existing configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, this is fine for the default behavior, it can still be customized. Resolving this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of the box, if no customization is used,
$storagePathChannelsincludesdailywhich does not uselaravel.lognames, but day-specific names. Seems like something that should be checked and tested.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is checked and tested already, I'd just add a short comment with an explanation of how
dailyworks.dailydriver uses RotatingFileHandler that parses the file name. The current code (=storage_path('logs/laravel.log')) corresponds to thedailylog channel config. It is correct, so I'd just clarify this since this can indeed be quite confusing