15

Resolved

Log4Net Logger should output additional information

description

URL, Referrer, and Tenant name would be helpful.

It's the kind of update that does belong in core. I don't see a reason that it should be a module extension. There's no reason why anyone wouldn't want that information.

Discussion posted here as well: http://orchard.codeplex.com/discussions/429913

file attachments

comments

morrisonbrett wrote Jan 18, 2013 at 3:08 AM

Updated core to support logging of additional properties. If the extended properties are available at the time of logging, they are set as available to the logging appender.

Commit message from attached patch:

Work Item: http://orchard.codeplex.com/workitem/19401
Added logging of Request Url, Referring Url, Tenant Name, if those properties are available, they're part of
the log4net stream. To use them in log4net config, use the %P{propertyname} token which is the token for custom property.
Properties added:
    %P{Url}
    %P{HttpReferer} NOTE: misspelling is acknowledged http://en.wikipedia.org/wiki/HTTP_referer
    %P{Tenant}

TheMonarch wrote Feb 26, 2013 at 1:28 PM

Has this work item gone through triage yet? If so, when will this patch be integrated?

BertrandLeRoy wrote Feb 26, 2013 at 5:07 PM

It hasn't: it's still Proposed/Unassigned.

BertrandLeRoy wrote Feb 26, 2013 at 6:42 PM

There is a patch, this is useful and proven to work in at least one production environment. We should take this.

TheMonarch wrote Feb 26, 2013 at 9:34 PM

I've also been running the patch in prod since this morning. No issues yet.

mberacochea wrote Apr 5, 2013 at 4:45 PM

Just applied the patch to in 1.5.1 and seems to work correctly. Great debug information.

morrisonbrett wrote Apr 5, 2013 at 6:24 PM

Glad you like it. It's great especially for multi-node environments.

jrmurdoch wrote Jul 29, 2013 at 5:33 AM

Any update on when this patch is likely to be integrated? Tenant name is a must for anyone using multi tenancy

sfmskywalker wrote Jul 29, 2013 at 7:02 AM

It's marked to be included as part of 1.7.1, which will probably be released in a matter of weeks right after 1.7. is released. We want to release more often, so it should be soon.

sfmskywalker wrote Nov 5, 2013 at 10:44 PM

Brett, the attached patch file is a Mercurial file. Would you mind sending a pull request instead? Thanks!

jao28 wrote Nov 6, 2013 at 2:56 PM

Brett, I just integrated your changes by hand from your Mercurial as they look like exactly what I need. I did add an "AddExtendedThreadInfo()" onto one of the methods (where it seems like you might have missed one):
public void Debug(String message) {
    if (IsDebugEnabled) {
        AddExtendedThreadInfo();
        Logger.Log(declaringType, Level.Debug, message, null);
    }
}
Other than that, looks fantastic. Please let me know if there was a reason to not have the extended thread info on that method.

Also, I am already on .GIT and could set up a pull request if this is something you are not able to do / don't have the time. Just shoot back a comment and would be glad to help.

morrisonbrett wrote Nov 6, 2013 at 4:19 PM

Sure, go ahead. I'm on git too (migrated 50+ repos a few months ago). I haven't had a chance to cherry pick my original commit. Go ahead and submit it.

jao28 wrote Nov 6, 2013 at 6:58 PM

I have gone ahead and attached the .git patch (for those who desire a quick fix) and the pull request can be found here: https://orchard.codeplex.com/SourceControl/network/forks/jao28/development/contribution/5625

hkui wrote Nov 7, 2013 at 1:25 PM

Wouldn't it be more convenient if we had logs per tenant?

So a codeplex.log, orchardproject.log, etc.
And then an orchard log (for the stuff that happens not specifically inside a tenant).

jao28 wrote Nov 7, 2013 at 2:16 PM

That is an interesting thought. My only concern would be the volume of logs I would have to look through to ensure there are no issues in the website. For example, if I have 50 tenants, that would require I check 50+ logs to ensure there are no trending problems. For me, having the one log file to monitor with the ability to trace back to a tenant / url if needed (as the current patch provides) is about perfect.

One advantage I could see with your suggestion though is if you want to allow a client to manage their own tenant and see their own logs but not any of your other tenants. This is not appealing for my setup, but could be for others. I look forward to hear what others have to say.

morrisonbrett wrote Nov 7, 2013 at 3:07 PM

We actually have configured log4net.config to log to a database, with tenant as a column. We can query it easily by tenant. We also have an hourly email that rolls up the errors and summarizes, sorted by repeat duplicates and error count.

jao28 wrote Nov 7, 2013 at 3:47 PM

Ok, that was a mean tease :). What are the odds this is a module that you would love to share with the community? Having multiple tenants, I can see that would be an excellent feature in managing the log files. I suppose if you don't have a module, I might be inspired enough to build one as I think it is a great idea. Of course, if you have an existing module, I actively contribute to source code so it would help grow your module also. Look forward to learning more (feel free to contact me offline if we are hi-jacking this work item).

hkui wrote Nov 7, 2013 at 5:08 PM

I'd be interested in that too!

sebastienros wrote Nov 7, 2013 at 5:36 PM

@hkui, I have tried to do that but couldn't make it work. I think you can't get the tenant's name at the time the logger is created.

morrisonbrett wrote Dec 4, 2013 at 9:45 PM

Is the pull request by jao28 accepted?

morrisonbrett wrote Dec 4, 2013 at 9:46 PM

Ah - I see it was. Thanks Seb!

sebastienros wrote Feb 4 at 5:18 PM

Fixed in changeset 0c124207eb3f54f9a1c3d5105eb477774fbc3dd5