MindscapeHQ / serilog-sinks-raygun

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

Adding ability to override MachineName on Raygun #49

Closed jlinker7 closed 1 year ago

jlinker7 commented 1 year ago

When working with Mobile Devices, machine name was often coming in as localhost. I realized that it is using the user specified name, in our context that was not helpful. We wanted to override the machine name to populate it with the device type. Raygun pointed us in the direction of the SendingMessage event, and suggested connecting to it and overriding the machine there.

Upon further review I realized we were using Serilog with the Raygun Sink and that it was spinning up its own Raygun client, so I could not add a listener to the event handler. They then suggested submitting a pull request to this repository to allow passing in the machineName, and if passed in to then add a listener to the SendingMessage event. I would love to see this merged in as soon as possible if it looks ok to the maintainers. Thanks.

jlinker7 commented 1 year ago

@QuantumNightmare Any chance this change could be considered? Thanks!

QuantumNightmare commented 1 year ago

Thanks for submitting this PR. We have scheduled some time next week to go over it.

jlinker7 commented 1 year ago

@QuantumNightmare Thank you!

QuantumNightmare commented 1 year ago

We've taken a look through your PR, but before moving ahead with this, it's given us ideas of making the solution more flexible.

Specifically, we're thinking about being able to pass a function to the RaygunSink. This would get called when the message is about to be sent to Raygun. This function would allow anyone using this sink to manipulate any part of the Raygun payload based on their use cases. In your case you could use this to set the MachineName property. The thought behind this idea is to avoid filling up the constructor will specific single-use arguments over time.

We'll need some time to experiment with this idea to see how well it works out, and would get back to you with how it goes.

jlinker7 commented 1 year ago

@QuantumNightmare Sounds like a great idea, thanks for keeping me updated. If the function idea doesn't work for some reason a fall back might be to have an interface that has all of the properties that can be changed on the Raygun payload, the user could set the properties they wish to update and then your code could iterate through only updating the ones that are set.

Either way I look forward to your solution!

QuantumNightmare commented 1 year ago

Thank you, that's a good alternative that we hadn't considered. Indeed we could explore that if the before-send function doesn't pan out 👍

PanosNB commented 1 year ago

Hi @jlinker7 ,

QuantumNightmare and I worked on a PR (now merged on dev) that exposes an OnBeforeSend action. Would you be able to confirm with us if that solution works for your use case? We tailored the unit tests against it as well as added a console sample app that achieves renaming the machine name before sending the message to Raygun.

Please let us know if that works ok for you (plus any other comments/feedback you might have) and then we'll proceed with release v5.2.0 on nuget.

Thanks Panos

jlinker7 commented 1 year ago

@PanosNB, It works beautifully! I tested with and without passing in the parameter and both work as expected. Thank you for improving on my solution, and for providing the specific example for my scenario in the samples! I will close my Pull Request as your solution is more flexible and works!

PanosNB commented 1 year ago

Thanks for your kind feedback @jlinker7 ! Please, keep us in the loop for any potential improvements to our products. We'll proceed with releasing the new version to nuget today then

PanosNB commented 1 year ago

Hi @jlinker7 , version 5.2.0 is up! https://www.nuget.org/packages/Serilog.Sinks.Raygun/5.2.0 Thanks for your work on this! Looking forward to any other comments and ideas you might have!