-
Notifications
You must be signed in to change notification settings - Fork 27
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
Added parameters 'entryid' and 'attacheventidtokey' to eventlog section #30
Conversation
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.
Hi,
thank you for your pull request.
I added some comments to the changes with suggestions for improvements. Maybe you can have a look at them and incorporate the changes?
Thanks.
@@ -119,8 +125,19 @@ private void OnEntryWritten(object sender, EntryWrittenEventArgs e) | |||
if (!string.IsNullOrEmpty(this.category) && !this.category.Equals(e.Entry.Category, StringComparison.OrdinalIgnoreCase)) | |||
return; | |||
|
|||
if (this.eventid.HasValue && !this.eventid.Equals(e.Entry.EventID)) |
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.
EventID
seems to be obsolete (see https://msdn.microsoft.com/en-us/library/system.diagnostics.eventlogentry.eventid.aspx). Could you change it to InstanceId
? I think it should have the same meaning.
var key = this.key; | ||
if (this.attacheventidtokey) | ||
{ | ||
key = string.Format("{0}.{1}", this.key, e.Entry.EventID); |
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.
same as above
this.channel.Report( | ||
this.key.Replace("${type}", typeTranslation[e.Entry.EntryType]), | ||
key.Replace("${type}", typeTranslation[e.Entry.EntryType]), |
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'd suggest adding a call to .Replace("${eventId}"...
here, instead of introducing the additional configuration option AttachEventIDToKey
. That would be more flexible as well.
<add | ||
key="eventlog.IIS" | ||
protocol="System" source="IIS-IISReset" entrytypes="Error, Warning, Information" | ||
type="counter" eventid="3202" target="statsd" /> |
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 think this should be in the <eventlog>
section.
- InstanceId instead of EventID (obsolete) - InstanceId is of type long - more generic replacement of ${eventId} in key
Hi! This is my first contribution to this project :)
I needed some extra features on eventlog listener, so I've added them.
These two features can be enabled via new parameters on section:
eventid: Filter events to be sent to graphite based on their "EventID".
Let's say I need to trace how may times an IIS appPool is recycled manually. This is shown on Event Viewer as source "WAS" / EventID "5079", so I'd need to configure it like this:
attacheventidtokey: Send every different event type as a separate metric. Enabling this flag, the application will attach the "EventID" to the end of the key. This way you
Let's say I need to count all WAS' Error events. I'd need to configure it like this:
Hope you find this as useful as me :)