Skip to content
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

Critical time not taking delayed delivery into account breaking the metric #5893

Closed
ramonsmits opened this issue Feb 2, 2021 · 6 comments
Closed
Assignees

Comments

@ramonsmits
Copy link
Member

ramonsmits commented Feb 2, 2021

A user requested if it was possible to take delayed delivery into account for critical time which makes sense. The framework currently does not have a header that stores the DeliveryAt or something.

This makes saga timeouts more useful as such timeouts can be far into the future and it doesn't make sense to use TimeSent for these.

Having such a header gives the ability to calculate critical time based not on TimeSent but on DeliveryAt. We could then do something like CriticalTime = ProcessesAt - (DeliveryAt??TimeSent)

Related to this is the following bug in the timeout manager which resets the TimeSent header for retries.

@ramonsmits ramonsmits changed the title Critical time not taking delayed delivery into account Critical time not taking delayed delivery into account breaking the metric Feb 2, 2021
@robbaman
Copy link

robbaman commented Feb 2, 2021

I've noticed this as well but since it seemed so obvious I assumed there was a good reason for it to work this way. Since we have some services that send have timers being sent an hour in the future the 'critical time' measurement has never really worked for us.

If this could be changed that would be awesome.

@MarcWils
Copy link

Any update on this issue? We would like to monitor on critical time.

@WilliamBZA
Copy link
Member

This is still on our radar. It won't be done as part of V8, but will possibly form part of a v8.1.

@TravisNickels
Copy link
Member

A new DeliverAt header has been added to messages in NServiceBus 7.7, which is now used to calculate the critical time of a message. This functionality is available for monitoring using NServiceBus.Metrics 3.1 and with ServicePulse. This will be available for auditing with ServiceInsight in a future release of ServiceControl.

NServiceBus 7.7 package: https://www.nuget.org/packages/NServiceBus/7.7.0
NServiceBus.Metrics 3.1 package: https://www.nuget.org/packages/NServiceBus.Metrics/3.1.0

This is the PR for NServiceBus 7.7: #6282
This is the PR for NServiceBus.Metrics 3.1: #291

@robbaman
Copy link

So do I just need to update my services with these new package versions or does it also require an update of the ServiceControl Windows services and the ServicePulse web interface?

@TravisNickels
Copy link
Member

@robbaman ServiceControl and ServicePulse do not require an update for this. You will need to update NServiceBus to 7.7.0 and if you are using the packages NServiceBus.Metrics.ServiceControl or NServiceBus.ServicePlatform.Connector on your endpoint(s) to monitor your messages using ServicePulse, you will need to add an explicit package reference to NServiceBus.Metrics 3.1.0 on the endpoint(s) in order to get the updated critical time attribute to display in the monitoring view in ServicePulse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants