archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.64k stars 429 forks source link

Logging with Tenancy #370

Closed watercayman closed 4 years ago

watercayman commented 4 years ago

I have multiple elements that are logged for each tenant (logins, file uploads, security breach attempts, etc), that each tenant needs to see via an admin log viewer.

Reading the Filesystem docs, I note it says "Logs will be saved to storage/logs regardless of any changes to storage_path(), and regardless of tenant."

However, I can't show a log to the client that has other tenant's log info in it, so I have tried to add it into Tenancy, but am failing. I have tried:

What am I missing? Is there an easy way to Log for each client?

Thank you.

stancl commented 4 years ago

Hey, I've never really played with scoping logs yet. Haven't had a use case for that.

watercayman commented 4 years ago

Thanks for the response. Just checking if it was something i was missing. I may try to write some kind of Log function myself to solve, so I can keep using this excellent package.

watercayman commented 4 years ago

Just a follow-up FYI.

I went through about 15-20 other attempts to get this working. Two were successful. There is some very strange priority in the Laravel Log area where the storage_path seems like it is almost hard-coded for the 'daily' and 'single' drivers. No matter what I write for the path, it comes back logs/laravel.log. Even if I could successfully pull in the tenant->id to config, it would still overwrite the path after it reads the config file (logging.php).

Two mostly working solutions:

1) Write a custom named channel that still uses the daily/single driver. Use this channel (either by Log::channel(), or by env variable globally) and then override the channel via config() on every call to Log().

2) Write a custom driver and the class to support it. Even the tutorials say avoid this or find one already written. Which is what I did. This is not well documented in Laravel.

Samuel, my recommendation on this is that unless you get a bunch of people asking for this very specific area - I'm not sure I'd spend the time to scope Logs into your package.

stancl commented 4 years ago

Re: variant 1 - can't you do this?

// AppServiceProvider::boot()
tenancy()->hook('bootstrapped', function (TenantManager $tm, Tenant $tenant) {
    config(...); // change log file
});
watercayman commented 4 years ago

Well... yep. I was so close to this a half dozen times, but didn't use the bootstrapped event. Yes! Not only does this work for variant 1, it also supersedes whatever was causing Laravel to stop the config change on the daily/single drivers. IE storage_path is successfully tenant aware within this hook, whereas it would not override when using config() within the app.

So - this is the answer for Logging, there is no need to do a single bit of what I said above, lol, your package already nails it perfectly.

Thank you again.

stancl commented 4 years ago

Glad to help :) Can you share the final code you went with? So that if someone needs this in future, they can find it here.

watercayman commented 4 years ago

Sure - pretty much exactly what you wrote already:

 tenancy()->hook('bootstrapped', function (TenantManager $tm, Tenant $tenant) {
       // change log file folder to sit within the tenant path
       config(['logging.channels.daily.path' => storage_path().'/logs/laravel.log',]); 
  });

One of the things I had tried previously was a workaround because i couldn't get storage_path() to overwrite: config(['logging.channels.daily.path' => 'tenant'.$tenant->id.'/logs/laravel.log']); but this fails to go to the right folder.

The daily Log method still renames the file after this point (laravel-YYYY-etc), but using the tenant aware storage path, it separates out to the right folder for all levels of logging.

watercayman commented 4 years ago

Hi Samuel,

Had to update to V3 with Laravel 8... and I'm back to this same issue. The above hook in V2 was perfect, and worked across all Logs, info, debug, and error.

I am unable to achieve success with V3, though. Nearly every log that should be going to a tenant is going to the central app. This is for Queued Jobs only.

I am running these jobs within a tenant function, and after changing out all the V2 code, the tenancy is working well now for these jobs. But even adding a config(), it is still sending tenant logs to the central log.

I've tried many combinations of similar to below:

` tenant()->run(function ($tenant) {
         config(['logging.channels.tenant.path' => storage_path("logs/laravel-" . date('Y-m-d') . '.log')]);`

Using daily Log, normal log, etc. I also tried without using the config() method, thinking perhaps it was already tenant aware, but no luck.

Am I missing something obvious, or am I doing something wrong with this new version?

stancl commented 4 years ago

Your previous code was:

 tenancy()->hook('bootstrapped', function (TenantManager $tm, Tenant $tenant) {
       // change log file folder to sit within the tenant path
       config(['logging.channels.daily.path' => storage_path().'/logs/laravel.log',]); 
  });

What exact code are you using now?

run() doesn't replace hook(). The Laravel event system is used now.

watercayman commented 4 years ago

Thanks for the quick response. I will have to study the docs on how to use events here. I did not realise this was the way to do this.

Currently for my commands, I'm loading all tenants and looping on them (like the old V2 trait used to do).

