Replace TimedApi with Micrometer's Timed annotation (apache#527)

This change replaces the custom `@TimedApi` annotation with Micrometer's `@Timed` annotation. This should facilitate future integration with Quarkus. For the Dropwizard runtime, this change should be a no-op.
d87zhang · Dec 11, 2024 · 6a71d8e · 6a71d8e
1 parent 9e828a5
commit 6a71d8e
Show file tree

Hide file tree

Showing 9 changed files with 29 additions and 66 deletions.
diff --git a/...izard/service/src/main/java/org/apache/polaris/service/dropwizard/PolarisApplication.java b/...izard/service/src/main/java/org/apache/polaris/service/dropwizard/PolarisApplication.java
@@ -89,7 +89,6 @@
 import org.apache.polaris.core.context.CallContext;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.context.RealmScoped;
-import org.apache.polaris.core.monitor.PolarisMetricRegistry;
 import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
 import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
 import org.apache.polaris.core.persistence.cache.EntityCache;
@@ -119,6 +118,7 @@
 import org.apache.polaris.service.dropwizard.config.PolarisApplicationConfig;
 import org.apache.polaris.service.dropwizard.context.RealmScopeContext;
 import org.apache.polaris.service.dropwizard.exception.JerseyViolationExceptionMapper;
+import org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry;
 import org.apache.polaris.service.dropwizard.persistence.cache.EntityCacheFactory;
 import org.apache.polaris.service.dropwizard.throttling.StreamReadConstraintsExceptionMapper;
 import org.apache.polaris.service.dropwizard.tracing.TracingFilter;

diff --git a/...ce/src/main/java/org/apache/polaris/service/dropwizard/TimedApplicationEventListener.java b/...ce/src/main/java/org/apache/polaris/service/dropwizard/TimedApplicationEventListener.java
@@ -18,28 +18,28 @@
  */
 package org.apache.polaris.service.dropwizard;
 
-import static org.apache.polaris.core.monitor.PolarisMetricRegistry.TAG_RESP_CODE;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.TAG_RESP_CODE;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Stopwatch;
+import io.micrometer.core.annotation.Timed;
 import io.micrometer.core.instrument.Tag;
 import jakarta.inject.Inject;
 import jakarta.ws.rs.ext.Provider;
 import java.lang.reflect.Method;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import org.apache.polaris.core.context.CallContext;
