Fix incorrect handling of omitted display name (and description) revealed by gRPC (helidon-io#3178)

Author: tjquinno

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

@@ -563,7 +563,7 @@ static class MetricsRules {
          *
          * @see org.eclipse.microprofile.metrics.Metadata
          */
-        private String displayName;
+        private Optional<String> displayName = Optional.empty();
 
         /**
          * The unit of the metric.
@@ -623,9 +623,7 @@ org.eclipse.microprofile.metrics.Metadata metadata(ServiceDescriptor service, St
 
             this.description.ifPresent(builder::withDescription);
             this.units.ifPresent(builder::withUnit);
-
-            String displayName = this.displayName;
-            builder.withDisplayName(displayName == null ? name : displayName);
+            this.displayName.ifPresent(builder::withDisplayName);
 
             return builder.build();
         }
@@ -648,7 +646,7 @@ private MetricsRules description(String description) {
 
         private MetricsRules displayName(String displayName) {
             MetricsRules rules = new MetricsRules(this);
-            rules.displayName = displayName;
+            rules.displayName = Optional.of(displayName);
             return rules;
         }
 

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

@@ -229,8 +229,15 @@ private void addMetric(ServiceDescriptor.Builder builder,
 
             MetricAnnotationInfo<?> mInfo = METRIC_ANNOTATION_INFO.get(annotation.annotationType());
             if (mInfo != null && mInfo.annotationClass.isInstance(annotation)) {
-                interceptor = interceptor.description(mInfo.description(annotatedMethod))
-                        .displayName(mInfo.displayName(annotatedMethod))
+                String candidateDescription = mInfo.description(annotatedMethod);
+                if (candidateDescription != null && !candidateDescription.trim().isEmpty()) {
+                    interceptor = interceptor.description(candidateDescription.trim());
+                }
+                String candidateDisplayName = mInfo.displayName(annotatedMethod);
+                if (candidateDisplayName != null && !candidateDisplayName.trim().isEmpty()) {
+                    interceptor = interceptor.displayName(candidateDisplayName.trim());
+                }
+                interceptor = interceptor
                         .reusable(mInfo.reusable(annotatedMethod))
                         .units(mInfo.units(annotatedMethod));
             }

microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricAnnotationInfo.java

@@ -27,6 +27,7 @@
 import io.helidon.microprofile.metrics.MetricUtil.MatchingType;
 
 import org.eclipse.microprofile.metrics.Metadata;
+import org.eclipse.microprofile.metrics.MetadataBuilder;
 import org.eclipse.microprofile.metrics.Metric;
 import org.eclipse.microprofile.metrics.MetricRegistry;
 import org.eclipse.microprofile.metrics.MetricType;
@@ -68,18 +69,22 @@ RegistrationPrep<?> create(A annotation,
 
             String metricName = MetricUtil.getMetricName(annotatedElement, clazz, matchingType, info.name(annotation),
                     info.absolute(annotation));
-            String candidateDisplayName = info.displayName(annotation);
-            Metadata metadata = Metadata.builder()
+            MetadataBuilder metadataBuilder = Metadata.builder()
                     .withName(metricName)
-                    .withDisplayName(candidateDisplayName.isEmpty() ? metricName : candidateDisplayName)
-                    .withDescription(info.description(annotation)
-                            .trim())
                     .withType(ANNOTATION_TYPE_TO_METRIC_TYPE.get(annotation.annotationType()))
                     .withUnit(info.unit(annotation)
                             .trim())
-                    .reusable(info.reusable(annotation))
-                    .build();
-            return new RegistrationPrep<>(metricName, metadata, info.tags(annotation), info.registerFunction);
+                    .reusable(info.reusable(annotation));
+
+            String candidateDescription = info.description(annotation);
+            if (candidateDescription != null && !candidateDescription.trim().isEmpty()) {
+                metadataBuilder.withDescription(candidateDescription.trim());
+            }
+            String candidateDisplayName = info.displayName(annotation);
+            if (candidateDisplayName != null && !candidateDisplayName.trim().isEmpty()) {
+                metadataBuilder.withDisplayName(candidateDisplayName.trim());
+            }
+            return new RegistrationPrep<>(metricName, metadataBuilder.build(), info.tags(annotation), info.registerFunction);
         }
 
         private RegistrationPrep(String metricName, Metadata metadata, Tag[] tags, Registration<T> registration) {

microprofile/metrics/src/test/java/io/helidon/microprofile/metrics/MetricsTest.java

@@ -35,8 +35,11 @@
 
 import org.eclipse.microprofile.metrics.Counter;
 import org.eclipse.microprofile.metrics.Gauge;
+import org.eclipse.microprofile.metrics.Metadata;
 import org.eclipse.microprofile.metrics.Meter;
 import org.eclipse.microprofile.metrics.MetricID;
+import org.eclipse.microprofile.metrics.MetricType;
+import org.eclipse.microprofile.metrics.MetricUnits;
 import org.eclipse.microprofile.metrics.SimpleTimer;
 import org.eclipse.microprofile.metrics.Timer;
 import org.hamcrest.CoreMatchers;
@@ -203,4 +206,24 @@ public void testAbsoluteGaugeBeanName() {
                 .keySet().stream().map(MetricID::getName).collect(Collectors.toSet());
         assertThat(gauges, CoreMatchers.hasItem("secondsSinceBeginningOfTime"));
     }
+
+    @Test
+    void testOmittedDisplayName() {
+        MeteredBean bean = newBean(MeteredBean.class);
+        String metricName = MeteredBean.class.getName() + ".method1";
+        Metadata metadata = getMetricRegistry().getMetadata().get(metricName);
+        assertThat("Metadata for meter of annotated method", metadata, is(notNullValue()));
+
+        // The displayName value stored in the retrieved metadata should be null, but Metadata.getDisplayName() returns the name
+        // in those cases. So an easy way to check is to attempt to re-register/look up the meter using a new Metadata instance
+        // for which we know the displayName is null.
+        Metadata newMetadata = Metadata.builder()
+                .withName(metadata.getName())
+                .withType(MetricType.METERED)
+                .reusable(false)
+                .withUnit(MetricUnits.PER_SECOND)
+                .build();
+        // Should return the existing meter. Throws exception if metadata is mismatched.
+        Meter meter = getMetricRegistry().meter(newMetadata);
+    }
 }