Overhauling logging strategy

splunk / splunk-sdk-csharp-pcl

Splunk's next generation C# SDK

https://dev.splunk.com/enterprise/docs/csharp

Apache License 2.0

64 stars 46 forks source link

Overhauling logging strategy #29

Closed glennblock closed 9 years ago

glennblock commented 9 years ago

logging.

glennblock commented 9 years ago

@shakeelmohamed take a quick peek.

shakeelmohamed commented 9 years ago

LGTM after nits, & possibly some test changes

itay commented 9 years ago

Is there any way to get the instance name of this of mod input (i.e. the name of the stanza) rather than using the class name?

glennblock commented 9 years ago

@itay It will be significantly more work to do that:

For scheme, we currently don't do anything but return the scheme and that code could fail, so there is no name there.
For validate it would be relatively easy.
For stream it's a lot more complicated as multiple input instances are spun up and started on separate tasks. We'd have to track when the exception occurs, what task it occured on and then figure out based on that task what input it was associated with. Currently we just let the exception occur and bubble up via the task infra.

I think having the fully qualified type name (which includes the namespace) is enough as it should uniquely identify the app and input type.

itay commented 9 years ago

The problem is that even though it'll identify the app and input type, it won't identify which input it is happening on, which is crucial, IMO. That said, something is better than nothing.

glennblock commented 9 years ago

I see what you mean. Let me look at it and see if I can do it easily..

On Tue Dec 09 2014 at 10:11:28 PM Itay Neeman notifications@github.com wrote:

The problem is that even though it'll identify the app and input type, it won't identify which input it is happening on, which is crucial, IMO. That said, something is better than nothing.

— Reply to this email directly or view it on GitHub https://github.com/splunk/splunk-sdk-csharp-pcl/pull/29#issuecomment-66408908 .

glennblock commented 9 years ago

Ok @itay I think I got it working. Basically I add a continuation for each task and in the delegate it will log the associated input name. I've added a unit test as well to confirm the behavior.

glennblock commented 9 years ago

Added unit tests now for testing error logging for Scheme and Validate as well. It has the following behavior now

For stream events it will capture errors for each input instance and record the instance name.
For validate it will capture errors and log the current instance name being validated
For scheme it will just log the full type name.
For any other errors like invalid args, it will log the full type name.

Make sense?