ServiceControl Integration added additional events #5289
Conversation
danielmarbach
left a comment
danielmarbach
left a comment
There was a problem hiding this comment.
Will you also update the sample? As a different PR?
I have an updated version of the sample, but for it to work I will need to update the platform sample with the serviceControl.Contracts... Will I do this after the release? |
seanfarmar
force-pushed
the
exposing-additional-events
branch
from
a56532a to
5015d12
Compare
seanfarmar
force-pushed
the
exposing-additional-events
branch
from
e13a866 to
f4ea629
Compare
seanfarmar
changed the title
Co-authored-by: Mike Minutillo <mike.minutillo@particular.net>
danielmarbach
left a comment
danielmarbach
left a comment
There was a problem hiding this comment.
I think what is missing is the documentation of the flag to disable the event publishing on the settings/configuration page (can also be done in another PR). Other than that this looks good to me
| using NServiceBus.Logging; | ||
| using ServiceControl.Contracts; | ||
|
|
||
| #region AzureMonitorConnectorEventsHandler |
There was a problem hiding this comment.
Do we need to show the whole handler with all the other noise or could we just scope it to the handle method ? Or is it because we consider the part of IHandleMessages<> important?
| using NServiceBus.Logging; | ||
| using ServiceControl.Contracts; | ||
|
|
||
| #region AzureMonitorConnectorEventsHandler |
There was a problem hiding this comment.
Do we need to show the whole handler with all the other noise or could we just scope it to the handle method ? Or is it because we consider the part of IHandleMessages<> important?
There was a problem hiding this comment.
We wanted to show the whole handler including the telemetry client injection and the message name. To make it smaller we moved all other supported messaged to a CustomEventsHandler.
Related to Particular/ServiceControl#2465