Add App Insights telemetry to Redis Caching

[ncomben]

Hi Hamish

Remember when we did all that lovely Yalago into Cloud work that got canned? Well they've brought it back to life to get us through the peak season. The main thing that has changed since before is that CacheMeIfYouCan is heavily used in the Hotels Engine particularly for the Redis based supplier caching. It turns out that we see no telemetry in App Insights when we enable Redis caching so I've done some work to add it.
I'm open to any suggestions on how the implementation might be improved, but I've tried to stick to the general style.

I'd rather contribute to this than just create our own version of the NuGet, so would appreciate it you would get back to me!

Cheers

Nathan

[hpeebles]

Hey! Long time no speak!

I've had a look through the PR and it would do the job fine but there is already some inbuilt functionality that should make this easier for you to achieve while avoiding adding any new methods to the core codebase.

Check out the class called DistributedCacheEventsWrapperBase<TKey, TValue> (https://github.com/hpeebles/CacheMeIfYouCan/blob/master/src/CacheMeIfYouCan/DistributedCacheEventsWrapperBase.cs).

You could create a class which inherits from this and then overrides the methods that you want to trace.

Eg. https://gist.github.com/hpeebles/510c96786dd4f09ca069dac547b6653c

If the resulting class can be made generic enough then we could create a new CMIYC.ApplicationInsights package, but I think first create it exactly for your use case and then see if its worth making it generic.

I'm happy to jump on a call if you wanted to talk through the different options.

[ncomben]

Hi Hamish... I got side tracked on some other work but I've managed to implement some changes using a wrapper class.
I'm not happy on how the configuration is done., In the time I had, I couldn't figure out a better way of doing it using ICachedObjectConfigurationManager maybe instead of my WithXYZ extensions..

[ncomben]

Would you take a look at this PR again please Hamish?

[hpeebles]

Hey!
Sorry I've been swamped with work so haven't been able to give this a proper look until now.
I'm still thinking that you don't actually need to make any changes to the CMIYC codebase and that you can create a simple wrapper which would live external to this repo.
The data you are tracking is very specific to your use case and in this PR the Redis package now has a dependency on AppInsights which looks pretty nasty.
If the DistributedCacheApplicationInsightsWrapper just sat in a separate package then everything would still work for your use case... I think?
I know you said this "I'd rather contribute to this than just create our own version of the NuGet", but this change (in its current form at least) is far too opinionated to include in the core CMIYC codebase.
Happy to jump on a call to talk through the options though.

[ncomben]

Ah, I didn't quite get what you meant before. I'll see if I can do the same, but external to CMIYC using the interfaces it exposes!

…

On Mon, Dec 6, 2021 at 10:56 PM Hamish Peebles ***@***.***> wrote: Hey! Sorry I've been swamped with work so haven't been able to give this a proper look until now. I'm still thinking that you don't actually need to make any changes to the CMIYC codebase and that you can create a simple wrapper which would live external to this repo. The data you are tracking is very specific to your use case and in this PR the Redis package now has a dependency on AppInsights which looks pretty nasty. If the DistributedCacheApplicationInsightsWrapper just sat in a separate package then everything would still work for your use case... I think? I know you said this "I'd rather contribute to this than just create our own version of the NuGet", but this change (in its current form at least) is far too opinionated to include in the core CMIYC codebase. Happy to jump on a call to talk through the options though. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIPMUBPBELF5SO6CPYSQGMDUPU5TBANCNFSM5IRVEIEA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.