[FEATURE] Add FactorySuccess event

[bartlomiejgawel]

Is your feature request related to a problem? Please describe. I use background factory completion and would like to add a metric on successfully executed factory methods. I know there is an event called BackgroundFactorySuccess but it doesn't cover cases where the factory method is either synchronous or executed before a synthetic timeout: https://github.com/ZiggyCreatures/FusionCache/blob/main/src/ZiggyCreatures.FusionCache/Events/FusionCacheEventsHub.cs#L65

It could be useful to ensure that factory methods are executed correctly. I know I can check a FactoryError event to verify that but it would be easier to double-check whether:

1. Metrics are collected.
2. The factory method works.
3. How many times has the factory method been successfully called.

Describe the solution you'd like I would like to have an event called FactorySuccess analogous to FactoryError.

Describe alternatives you've considered I can increment the metric within the factory method. Unfortunately, this approach has the following disadvantages:

• It's not a generic solution.
• It doesn't work out of the box. I have to add an explicit dependency to my metrics to do that. I would prefer to keep it optional by adding an appropriate plugin.

I also thought about checking distributed (or eventually memory) cache sets but it's not a correct solution.

Additional context Feel free to express your opinion. If you wish I can try to add this feature in my free time.

[jodydonetti]

Hi @bartlomiejgawel and thanks for using FusionCache!

I know there is an event called BackgroundFactorySuccess but it doesn't cover cases where the factory method is either synchronous or executed before a synthetic timeout

One thing I'd like to point out is that BackgroundFactorySuccess in fact does not get triggered when the execution successfully finishes before any timeout, BUT it will be triggered even if the factory is synchronous (meaning, no usage of async/await). I don't know if with "synchronous" you meant something else, but I just wanted to clarify this.

Apart from this yeah, you are right: there a FactoryError and a BackgroundFactoryError, but there's only a BackgroundFactorySuccess and not a FactorySuccess. If I remember correctly my thought back then was that a "normal" factory success would be "the norm" whereas a background completion or an error (both normal or in the background) would've been the exception to the rule, and so more interesting to track. But yeah, it makes sense to also add that.

Also, while we are at it, I may think about adding some more events to the mix: do you have any other event needed?

I also thought about checking distributed (or eventually memory) cache sets but it's not a correct solution.

You are correct, in that it would not be a correct solution.

If you wish I can try to add this feature in my free time.

If you like I'm for sure open to contributions! But then again I can imagine that, even though I like to think that I've been able to make the code overall readable and understandable (at least I hope), I'm also aware that the subject can be quite complex and the project has grown a lot from its inception, so what for me is an easy addition for others may be more time consuming.

One final thing, if I may: when you say "It doesn't work out of the box. I have to add an explicit dependency to my metrics to do that. I would prefer to keep it optional by adding an appropriate plugin" do you care to expand the "my metrics" and "appropriate plugin" parts? Have you developed your own metrics system or plugin? Have you plugged an existing one? Can I know which one and how, at a high level? How is it working?

Of course feel free not to answer in case you can't (NDAs, company policies, etc) or just don't want, no worries! It's just that I'm really curious about how my library is being used, and metrics it's definitely an interesting topic I'd like to explore more.

I will update you about this as soon as I have something ready to show you (hopefully soon).

Thanks!

[bartlomiejgawel]

Thank you for the detailed answer!

One thing I'd like to point out is that BackgroundFactorySuccess in fact does not get triggered when the execution successfully finishes before any timeout, BUT it will be triggered even if the factory is synchronous (meaning, no usage of async/await). I don't know if with "synchronous" you meant something else, but I just wanted to clarify this.

Yes, you're absolutely right, that's what I meant. I should have made it more precise.

Also, while we are at it, I may think about adding some more events to the mix: do you have any other event needed?

I also thought about timeout events (other than synthetic) If I remember correctly, when I use distributed cache and it (for some reason) is not available then no event about this situation is being triggered. It would be nice to have information about such cases.

Have you developed your own metrics system or plugin? Have you plugged an existing one? Can I know which one and how, at a high level? How is it working?

Yes, I've created a nuget package with a plugin to add metrics. I really like the idea of plugins because I can register my own plugin in any app and it starts publishing metrics without any additional work. I saw there is a similar package created by Joe but I needed to customize it and use other dependencies to publish metrics so I created a new solution. Unfortunately, I cannot publish sources of that package but it works similarly.

But then again I can imagine that, even though I like to think that I've been able to make the code overall readable and understandable (at least I hope), I'm also aware that the subject can be quite complex and the project has grown a lot from its inception, so what for me is an easy addition for others may be more time consuming.

Sure, I totally understand. I really enjoy using FusionCache and simply thought that I could help you in this way to thank you for this package and your hard work!

Once again, thank you for everything!

[jodydonetti]

Hi @bartlomiejgawel , I have some news.

I also thought about timeout events (other than synthetic) If I remember correctly, when I use distributed cache and it (for some reason) is not available then no event about this situation is being triggered. It would be nice to have information about such cases.

I'm not sure I can reliably detect non-synthetic timeouts, because each piece of code out there may use different exception types, not necessarily deriving from the core System.TimeoutException type itself.

Having said this, if we can accept this tradeoff, it is surely something I can add: in that case though, how do you think it should work? Suppose I add a new FactoryTimeout event: should that event be raised for any timeout event or only for non-synthetic timeout events?

Instead of this, I'm thinking about adding a GenericFactoryError event (or using the existing FactoryError event and making a small breaking change) and use it to centralize any kind of factory exception event, with various props in the eventargs like bool IsBackground (to indicate if the factory throw an error while running in the background), Exception Error (with the exception thrown), etc... maybe a unified, centralized approach would be better.

What do you think?

Yes, I've created a nuget package with a plugin to add metrics. I really like the idea of plugins because I can register my own plugin in any app and it starts publishing metrics without any additional work.

Awesome, thanks for sharing! And I'm glad to know you're liking it 🙂

Once again, thank you for everything!

Thanks to you for using it, hope you're finding it useful.

I'm release a new version with the new FactorySuccess event today or tomorrow.

[bartlomiejgawel]

Having said this, if we can accept this tradeoff, it is surely something I can add: in that case though, how do you think it should work? Suppose I add a new FactoryTimeout event: should that event be raised for any timeout event or only for non-synthetic timeout events?

Good question. Both options have pros and cons. I think that your second idea about adding a GenericFactoryError event is the best choice.

Instead of this, I'm thinking about adding a GenericFactoryError event (or using the existing FactoryError event and making a small breaking change) and use it to centralize any kind of factory exception event, with various props in the eventargs like bool IsBackground (to indicate if the factory throw an error while running in the background), Exception Error (with the exception thrown), etc... maybe a unified, centralized approach would be better.

An excellent idea. I do like both: adding a GenericFactoryError event or using the existing FactoryError event. Maybe I slightly prefer the second option (using the existing FactoryError event) but once again - both options are great. I think it'll cover every use case scenario and satisfy even the most demanding person 🙂

Awesome, thanks for sharing! And I'm glad to know you're liking it 🙂

You're welcome!🙂

[jodydonetti]

The new v0.19.0 has been released, which includes this 🎉