search

MindscapeHQ / serilog-sinks-raygun

A Serilog sink that writes events to Raygun
Apache License 2.0
11 stars 20 forks source link

Forced to specify API Key #31

Closed phillip-haydon closed 3 years ago

phillip-haydon commented 3 years ago

Raygun client can be configured without a key:

https://raygun.com/documentation/language-guides/dotnet/crash-reporting/aspnetcore/

https://raygun.com/documentation/language-guides/dotnet/crash-reporting/mvc/

Can the requirement of needing an api key be removed from the sink so I can just write:

.WriteTo.Raygun()

This also allows raygun client to pick up the diagnostics settings to verify issues with raygun itself.

<RaygunSettings apikey="...." throwOnError="false" />

QuantumNightmare commented 3 years ago

Hi Phillip, thanks for the suggestion, this should be fine. I'll look into making this change for the upcoming version.

QuantumNightmare commented 3 years ago

I've included this change in https://github.com/serilog/serilog-sinks-raygun/pull/32

The sink will no longer check if the applicationKey is null or whitespace, however the applicationKey parameter will still be required. My concern with giving this a default value of null is that it could be overlooked when setting it up in code, but it's the most important parameter to get it working correctly.

Setting this to null or whitespace in a .NET Framework app will cause the internal RaygunClient to fall back to fetching the key from the config if available. In .NET Core however, this fallback doesn't seem to be available, and so it will just not sending crash reports to Raygun.

I'll mention that setting a null applicationKey when setting up the Raygun Serilog Sink isn't required for the other RaygunSettings to be picked up. If you do provide an applicationKey to the Raygun Serilog Sink, then relevant settings will still be read from the config file. Your example of throwOnError wouldn't be used much however, because the Raygun Serilog sink mainly uses the RaygunClient for sending a manually constructed message model - but I'm wanting to change this as seen in the PR I linked.

Let me know if that all sounds alright to you.

phillip-haydon commented 3 years ago

The sink will no longer check if the applicationKey is null or whitespace, however the applicationKey parameter will still be required. My concern with giving this a default value of null is that it could be overlooked when setting it up in code, but it's the most important parameter to get it working correctly.

This sounds perfectly reasonable.

In .NET Core however, this fallback doesn't seem to be available, and so it will just not sending crash reports to Raygun.

Correct, I double checked this and realise I misread, the auto-config for .net core is only available when using the middleware.

https://github.com/MindscapeHQ/raygun4net/blob/master/Mindscape.Raygun4Net.AspNetCore/RaygunAspNetMiddleware.cs#L110

Looks good <3

QuantumNightmare commented 3 years ago

Version 5.0.1 has just been released to NuGet which includes this change.