+1 I like it!
I like that we are not having to maintain duplicate meter and stat code. Also, I think keeping hiding metrics and stats inside an instrumentation class with methods like regionAdded makes it much easier to unit test classes that record metrics. -Dan On Mon, Jun 10, 2019, 5:13 PM Aaron Lindsey <alind...@pivotal.io> wrote: > +1 > > I like this approach compared to the previous proposals because it's > simpler (doesn't require a custom registry) and makes it more > straightforward to replace stats with Micrometer meters in the future. > > - Aaron > > > On Fri, Jun 7, 2019 at 4:58 PM Dale Emery <dem...@pivotal.io> wrote: > >> Proposal for instrumenting Geode >> >> >> *Goals* >> >> - Allow routing measurements to statistics, Micrometer meters, or both as >> appropriate, to allow publishing each measurement to the appropriate places. >> >> - Minimize the intrusiveness of new and existing metrics instrumentation. >> >> - Minimize the amount of code that must know how to route measurements to >> stats and meters. >> >> - Maintain Geode performance. >> >> - Not preclude any options for deprecating and removing Geode’s >> statistics mechanism in the future. And take steps to make deprecating and >> removing the existing statistics mechanism easier. >> >> >> *Proposal* >> >> - Continue to use Geode’s existing style of instrumentation class (e.g. >> CachePerfStats) to instrument code. >> >> - Enhance the instrumentation classes to create and register meters when >> appropriate, and to route each measurement to the appropriate stats and >> meters. >> >> - When we want to route a given measurement to both a statistic and a >> meter, use the "legacy meter" wrapper mechanism (described below). >> >> - Incrementally improve the instrumentation classes to focus more on >> reporting domain events (e.g. regionAdded()) and less on reporting >> measurements (e.g. incRegions()). >> >> >> *Legacy Meter Wrappers* >> >> To route a given measurement to both a Micrometer meter and a Geode >> statistic, do the following in the instrumentation class's constructor: >> >> - Create and register a normal Micrometer meter in the cache's meter >> registry. >> >> - Wrap the registered meter in a custom "legacy meter" that reads and >> writes the stat, and also writes to the registered meter. >> >> In the instrumentation methods (e.g. endPut()): >> >> - Use the legacy meter to report measurements. >> >> I've attached two diagrams below to show how the wrapper mechanism works >> for "counter" style stats and meters. The first is a class diagram, showing >> how the parts relate in general. The second is a sequence diagram that >> shows in some detail how the parts interact to route a given measurement to >> both a meter and a stat. >> >> If you want even more details, Jake Barrett has created a nice "proof of >> concept" implementation: >> https://github.com/apache/geode/compare/develop...pivotal-jbarrett:wip/LegacyMeterBuilder >> >> >> >> Please let us know if you have questions, comments, or concerns. >> >> >> Cheers, >> Dale >> >> — >> Dale Emery >> dem...@pivotal.io >> >> >> >> >> >> >> >> >>