For jobs I'm loading the tenant in the handle() method like this:

public function handle()
{
    tenant()->run(function ($tenant) {
          config(['logging.channels.tenant.path' => storage_path("logs/laravel-" . date('Y-m-d') . '.log')]);
          // do some tenant DB stuff
          $emp = \App\Employee::something();
         \Log::channel('tenant')->info('Email sent to: ' . $emp->name);

I'll go learn the event stuff - thank you for your help.

stancl commented 4 years ago

To just rewrite the old code:

 tenancy()->hook('bootstrapped', function (TenantManager $tm, Tenant $tenant) {
       // change log file folder to sit within the tenant path
       config(['logging.channels.daily.path' => storage_path().'/logs/laravel.log',]); 
  });

becomes

Event::listen(TenancyBootstrapped::class, function () {
       // change log file folder to sit within the tenant path
       config(['logging.channels.daily.path' => storage_path().'/logs/laravel.log',]); 
  });

obviously import both Event and TenancyBootstrapped

watercayman commented 4 years ago

LOL. That's easy!

Roger that. Thank you again!

stancl commented 4 years ago

Happy to help :)

soufiene-slimi commented 3 years ago

Hi @watercayman, @stancl

This should work for version 3, seems like the issue was to refresh the log container.

In AppServiceProvider, add this:

use Illuminate\Support\Facades\Event;
use Stancl\Tenancy\Events\TenancyBootstrapped;

...

/**
* Bootstrap any application services.
*
* @return void
*/
public function boot()
{
    Event::listen(TenancyBootstrapped::class, function (TenancyBootstrapped $event) {
        config(['logging.channels.daily.path' => storage_path('logs/laravel.log')]);
        app()->instance('log', new \Illuminate\Log\LogManager(app()));
    });
}

And change this config to make the logs folders more readable:

'filesystem' => [
    /**
     * Each disk listed in the 'disks' array will be suffixed by the suffix_base, followed by the tenant_id.
     */
-   'suffix_base' => 'tenant',
+   'suffix_base' => 'tenant_',

PS: If you are not using the FilesystemTenancyBootstrapper, then there is no need for changing the filesystem config, and the changes should look like this:

use Illuminate\Support\Facades\Event;
use Stancl\Tenancy\Events\TenancyBootstrapped;

...

/**
* Bootstrap any application services.
*
* @return void
*/
public function boot()
{
    Event::listen(TenancyBootstrapped::class, function (TenancyBootstrapped $event) {
-       config(['logging.channels.daily.path' => storage_path('logs/laravel.log')]);
+       config(['logging.channels.daily.path' => storage_path('logs/'.$event->tenancy->tenant->id.'/laravel.log')]);
        app()->instance('log', new \Illuminate\Log\LogManager(app()));
    });
}
tamkeen-tms commented 3 years ago

Sadly all of the available solutions here doesn't work anymore! tried changing the config on the TenancyBootstrapped event, but it doesn't change! Seems like Laravel sets this in a very early stage now and changing it doesn't take effect.

MrJmpl3 commented 3 years ago

Sadly all of the available solutions here doesn't work anymore! tried changing the config on the TenancyBootstrapped event, but it doesn't change! Seems like Laravel sets this in a very early stage now and changing it doesn't take effect.

Works for me, and I dont need refresh the container (Well, I don't know why need refresh container in the previous example)

I use in a Feature:

<?php

namespace App\Tenancy\Feature;

use Illuminate\Support\Facades\Config;
use Illuminate\Support\Facades\Event;
use Stancl\Tenancy\Contracts\Feature;
use Stancl\Tenancy\Events\RevertedToCentralContext;
use Stancl\Tenancy\Events\TenancyBootstrapped;
use Stancl\Tenancy\Tenancy;

class LogConfig implements Feature
{
    public function bootstrap(Tenancy $tenancy): void
    {
        Event::listen(TenancyBootstrapped::class, function () {
            Config::set('logging.channels.daily.path', storage_path('logs/laravel.log'));
        });

        Event::listen(RevertedToCentralContext::class, function () {
            Config::set('logging.channels.daily.path', storage_path('logs/laravel.log'));
        });
    }
}
masiorama commented 2 years ago

Hello, I might be wrong so instead of opening a new issue I prefer to write down here.

In the docs this is stated (v3): "You want to use the bootstrapped event if you want to execute something after the app has been transitioned to the tenant context." , aka TenancyBootstrapped.

As far as I understand this should be triggered only once when transitioning from master to one tenant (identified by domain).

Is this assumption right or am I totally missing the point here?

Because in my tests this event is executed on every request, which is no use to me, since I would like to log when the logged user access the tenant for the first time (in the current session).

This could be an error in my configuration, so any suggestion could point me to the right direction.

Thanks!