MetricsTest should clear metrics in advance to make test result stable fixes #94

[ocadaruma]

Summary

• Currently, MetricsTest.testMetricsLifecycleManagement fails if CompactionProcessorTest is executed first (depending on test execution order), because metrics for CompactionProcessor isn't cleaned up in any place
  • https://github.com/line/decaton/blob/master/processor/src/main/java/com/linecorp/decaton/processor/processors/CompactionProcessor.java#L58
• One possible fix might be creating enclosing class for CompactionProcessor metrics and extends AbstractMetrics and close it upon processor close
• But rather, I fixed by clearing metrics in @Before to make testMetricsLifecycleManagement stable even we forget closing metrics in other classes

[kawamuray]

Thanks for catching this 👍

Besides the necessity of clearing metrics at the beginning, I think CompationProcessor should make sure to cleanup its metrics on its close()?

[ocadaruma]

@kawamuray Good point.

But I found that closing CompationProcessor metrics properly requires extra work, that is:

• Since CompactionProcessor's metrics are registered per every task to tag with subscriptionId, we can't use AbstractMetrics
• Possible solution is to make CompactionProcessor's constructor receives subscriptionId and pass it through ProcessorsBuilder
• But it causes breaking changes, and until then, master's test keeps failing.
• By the way, even CompactionProcessor's metrics are not cleaned up, it doesn't cause actual problem because it's tagged by subscriptionId (unlike other metrics (partition, thread,)), so the tag will still be valid even after rebalance

So, let's merge this PR as-is, and solve CompactionProcessor's metrics clean-up problem in another PR?