Terasology icon indicating copy to clipboard operation
Terasology copied to clipboard

feat(MonitoringSubsystem): provide Micrometer gauges for rendering.world

Open keturn opened this issue 2 years ago • 2 comments

I started looking at sending the stats available on the DebugOverlay screens to Micrometer, and this happened.

Conceptually, this is working in the area of MetricsMode and DebugMetricsSystem, but when I saw those were all relying on formatted strings with comments like this, it was clear I wouldn't be able to use those classes as they are now. https://github.com/MovingBlocks/Terasology/blob/ce8d34fd6799b215b00fd8ff62481af60e63da40/engine/src/main/java/org/terasology/engine/rendering/world/WorldRenderer.java#L216-L219

Example of Defining Meters

https://github.com/MovingBlocks/Terasology/blob/6098b45a2e297f2987bad5b668d6ef3fc2e0a06e/engine/src/main/java/org/terasology/engine/rendering/world/Meters.java#L15-L49

Notable qualities of this:

  • To avoid the need to add Micrometer-specific stuff in to all our current classes, the meters are defined in a separate class. But it can be in the same package to allow them to access package-private variables.
  • Definitions are grouped by the interface the object is registered under in the Context, but a definition may be for a more specific implementation of that interface.
  • We can have these definitions available without connecting them to live instances and publishers.

Other miscellania:

  • The use of Flux here isn't for anything asynchronous. I switched from Stream because Flux has a few convenient processing methods that Stream lacks, such as doOnDiscard and mapNotNull.

Future considerations:

  • There are other kinds of meters besides Gauges. We're sure to use Timers, and probably Counters too. They have the same metadata as gauges (name, description, tags, etc) but they have some state that gauges don't, so there will be some differences in how we set those up.

How to test

Brief description of how to test / confirm this PR before merging

Outstanding before merging

  • [ ] migrate WorldRendererMode to read from the meter registry.
  • [ ] could use translation work, but we'll probably consider these to be more like logger calls than user-facing, and skip them.

keturn avatar Nov 15 '21 06:11 keturn

Oh no, I thought I had finally reached agreement with the type system, but I checked in something that doesn't compile?

I was thinking that the method signatures ended up suspiciously uncluttered in my latest version. Will re-examine tomorrow. 🤞

keturn avatar Nov 15 '21 07:11 keturn

Okay, now it compiles and initializes correctly. 😌

keturn avatar Nov 15 '21 18:11 keturn