-import org.apache.polaris.core.monitor.PolarisMetricRegistry;
-import org.apache.polaris.core.resource.TimedApi;
+import org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry;
 import org.glassfish.jersey.server.monitoring.ApplicationEvent;
 import org.glassfish.jersey.server.monitoring.ApplicationEventListener;
 import org.glassfish.jersey.server.monitoring.RequestEvent;
 import org.glassfish.jersey.server.monitoring.RequestEventListener;
 
 /**
  * An ApplicationEventListener that supports timing and error counting of Jersey resource methods
- * annotated by {@link TimedApi}. It uses the {@link PolarisMetricRegistry} for metric collection
- * and properly times the resource on success and increments the error counter on failure.
+ * annotated by {@link Timed}. It uses the {@link PolarisMetricRegistry} for metric collection and
+ * properly times the resource on success and increments the error counter on failure.
  */
 @Provider
 public class TimedApplicationEventListener implements ApplicationEventListener {
@@ -89,8 +89,8 @@ public void onEvent(RequestEvent event) {
       if (event.getType() == RequestEvent.Type.REQUEST_MATCHED) {
         Method method =
             event.getUriInfo().getMatchedResourceMethod().getInvocable().getHandlingMethod();
-        if (method.isAnnotationPresent(TimedApi.class)) {
-          TimedApi timedApi = method.getAnnotation(TimedApi.class);
+        if (method.isAnnotationPresent(Timed.class)) {
+          Timed timedApi = method.getAnnotation(Timed.class);
           metric = timedApi.value();
 
           // Increment both the counter with the API name in the metric name and a common metric

diff --git a/...s/core/monitor/PolarisMetricRegistry.java → ...wizard/monitor/PolarisMetricRegistry.java b/...s/core/monitor/PolarisMetricRegistry.java → ...wizard/monitor/PolarisMetricRegistry.java
@@ -16,9 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.polaris.core.monitor;
+package org.apache.polaris.service.dropwizard.monitor;
 
 import com.google.common.annotations.VisibleForTesting;
+import io.micrometer.core.annotation.Timed;
 import io.micrometer.core.instrument.Counter;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tag;
@@ -34,7 +35,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
-import org.apache.polaris.core.resource.TimedApi;
 
 /**
  * Wrapper around the Micrometer {@link MeterRegistry} providing additional metric management
@@ -93,8 +93,8 @@ public void init(Class<?>... classes) {
     for (Class<?> clazz : classes) {
       Method[] methods = clazz.getDeclaredMethods();
       for (Method method : methods) {
-        if (method.isAnnotationPresent(TimedApi.class)) {
-          TimedApi timedApi = method.getAnnotation(TimedApi.class);
+        if (method.isAnnotationPresent(Timed.class)) {
+          Timed timedApi = method.getAnnotation(Timed.class);
           String metric = timedApi.value();
           timers.put(metric, Timer.builder(metric).register(meterRegistry));
           counters.put(

diff --git a/...d/service/src/test/java/org/apache/polaris/service/TimedApplicationEventListenerTest.java b/...d/service/src/test/java/org/apache/polaris/service/TimedApplicationEventListenerTest.java
@@ -18,27 +18,33 @@
  */
 package org.apache.polaris.service;
 
-import static org.apache.polaris.core.monitor.PolarisMetricRegistry.*;
 import static org.apache.polaris.service.context.DefaultContextResolver.REALM_PROPERTY_KEY;
 import static org.apache.polaris.service.dropwizard.TimedApplicationEventListener.SINGLETON_METRIC_NAME;
 import static org.apache.polaris.service.dropwizard.TimedApplicationEventListener.TAG_API_NAME;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.SUFFIX_COUNTER;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.SUFFIX_ERROR;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.SUFFIX_REALM;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.TAG_REALM;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.TAG_REALM_DEPRECATED;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.TAG_RESP_CODE;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.TAG_RESP_CODE_DEPRECATED;
 
 import io.dropwizard.testing.ConfigOverride;
 import io.dropwizard.testing.ResourceHelpers;
 import io.dropwizard.testing.junit5.DropwizardAppExtension;
 import io.dropwizard.testing.junit5.DropwizardExtensionsSupport;
+import io.micrometer.core.annotation.Timed;
 import io.micrometer.core.instrument.Tag;
 import jakarta.ws.rs.core.Response;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import org.apache.polaris.core.monitor.PolarisMetricRegistry;
-import org.apache.polaris.core.resource.TimedApi;
 import org.apache.polaris.service.admin.api.PolarisPrincipalsApi;
 import org.apache.polaris.service.dropwizard.PolarisApplication;
 import org.apache.polaris.service.dropwizard.TimedApplicationEventListener;
 import org.apache.polaris.service.dropwizard.config.PolarisApplicationConfig;
+import org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry;
 import org.apache.polaris.service.test.PolarisConnectionExtension;
 import org.apache.polaris.service.test.PolarisRealm;
 import org.apache.polaris.service.test.SnowmanCredentialsExtension;
@@ -74,7 +80,7 @@ public class TimedApplicationEventListenerTest {
           .filter(m -> m.getName().contains("getPrincipal"))
           .findFirst()
           .orElseThrow()
-          .getAnnotation(TimedApi.class)
+          .getAnnotation(Timed.class)
           .value();
 
   private static PolarisConnectionExtension.PolarisToken userToken;

diff --git a/...d/service/src/test/java/org/apache/polaris/service/ratelimiter/RateLimiterFilterTest.java b/...d/service/src/test/java/org/apache/polaris/service/ratelimiter/RateLimiterFilterTest.java
@@ -18,9 +18,10 @@
  */
 package org.apache.polaris.service.ratelimiter;
 
-import static org.apache.polaris.core.monitor.PolarisMetricRegistry.*;
 import static org.apache.polaris.service.dropwizard.TimedApplicationEventListener.SINGLETON_METRIC_NAME;
 import static org.apache.polaris.service.dropwizard.TimedApplicationEventListener.TAG_API_NAME;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.SUFFIX_ERROR;
+import static org.apache.polaris.service.dropwizard.monitor.PolarisMetricRegistry.TAG_RESP_CODE;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import io.dropwizard.testing.ConfigOverride;

diff --git a/polaris-core/build.gradle.kts b/polaris-core/build.gradle.kts
@@ -95,9 +95,6 @@ dependencies {
   implementation(platform(libs.google.cloud.storage.bom))
   implementation("com.google.cloud:google-cloud-storage")
 
-  implementation(platform(libs.micrometer.bom))
-  implementation("io.micrometer:micrometer-core")
-
   testFixturesApi(platform(libs.junit.bom))
   testFixturesApi("org.junit.jupiter:junit-jupiter")
   testFixturesApi(libs.assertj.core)

diff --git a/polaris-core/src/main/java/org/apache/polaris/core/resource/TimedApi.java b/polaris-core/src/main/java/org/apache/polaris/core/resource/TimedApi.java
diff --git a/server-templates/api.mustache b/server-templates/api.mustache
@@ -22,7 +22,7 @@ package {{package}};
 import {{import}};
 {{/imports}}
 
-import org.apache.polaris.core.resource.TimedApi;
+import io.micrometer.core.annotation.Timed;
 
 import java.util.Map;
 import java.util.List;
@@ -104,7 +104,7 @@ public class {{classname}}  {
   @Consumes({ {{#consumes}}"{{{mediaType}}}"{{^-last}}, {{/-last}}{{/consumes}} }){{/hasConsumes}}{{#hasProduces}}
   @Produces({ {{#produces}}"{{{mediaType}}}"{{^-last}}, {{/-last}}{{/produces}} }){{/hasProduces}}{{#hasAuthMethods}}
   {{#authMethods}}{{#isOAuth}}@RolesAllowed({ {{#scopes}}"{{scope}}"{{^-last}}, {{/-last}}{{/scopes}} }){{/isOAuth}}{{/authMethods}}{{/hasAuthMethods}}
-  @TimedApi("{{metricsPrefix}}.{{baseName}}.{{nickname}}")
+  @Timed("{{metricsPrefix}}.{{baseName}}.{{nickname}}")
   public Response {{nickname}}({{#isMultipart}}MultipartFormDataInput input,{{/isMultipart}}{{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{^isMultipart}}{{>formParams}},{{/isMultipart}}{{#isMultipart}}{{^isFormParam}},{{/isFormParam}}{{/isMultipart}}{{/allParams}}@Context SecurityContext securityContext) {
 {{! Don't log form or header params in case there are secrets, e.g., OAuth tokens }}
     LOGGER.atDebug().setMessage("Invoking {{baseName}} with params")

diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts
@@ -83,6 +83,9 @@ dependencies {
 
   implementation(platform(libs.azuresdk.bom))
   implementation("com.azure:azure-core")
+
+  compileOnly(platform(libs.micrometer.bom))
+  compileOnly("io.micrometer:micrometer-core")
 }
 
 openApiGenerate {