Add missing metrics coverage and ensure proper coverage of future metrics in gRPC

grpc/metrics/src/main/java/io/helidon/grpc/metrics/GrpcMetrics.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2021 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 package io.helidon.grpc.metrics;
 
+import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -36,13 +37,15 @@
 import io.grpc.ServerCallHandler;
 import io.grpc.ServerInterceptor;
 import io.grpc.Status;
+import org.eclipse.microprofile.metrics.ConcurrentGauge;
 import org.eclipse.microprofile.metrics.Counter;
 import org.eclipse.microprofile.metrics.Histogram;
 import org.eclipse.microprofile.metrics.MetadataBuilder;
 import org.eclipse.microprofile.metrics.Meter;
 import org.eclipse.microprofile.metrics.MetricRegistry;
 import org.eclipse.microprofile.metrics.MetricType;
 import org.eclipse.microprofile.metrics.MetricUnits;
+import org.eclipse.microprofile.metrics.SimpleTimer;
 import org.eclipse.microprofile.metrics.Tag;
 import org.eclipse.microprofile.metrics.Timer;
 
@@ -223,6 +226,26 @@ public static GrpcMetrics timed() {
         return new GrpcMetrics(new MetricsRules(MetricType.TIMER));
     }
 
+    /**
+     * A static factory method to create a {@link GrpcMetrics} instance
+     * to time gRPC method calls.
+     *
+     * @return a {@link GrpcMetrics} instance to time gRPC method calls
+     */
+    public static GrpcMetrics concurrentGauge() {
+        return new GrpcMetrics(new MetricsRules(MetricType.CONCURRENT_GAUGE));
+    }
+
+    /**
+     * A static factory method to create a {@link GrpcMetrics} instance
+     * to time gRPC method calls.
+     *
+     * @return a {@link GrpcMetrics} instance to time gRPC method calls
+     */
+    public static GrpcMetrics simplyTimed() {
+        return new GrpcMetrics(new MetricsRules(MetricType.SIMPLE_TIMER));
+    }
+
     @Override
     public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call,
                                                                  Metadata headers,
@@ -253,6 +276,14 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(ServerCall<ReqT, Re
                 serverCall = new TimedServerCall<>(APP_REGISTRY.timer(
                         rules.metadata(service, methodName), rules.toTags()), call);
                 break;
+            case SIMPLE_TIMER:
+                serverCall = new SimplyTimedServerCall<>(APP_REGISTRY.simpleTimer(
+                        rules.metadata(service, methodName), rules.toTags()), call);
+                break;
+            case CONCURRENT_GAUGE:
+                serverCall = new ConcurrentGaugeServerCall<>(APP_REGISTRY.concurrentGauge(
+                        rules.metadata(service, methodName), rules.toTags()), call);
+                break;
             case GAUGE:
             case INVALID:
             default:
@@ -332,6 +363,65 @@ public void close(Status status, Metadata responseHeaders) {
         }
     }
 
+    /**
+     * A {@link GrpcMetrics.MeteredServerCall} that captures call times.
+     *
+     * @param <ReqT>   the call request type
+     * @param <RespT>  the call response type
+     */
+    private class SimplyTimedServerCall<ReqT, RespT>
+            extends MetricServerCall<ReqT, RespT, SimpleTimer> {
+        /**
+         * The method start time.
+         */
+        private final long startNanos;
+
+        /**
+         * Create a {@link SimplyTimedServerCall}.
+         *
+         * @param delegate the call to time
+         */
+        private SimplyTimedServerCall(SimpleTimer simpleTimer, ServerCall<ReqT, RespT> delegate) {
+            super(simpleTimer, delegate);
+
+            this.startNanos = System.nanoTime();
+        }
+
+        @Override
+        public void close(Status status, Metadata responseHeaders) {
+            super.close(status, responseHeaders);
+
+            long time = System.nanoTime() - startNanos;
+            getMetric().update(Duration.ofNanos(time));
+        }
+    }
+
+    /**
+     * A {@link GrpcMetrics.MeteredServerCall} that captures call times.
+     *
+     * @param <ReqT>   the call request type
+     * @param <RespT>  the call response type
+     */
+    private class ConcurrentGaugeServerCall<ReqT, RespT>
+            extends MetricServerCall<ReqT, RespT, ConcurrentGauge> {
+
+        /**
+         * Create a {@link SimplyTimedServerCall}.
+         *
+         * @param delegate the call to time
+         */
+        private ConcurrentGaugeServerCall(ConcurrentGauge concurrentGauge, ServerCall<ReqT, RespT> delegate) {
+            super(concurrentGauge, delegate);
+        }
+
+        @Override
+        public void close(Status status, Metadata responseHeaders) {
+            super.close(status, responseHeaders);
+
+            getMetric().inc();
+        }
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {

grpc/metrics/src/test/java/io/helidon/grpc/metrics/GrpcMetricsInterceptorIT.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2021 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,13 +33,15 @@
 import java.util.HashMap;
 import java.util.stream.Collectors;
 
+import org.eclipse.microprofile.metrics.ConcurrentGauge;
 import org.eclipse.microprofile.metrics.Counter;
 import org.eclipse.microprofile.metrics.Histogram;
 import org.eclipse.microprofile.metrics.Meter;
 import org.eclipse.microprofile.metrics.Metric;
 import org.eclipse.microprofile.metrics.MetricID;
 import org.eclipse.microprofile.metrics.MetricRegistry;
 import org.eclipse.microprofile.metrics.MetricUnits;
+import org.eclipse.microprofile.metrics.SimpleTimer;
 import org.eclipse.microprofile.metrics.Timer;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -169,6 +171,44 @@ public void shouldUseTimerMetric() throws Exception {
         assertThat(appTimer.getCount(), is(1L));
     }
 
+    @Test
+    public void shouldUseSimpleTimerMetric() throws Exception {
+        ServiceDescriptor descriptor = ServiceDescriptor.builder(createMockService())
+                .unary("barSimplyTimed", this::dummyUnary)
+                .build();
+
+        MethodDescriptor methodDescriptor = descriptor.method("barSimplyTimed");
+        GrpcMetrics metrics = GrpcMetrics.simplyTimed();
+
+        ServerCall<String, String> call = call(metrics, methodDescriptor);
+
+        call.close(Status.OK, new Metadata());
+
+        SimpleTimer appSimpleTimer = appRegistry.simpleTimer("Foo.barSimplyTimed");
+
+        assertVendorMetrics();
+        assertThat(appSimpleTimer.getCount(), is(1L));
+    }
+
+    @Test
+    public void shouldUseConcurrentGaugeMetric() throws Exception {
+        ServiceDescriptor descriptor = ServiceDescriptor.builder(createMockService())
+                .unary("barConcurrentGauge", this::dummyUnary)
+                .build();
+
+        MethodDescriptor methodDescriptor = descriptor.method("barConcurrentGauge");
+        GrpcMetrics metrics = GrpcMetrics.concurrentGauge();
+
+        ServerCall<String, String> call = call(metrics, methodDescriptor);
+
+        call.close(Status.OK, new Metadata());
+
+        ConcurrentGauge appConcurrentGauge = appRegistry.concurrentGauge("Foo.barConcurrentGauge");
+
+        assertVendorMetrics();
+        assertThat(appConcurrentGauge.getCount(), is(1L));
+    }
+
     @Test
     public void shouldApplyTags() throws Exception {
         ServiceDescriptor descriptor = ServiceDescriptor.builder(createMockService())

microprofile/grpc/metrics/README.md

@@ -0,0 +1,104 @@
+# gRPC Metrics
+
+## Adding support for a new metric type
+From time to time, the metrics spec evolves and new metrics appear. 
+To add support for a new metric to gRPC, follow these steps.
+
+### Update `GrpcMetrics`
+#### Add `xxx` metric factory method
+The class contains a very simple method for each metric type like this:
+```java
+public static GrpcMetrics concurrentGauge() {...}
+```
+Just add a new method following the same pattern for the new metric type.
+
+#### Add `xxxServerCall` method
+The class also contains a method per metrics like this:
+```java
+private class ConcurrentGaugeServerCall<ReqT, RespT>
+            extends MetricServerCall<ReqT, RespT, ConcurrentGauge> {...}
+```
+Add a new method following the same pattern for the new metric type. The new method will need to 
+update its metric in a way that varies among the different metrics types. 
+
+#### Update the `interceptCall` method
+Add a new `case` for the new metric to the `switch` statement. 
+
+### Update `GrpcMetricsInterceptorIT`
+Add a test for the new metric:
+```java
+public void shouldUseConcurrentGaugeMetric() throws Exception {...}
+```
+Follow the pattern set by the methods for other metrics, changing the names and using a 
+metric-specific check to make sure the metric was correctly updated.
+### Update `MetricsConfigurer`
+Update the map near the top of the file to add the new metric annotation. 
+The rest of the logic uses the map rather than hard-coded methods.
+
+### Update `GrpcMetricsCdiExtension`
+
+Add the new metric and its corresponding annotation to the `METRICS_ANNOTATIONS` `EnumMap` 
+initialization.
+
+On the `registerMetrics` method, add the new metrics annotation(s) to the `@WithAnnotation` list of 
+metrics 
+annotations.
+
+### Update `MetricsConfigurerTest`
+#### Update `ServiceOne` inner class
+Add an empty method for the new metric type.
+```java
+@Unary
+@ConcurrentGauge
+public void concurrentGauge(String request, StreamObserver<String> response) {}
+```
+
+#### Update `ServiceTwo` interface
+Add a method for the new metric type:
+```java
+@Unary
+@ConcurrentGauge
+void concurrentGauge(String request, StreamObserver<String> response);
+```
+
+#### Update `ServiceTwoImpl` class
+Add a method for the new metric type:
+```java
+@Override
+public void concurrentGauge(String request, StreamObserver<String> response) {
+}
+```
+
+Add an invocation of the new method to the `update` method:
+```java
+    ...
+    .unary("concurrentGauge", this::concurrentGauge);
+```
+#### Add tests
+##### Add a test to make sure the metric is updated
+
+Follow the pattern:
+```java
+@Test
+public void shouldAddConcurrentGaugeMetricFromClassAnnotation() {...}
+```
+##### Add a test to make sure the new metric annotation on the interface is ignored
+
+Follow the pattern:
+```java
+@Test
+public void shouldIgnoreConcurrentGaugeMetricFromInterfaceAnnotation {...}
+```
+### Add `CoverageTestBeanXXX` test class
+where `XXX` is the simple name of the new metrics annotation (e.g., `Counted`).
+Use one of the existing classes as a pattern, removing the existing metrics annotation from the 
+method and adding the new metrics annotation in its place.
+
+## Testing
+To help make sure all known metrics annotations (from the `microprofile/metrics` artifact) are 
+handled, a test-time CDI extension finds all the coverage test beans and adds them to CDI. 
+It also examines those classes during the normal CDI extension life cycle to make sure the 
+actual gRPC extension processes them as expected.
+
+Several tests make sure that all the metrics annotations are represented and processed, listing any annotations that were not processed as expected.
+

microprofile/grpc/metrics/pom.xml

@@ -90,6 +90,11 @@
             <artifactId>jersey-client</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.microprofile.tests</groupId>
+            <artifactId>helidon-microprofile-tests-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

microprofile/grpc/metrics/src/main/java/io/helidon/microprofile/grpc/metrics/GrpcMetricsCdiExtension.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2021 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,8 +17,8 @@
 package io.helidon.microprofile.grpc.metrics;
 
 import java.lang.annotation.Annotation;
-import java.util.Arrays;
-import java.util.List;
+import java.util.EnumMap;
+import java.util.Map;
 
 import javax.annotation.Priority;
 import javax.enterprise.event.Observes;
@@ -32,8 +32,11 @@
 import io.helidon.microprofile.grpc.core.Grpc;
 import io.helidon.microprofile.grpc.core.GrpcMethod;
 
+import org.eclipse.microprofile.metrics.MetricType;
+import org.eclipse.microprofile.metrics.annotation.ConcurrentGauge;
 import org.eclipse.microprofile.metrics.annotation.Counted;
 import org.eclipse.microprofile.metrics.annotation.Metered;
+import org.eclipse.microprofile.metrics.annotation.SimplyTimed;
 import org.eclipse.microprofile.metrics.annotation.Timed;
 
 /**
@@ -50,11 +53,21 @@
 public class GrpcMetricsCdiExtension
         implements Extension {
 
-    /**
-     * The supported metrics annotations.
-     */
-    private static final List<Class<? extends Annotation>> METRIC_ANNOTATIONS
-            = Arrays.asList(Counted.class, Timed.class, Metered.class);
+    static final int OBSERVER_PRIORITY = Interceptor.Priority.APPLICATION;
+
+    static final EnumMap<MetricType, Class<? extends Annotation>> METRICS_ANNOTATIONS;
+
+    static {
+        Map<MetricType, Class<? extends Annotation>> map = Map.of(
+                MetricType.CONCURRENT_GAUGE, ConcurrentGauge.class,
+                MetricType.COUNTER, Counted.class,
+                MetricType.METERED, Metered.class,
+                MetricType.SIMPLE_TIMER, SimplyTimed.class,
+                MetricType.TIMER, Timed.class
+        );
+        METRICS_ANNOTATIONS = new EnumMap<>(map);
+    }
+
 
     /**
      * Observer {@link ProcessAnnotatedType} events and process any method
@@ -63,11 +76,11 @@ public class GrpcMetricsCdiExtension
      * @param pat  the {@link ProcessAnnotatedType} to observer
      */
     private void registerMetrics(@Observes
-                                 @WithAnnotations({Counted.class, Timed.class, Metered.class, Grpc.class})
-                                 @Priority(Interceptor.Priority.APPLICATION)
+                                 @WithAnnotations({Counted.class, Timed.class, Metered.class, ConcurrentGauge.class,
+                                         SimplyTimed.class, Grpc.class})
+                                 @Priority(OBSERVER_PRIORITY)
                                  ProcessAnnotatedType<?> pat) {
-
-        METRIC_ANNOTATIONS.forEach(type ->
+        METRICS_ANNOTATIONS.values().forEach(type ->
                pat.configureAnnotatedType()
                   .methods()
                   .stream()