search

serilog / serilog-sinks-eventlog

A Serilog sink that writes events to the Windows Event Log
Apache License 2.0
50 stars 29 forks source link

Extensible EventIdProvider #22

Closed mrbcmorris closed 6 years ago

mrbcmorris commented 6 years ago

Hey team,

I have a use case where I need to be able to specify a particular EventId for log events. Do you feel this would fit with the goals of the project? I'd be happy to submit a pull request if so.

My current thoughts on implementation are introducing a new interface as an optional configuration parameter.

public interface IEventIdProvider
{
    int Compute(LogEvent logEvent);
}

If the parameter is unspecified, the default implementation would be a wrapper around the current EventIdHash class.

Cheers

nblumhardt commented 6 years ago

Hi Chris - this sounds okay, but I'm not sure how widely it'd be used - is the scenario one you think will be common?

The most conservative approach is probably for you to fork and implement the feature, use the forked version in your project, and if others find this issue and indicate they'd use it then merging it in should be fine.

What do you think?

Cheers, Nick

mrbcmorris commented 6 years ago

I’m not sure how wide of an application this feature would have. Regarding what I’m encountering: We utilize specific ranges of event ids for logging particular issues so we can integrate SCOM alerts over ranges. Bringing serilog into an pre-existing application requires me to persist my event ids for particular issues.

I don’t mind taking the fork approach if you don’t think it would have much use outside of my case.

Thanks for your work on this project!

nblumhardt commented 6 years ago

Great, let me know if you need any more help with it 👍

GavinSutherland commented 6 years ago

Hi,

I'd find this feature useful having had a requirement from our OPs team to log particular events under known EventIds. I couldn't see any implementation yet in the fork from @mrbcmorris.

Is this a feature request we could reopen?

Thanks, Gavin

nblumhardt commented 6 years ago

Hi @GavinSutherland, thanks for the feedback! Yes, if it's more widely-applicable than we first thought, this is something we'd consider integrating. Are you interested in fleshing out a design? Thanks!

GavinSutherland commented 6 years ago

Hi @nblumhardt ... yes I can take a look at this. I forked the repo yesterday so will hopefully get to it soon.

Gav

mrbcmorris commented 6 years ago

Hey guys, just thought I could get the ball rolling with a sample implementation. Let me know what you think!

GavinSutherland commented 6 years ago

Hi @nblumhardt

What is the process you go through (or what criteria needs to be met) for merging work from dev to master then pushing it to NuGet?

Thanks, Gav

nblumhardt commented 6 years ago

Hi Gav,

Normally, a -dev package would have been published automatically, and we'd release a master package once we had feedback on the -dev one.

However, we hit some build problems that prevented https://github.com/serilog/serilog-sinks-eventlog/pull/23 from being published. I'll have a quick dig in now and see if I can work it out.

Cheers!