WavefrontMeterRegistry.close() does not remove threads leading to memory leak

[Brain-Crumbs]

Describe the bug
WavefrontMeterRegistry.close() does not remove threads related to WavefrontMeterRegistry.wavefrontSender (WavefrontClient is the implementation for the sender)

When constructing the WavefrontMeterRegistry three named threads are created:

1. waveferont-metrics-publisher (created in WavefrontMeterRegistry)
2. WavefrontClientSender (created in WavefrontClient when WavefrontMeterRegistry instanciates wavefrontSender)
3. sdk-metrics-registry (created in WavefrontSdkMetricsRegistry when instanciating WavefrontClient)

Calling WavefrontMeterRegistry.close() will close the following threads safely

1. waveferont-metrics-publisher

Calling wavefrontSender.close() will close the following threads safely

1. WavefrontClientSender
2. sdk-metrics-registry

It is my understanding that Calling WavefrontMeterRegistry.close() should close all threads related to this registry but it only closes one (waveferont-metrics-publisher) leaving the other two (WavefrontClientSender and sdk-metrics-registry)

I believe that WavefrontMeterRegistry should override the MeterRegistry/PushMeterRegistry.close() method similar to something like this:
@Override public void close() { wavefrontSender.close(); super.close(); }

Because the wavefrontSender is not accessible it is not possible to safely close all threads related to the WavefrontMeterRegistry which is leading to a memory leak. The way our application is setup Tomcat is not restarted between deployments, instead Tomcat Context configuration is used (https://tomcat.apache.org/tomcat-7.0-doc/config/context.html). So each time we deploy our application new threads are created without removing the existing ones. Over time these build up and lead to a more serious memory leak.

Environment
micrometer-registry-wavefront-1.6.4
micrometer-core-1.6.4
wavefront-sdk-java-2.6.2 (wavefront code)

To Reproduce
How to reproduce the bug:

1. Deploy application which uses metrics using tomcat context configuration
2. Confirm metrics are pushing via catalina logs and all threads described above are running.
3. Destroy tomcat context (undeploying application) calls WavefrontMeterRegistry.close()
4. The two threads related to WavefrontClient/WavefrontSender are still running

Expected behavior
All three threads described above related to WavefrontMeterRegistry should be closed.

[jonatan-ivanov]

Hi @Brain-Crumbs

Thanks for reporting this issue. Indeed the Wavefront registry does not close the Wavefront sender and it should. I fixed this in da20001

You are using an unsupported version of Micrometer, the oldest one we support is 1.8, this fix will be applied to 1.8, 1.9, 1.10 and above so you need to upgrade if you want to use it. You can try out our latest snapshot of these versions, the client should be closed in them.

If you can't or don't want to upgrade, as a workaround you can call close on the WavefrontSender after you closed the registry.

[Brain-Crumbs]

Hi jonatan,

thank you so much for getting to this quickly. We will update our dependencies to the latest version.

Much Appreciation!