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

Factor out mapper caches into their own package #295

Closed
matthiasr opened this issue Mar 5, 2020 · 2 comments · Fixed by #363
Closed

Factor out mapper caches into their own package #295

matthiasr opened this issue Mar 5, 2020 · 2 comments · Fixed by #363
Labels
enhancement library Issues pertaining to the use of the packages as a library more than the exporter itself

Comments

@matthiasr
Copy link
Contributor

This is a follow-up to #281. Part of #234.

The mapper package has taken on a lot of variation and code branches, especially with the choice to use no/LRU/RR caching.

"Cache the result of a mapping" is a separate thing from "map this to that". However, they both have the same inputs (event with statsd metric types + dot-notation metric + key/value tags) and outputs (Prometheus metrics type + underscore metric name + key/value labels). These may need to be formalized a bit but fundamentally we are already handling them.

Define an interface that an initialized mapper should fulfill. Make the current mapper fulfill it. Write (based on the current caching code) several implementations that perform caching. They should be initialized with an "underlying" mapper (which could be the real mapper, a mock, or a completely different implementation).

Delete all caching related code from the mapper.

@matthiasr
Copy link
Contributor Author

@bakins I tried to write up how I imagine the cache extraction to go. Does this make sense? Do you want to claim this right away or should others feel free to do it?

@matthiasr
Copy link
Contributor Author

Another benefit of the defined interface is that the escapeMetricName fallback (#289) could be another implementation of the mapper interface, cleaning up the responsibilities further.

@matthiasr matthiasr added the library Issues pertaining to the use of the packages as a library more than the exporter itself label Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement library Issues pertaining to the use of the packages as a library more than the exporter itself
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant