micrometer-api module split off from micrometer-core #2980

shakuzen · 2022-01-20T04:37:39Z

This moves a portion of existing classes in micrometer-core to a new micrometer-api module. micrometer-core depends on the API module and now micrometer-core contains only the instrumentation that we maintain.

Projects that want to write their own instrumentation should depend on the micrometer-api module. This will save some kilobytes in the JAR size for them.

End users may continue to depend on micrometer-core to get the instrumentation we provide in addition to the API.

Package names are changed, which will require users to update their imports. We could not keep the same package name without causing split packages across micrometer-api and micrometer-core.

This moves a portion of existing classes in micrometer-core to a new micrometer-api module. micrometer-core depends on the API module and now micrometer-core contains only the instrumentation that we maintain. Projects that want to write their own instrumentation now only need to depend on the micrometer-api module. This will save some kilobytes in the JAR size for them. End users may continue to depend on micrometer-core to get the instrumentation we provide in addition to the API. Package names are changed, which will require users to update their imports. We could not keep the same package name without causing split packages across micrometer-api and micrometer-core.

micrometer-api/src/main/java/io/micrometer/api/instrument/push/PushRegistryConfig.java

micrometer-api/src/main/java/io/micrometer/api/instrument/dropwizard/DropwizardCounter.java

micrometer-api/src/main/java/io/micrometer/api/instrument/dropwizard/DropwizardGauge.java

...meter-api/src/main/java/io/micrometer/api/instrument/dropwizard/DropwizardMeterRegistry.java

micrometer-api/src/main/java/io/micrometer/api/instrument/push/PushRegistryConfig.java

micrometer-api/src/main/java/io/micrometer/api/instrument/simple/SimpleConfig.java

micrometer-api/src/main/java/io/micrometer/api/instrument/util/MeterEquivalence.java

...ntations/micrometer-registry-atlas/src/main/java/io/micrometer/atlas/AtlasMeterRegistry.java

shakuzen

I know this is a huge changeset with mostly uninteresting noise (rename of package in imports), so I've commented on most files that weren't that.

shakuzen · 2022-01-20T05:51:24Z

...-boot2/src/main/java/io/micrometer/boot2/samples/components/HttpMetricsTagConfiguration.java

+    // This will fail to compile since published Spring Boot versions are based on Micrometer 1.x and
+    // we have changed the package for Micrometer API it uses.
+//    @Bean
+//    WebMvcTagsProvider webMvcTagsProvider() {


I had to comment out this stuff because of the package change to Tag but Spring Boot classes still reference the previous package. This really just exemplifies why we shouldn't have Spring Boot samples in this repo - it's a circular dependency.

shakuzen · 2022-01-20T05:55:08Z

...java/io/micrometer/boot2/reactive/samples/boot/autoconfig/PocMetricsWebClientCustomizer.java

- * Copy of {@link org.springframework.boot.actuate.metrics.web.reactive.client.MetricsWebClientCustomizer}.
- * Uses {@link PocMetricsWebClientFilterFunction} instead of {@link org.springframework.boot.actuate.metrics.web.reactive.client.MetricsWebClientFilterFunction}.
- */
-public class PocMetricsWebClientCustomizer implements WebClientCustomizer {


This was copied and modified classes from Spring Boot to demonstrate instrumenting with Timer.start instead of Timer.record. We don't need to maintain it here, and it has the compile problem of depending on Spring Boot that compiles against a version of Micrometer without the package changes made here.

shakuzen · 2022-01-20T05:58:50Z

micrometer-test/src/main/java/io/micrometer/api/instrument/cache/AbstractCacheMetricsTest.java


 /**
 * Base class for cache metrics tests.
 *
 * @author Oleksii Bondar
 */
-abstract class AbstractCacheMetricsTest {
+public abstract class AbstractCacheMetricsTest {


Moved to micrometer-test. I'm not sure we need this and the CacheMeterBinderCompatibilityKit, but I tried to limit this pull request to moving things.

shakuzen · 2022-01-20T06:07:59Z

micrometer-test/build.gradle

-    testImplementation 'com.github.ben-manes.caffeine:caffeine'
-    testImplementation 'net.sf.ehcache:ehcache'
-    testImplementation 'javax.cache:cache-api'
-    testImplementation 'com.hazelcast:hazelcast'


The specific cache metric implementations are in micrometer-core, so moved the tests there, eliminating the need to have these dependencies here also. I think it's better we have only the abstract test in micrometer-test.

micrometer-core/build.gradle

shakuzen · 2022-01-20T06:20:48Z

micrometer-api/src/test/java/io/micrometer/api/ipc/http/OkHttpSenderTests.java


 import java.net.SocketTimeoutException;
 import java.util.concurrent.TimeUnit;

 import static com.github.tomakehurst.wiremock.client.WireMock.*;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

-@ExtendWith(WiremockResolver.class)
+@WireMockTest


I switched to the built-in JUnit Jupiter support now in Wiremock, rather than using the separate dependency to reduce one test dependency needed in micrometer-api. Same in HttpUrlConnectionSenderTests

shakuzen · 2022-01-20T06:26:32Z

micrometer-api/src/test/java/io/micrometer/api/aop/TimedAspectTest.java

@@ -282,7 +283,7 @@ void timeMethodFailureWithLongTaskTimerWhenCompleted() {

        assertThatExceptionOfType(MeterNotFoundException.class).isThrownBy(() -> {
            failingRegistry.get("longCall")
-                    .tag("class", "io.micrometer.core.aop.TimedAspectTest$AsyncTimedService")
+                    .tag("class", this.getClass().getName() + "$AsyncTimedService")


IntelliJ did not refactor this class name and these tests were failing. So I manually refactored it to be more robust in preparation of the next moving of this test class.

shakuzen · 2022-01-20T06:29:32Z

micrometer-api/src/main/java/io/micrometer/api/instrument/transport/http/tags/package-info.java

 */
+package io.micrometer.api.instrument.transport.http.tags;


I fixed the location of the JavaDoc for this package-info. We could do the same for maintenance branches also.

shakuzen · 2022-01-20T06:35:42Z

micrometer-api/src/main/java/io/micrometer/api/instrument/internal/TimedExecutorService.java

 *
 * @author Jon Schneider
- * @see io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics


ExecutorServiceMetrics is in micrometer-core, but this class is in micrometer-api. Cannot resolve the reference. If we think this note is important, we can ignore the warning.

shakuzen · 2022-01-20T06:38:25Z

micrometer-api/build.gradle

+
+    // TODO move dropwizard out of api module?
+    // DropwizardMeterRegistry for e.g. JMX registry
+    optionalApi 'io.dropwizard.metrics:metrics-core'


Left as a potential future task to consider moving dropwizard to its own module to keep it out of micrometer-api.

While passing on the command line, our dependency injection tests for Guice were failing when running from IntelliJ for me. Upgrading to the latest version of Guice resolved this for me.

shakuzen · 2022-01-20T07:35:05Z

Registry implementations continue to depend on micrometer-core, but should probably depend on micrometer-api. There are some issues with that change in the current code, so I left it as something to consider separately from this change.

shakuzen · 2022-01-20T09:38:27Z

dependencies.gradle

        'io.prometheus:simpleclient_common:latest.release',
        'io.prometheus:simpleclient_pushgateway:latest.release',
+        // TODO support HttpTagsProvider variants based on Jakarta APIs?
+//        'jakarta.servlet:jakarta.servlet-api:latest.release',


In working on this, I noticed we have a javax servlet based http tags provider. We should probably also have jakarta-based one. Supporting either in the instrumentation may be a challenge. But something to work on separately from this PR.

shakuzen · 2022-01-20T09:45:44Z

On JAR size reduction due to this splitting,
micrometer-core 2.0.0.M1: 664 KB

With these changes,
micrometer-api: 393 KB
micrometer-core: 279 KB

kazuki43zoo · 2022-03-14T08:36:00Z

@shakuzen This change does not apply to main branch. Is this intentional?

shakuzen · 2022-03-15T04:49:45Z

@kazuki43zoo This change was eventually reverted in #3046 in favor of a micrometer-binder module (#3043), which will be part of today's release of 2.0.0-M3. Thanks for asking. It made me realize #3046 wasn't in the milestone, and didn't link to the related issues, which would have helped discoverability. We plan to mention this prominently in the release notes, but we should have left a comment on this pull request to make it easier for people to figure out what happened. Hopefully this clears up what happened.

shakuzen added enhancement A general enhancement release notes Noteworthy change to call out in the release notes module: micrometer-core An issue that is related to our core module labels Jan 20, 2022

shakuzen added this to the 2.0.0-M2 milestone Jan 20, 2022

shakuzen requested a review from jonatan-ivanov January 20, 2022 04:37