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

Add support for Open Telemetry #157

Open
Turnerj opened this issue Apr 12, 2021 · 2 comments
Open

Add support for Open Telemetry #157

Turnerj opened this issue Apr 12, 2021 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@Turnerj
Copy link
Member

Turnerj commented Apr 12, 2021

Open Telemetry has reached v1 and may be worth directly supporting in Cache Tower.

See: https://medium.com/opentelemetry/opentelemetry-specification-v1-0-0-tracing-edition-72dd08936978

  • Tracing: Not sure where this would fit in - may relate closely with logging
  • Logging: Relates to Add a logging extension #150
  • Metrics: May be worth capturing cache hits/misses, background refreshes and the interaction of different layers

Further investigation required.

@Turnerj Turnerj added the enhancement New feature or request label Apr 12, 2021
@mgoodfellow
Copy link
Contributor

Hi @Turnerj

Just my 2 cents on this - I wouldn't tightly bind your lib to Open Telemetry - instead I think an approach of providing something like an ICacheMonitor interface, which has a bunch of definitions for "things" that could be monitored.

These could be things like (off the top of my head - I'm sure there could be many more!):

  • CacheHit(TimeSpan timeTaken, string layerName)
  • CacheMiss(TimeSpan timeTaken, string layerName)
  • CacheException(TimeSpan timeTaken, string layerName, Exception exception)
  • CacheStaleRefresh(TimeSpan timeTaken, TimeSpan cacheAge, string layerName)

By default there would be a NoOpCacheMonitor used unless replaced during setup, which just no ops the calls.

This way an Open Telemetry implementation could be built, or indeed, any other roll-your-own, or of the shelf system could easily be supported. The consumer just needs to wire up a concrete implementation of CacheMonitor which ties into their monitoring system.

@Turnerj
Copy link
Member Author

Turnerj commented Aug 11, 2021

Thanks for the feedback @mgoodfellow !

Everything you've said makes sense and is probably what I should do. Similarly doing it that method allows for integration with event counters too.

Depending on the access points needed to support it, I should be able to achieve this through my extension system (even if it requires new "hook" points).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants