-
-
Notifications
You must be signed in to change notification settings - Fork 486
[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
base: master
Are you sure you want to change the base?
Changes from all commits
01a06c9
96a05cd
43cf6d2
50853a3
b80d7b3
a13110c
718afd3
a806df0
ec47528
8cd35d3
62a0e39
582243c
bd44036
63bf4bf
81daa9d
42c837d
7bdbe9d
c180c2c
412c1d0
0b3f698
f878aaf
b36f3ce
108e0d1
e133c87
58a2447
ae39e4d
39fc72b
aedb33b
c68b91c
89b0d1c
9660faf
221a995
f705f58
697ba65
b744167
cdea112
8fda84f
34115e8
1ae418c
95fd046
06472d5
9ea3813
23ae15a
b234308
42d60e9
c2a80c2
2f60e76
fa075ef
c5683d8
8276f3b
6e474ac
55ee7d8
b51d5ca
8107ed4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,204 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Stancl\Tenancy\Bootstrappers; | ||
|
|
||
| use Closure; | ||
| use Illuminate\Contracts\Config\Repository; | ||
| use Illuminate\Database\Eloquent\Model; | ||
| use Illuminate\Log\LogManager; | ||
| use Illuminate\Support\Str; | ||
| use InvalidArgumentException; | ||
| use Stancl\Tenancy\Contracts\TenancyBootstrapper; | ||
| use Stancl\Tenancy\Contracts\Tenant; | ||
|
|
||
| /** | ||
| * Enable tenant-specific logging. | ||
| * | ||
| * All the storage path channels are configured to use tenant | ||
| * directories by default (see the $storagePathChannels property). | ||
| * | ||
| * For this to work correctly: | ||
| * - this bootstrapper must run *after* FilesystemTenancyBootstrapper, | ||
| * since FilesystemTenancyBootstrapper adjusts storage_path() for the tenant | ||
| * - storage path suffixing has to be enabled (= config('tenancy.filesystem.suffix_storage_path') | ||
| * must be true), since the storage path suffix is what separates filesystem-based logs | ||
| * | ||
| * For logging channels that are not filesystem-based, see the $channelOverrides logic. | ||
| * | ||
| * @see Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper | ||
| */ | ||
| class LogTenancyBootstrapper implements TenancyBootstrapper | ||
| { | ||
| protected array $defaultConfig = []; | ||
|
|
||
| protected array $configuredChannels = []; | ||
|
|
||
| /** | ||
| * Logging channels whose paths use storage_path() by default in the logging config. | ||
| * | ||
| * All channels included here will be configured to use tenant-specific storage paths | ||
| * generated using storage_path() in the tenant context. | ||
| * | ||
| * This is the default behavior. The $channelOverrides property can be used to override | ||
| * this behavior (the overrides take precedence over $storagePathChannels). | ||
|
Comment on lines
+44
to
+45
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I clarified the language in b51d5ca (too many mentions of "defaults" which don't make sense to me) but still why are we calling the storage approach default when it just serves a different purpose than the approach for overriding specific channels? It might be "default" in the sense that Laravel apps would by default use one of the two filesystem based logs, but that's more of a default configuration of this bootstrapper rather than being something like the default mode of this bootstrapper when there are just two separate config options for different purposes. It's likely that even apps using filesystem based logs would still be customizing this config. So we do want to have sane default values for these static props but I'd avoid saying something is the default approach or default behavior. The default value of the property speaks for itself but the docblock is intended for people customizing that value. |
||
| * | ||
| * Requires FilesystemTenancyBootstrapper to run before this bootstrapper, | ||
| * and storage path suffixing to be enabled. | ||
| * | ||
| * @see Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper | ||
| */ | ||
| public static array $storagePathChannels = ['single', 'daily']; | ||
|
|
||
| /** | ||
| * Custom channel configuration overrides. | ||
| * | ||
| * All channels included here will be configured using the provided override. | ||
| * The overrides take precedence over the $storagePathChannels behavior | ||
| * when both approaches are used for the same channel. | ||
| * | ||
| * You can either map tenant attributes to channel config keys using an array, | ||
| * or provide a closure that returns the full channel config array. | ||
| * | ||
| * Examples: | ||
| * - Array mapping: ['slack' => ['url' => 'webhookUrl']] | ||
| * - this 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])] | ||
| * - this merges ['url' => $tenant->slackUrl] into the channel's config. | ||
| * | ||
| * So the channel overrides can be arrays and closures that return arrays. | ||
| */ | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| public static array $channelOverrides = []; | ||
|
Comment on lines
+54
to
+72
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the key in this array is e.g. function (Config\Repository $config, Tenant $tenant): void {
$config->set('something', something based on $tenant);
}As opposed to returning a value that'd directly override the channel: function (array $channel, Tenant $tenant): array {
return array_merge($channel, [overrides based on $tenant]);
}The current approach would let you do for instance
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be resolved now f878aaf
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Swapped the parameter order. Example for current usage of overrides: LogTenancyBootstrapper::$channelOverrides = [
'single' => function (Tenant $tenant, array $channel) {
return array_merge($channel, ['path' => storage_path("logs/override-{$tenant->id}.log")]);
},
];Also updated the comments to clarify the bootstrapper's behavior. So I think this review can be resolved now |
||
|
|
||
| public function __construct( | ||
| protected Repository $config, | ||
| protected LogManager $logManager, | ||
| ) {} | ||
|
|
||
| public function bootstrap(Tenant $tenant): void | ||
| { | ||
| $this->defaultConfig = $this->config->get('logging.channels'); | ||
| $this->configuredChannels = $this->getChannels(); | ||
|
|
||
| try { | ||
| $this->configureChannels($this->configuredChannels, $tenant); | ||
| $this->forgetChannels($this->configuredChannels); | ||
| } catch (\Throwable $exception) { | ||
| // If an exception is thrown while updating the logging config, the logging config | ||
| // could be left in a corrupt state, so we revert to the original config to | ||
| // to avoid logging the exception in a tenant channel or a broken channel. | ||
| $this->revert(); | ||
|
|
||
| // We re-throw the exception after having reverted the logging config to central. | ||
| throw $exception; | ||
| } | ||
| } | ||
|
|
||
| public function revert(): void | ||
| { | ||
| $this->config->set('logging.channels', $this->defaultConfig); | ||
|
|
||
| $this->forgetChannels($this->configuredChannels); | ||
| } | ||
|
|
||
| /** | ||
| * Channels to configure and forget from the log manager so they can be | ||
| * re-resolved with the new, tenant-specific config on the next use. | ||
| * | ||
| * Includes: | ||
| * - the default channel (primarily because it can be 'stack') | ||
| * - all channels in the $storagePathChannels array | ||
| * - all channels that have custom overrides in the $channelOverrides property | ||
| */ | ||
| protected function getChannels(): array | ||
| { | ||
| /** | ||
| * Include the default channel in the list of channels to configure/re-resolve. | ||
| * | ||
| * Including the default channel is harmless (if it's not overridden and not in $storagePathChannels, | ||
| * it'll just be forgotten and re-resolved on the next use with the original config), and for the | ||
| * case where 'stack' is the default, this is necessary since the 'stack' channel will be resolved | ||
| * and saved in the log manager, and its stale config could accidentally be used instead of the stack member channels. | ||
| * | ||
| * For example, when you use 'stack' with the 'slack' channel, | ||
| * if only 'slack' is forgotten, 'stack' would still use the stale cached 'slack' driver, | ||
| * and if only 'stack' is forgotten, the 'slack' channel's config would remain unchanged (central). | ||
| */ | ||
|
Comment on lines
+105
to
+127
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we assuming that the only way a To me it'd make way more sense and be much cleaner to simply look at all the There might be an extra complication there where perhaps |
||
| $defaultChannel = $this->config->get('logging.default'); | ||
|
|
||
| return array_filter( | ||
| array_unique([ | ||
| $defaultChannel, | ||
| ...static::$storagePathChannels, | ||
| ...array_keys(static::$channelOverrides), | ||
| ]), | ||
| fn (string $channel): bool => $this->config->has("logging.channels.{$channel}") | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * 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 (overrides take precedence over storage path channels). | ||
| */ | ||
| 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 a tenant-specific directory (default behavior). | ||
| // The tenant log will be located at e.g. "storage/tenant{$tenantKey}/logs/laravel.log". | ||
| $originalChannelPath = $this->config->get("logging.channels.{$channel}.path"); | ||
| $centralStoragePath = Str::before(storage_path(), $this->config->get('tenancy.filesystem.suffix_base') . $tenant->getTenantKey()); | ||
|
|
||
| // The tenant log will inherit the segment that follows the storage path from the central channel path config. | ||
| // For example, if a channel's path is configured to storage_path('logs/foo.log') (storage/logs/foo.log), | ||
| // the 'logs/foo.log' segment will be passed to storage_path() in the tenant context (storage/tenant123/logs/foo.log). | ||
| $this->config->set("logging.channels.{$channel}.path", storage_path(Str::after($originalChannelPath, $centralStoragePath))); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| 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) { | ||
| /** @var Tenant&Model $tenant */ | ||
| $tenantAttribute = $tenant->getAttribute($tenantAttributeName); | ||
|
|
||
| if ($tenantAttribute !== null) { | ||
| $this->config->set("logging.channels.{$channel}.{$configKey}", $tenantAttribute); | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } elseif ($override instanceof Closure) { | ||
| $channelConfigKey = "logging.channels.{$channel}"; | ||
|
|
||
| $result = $override($tenant, $this->config->get($channelConfigKey)); | ||
|
|
||
| if (! is_array($result)) { | ||
| throw new InvalidArgumentException("Channel override closure for '{$channel}' must return an array."); | ||
| } | ||
|
|
||
| $this->config->set($channelConfigKey, $result); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Forget all passed channels from the log manager so that | ||
| * they can be re-resolved with the updated (tenant-specific) | ||
| * config on the next logging attempt. | ||
| */ | ||
| protected function forgetChannels(array $channels): void | ||
| { | ||
| foreach ($channels as $channel) { | ||
| $this->logManager->forgetChannel($channel); | ||
| } | ||
| } | ||
| } | ||
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.
This language seems weird.
singleanddailymay be the default in a fresh Laravel app but they're just usual channels, there may be many more or different ones.Would phrase this differently.