jlengrand/helidon
1
0
Fork 0
mirror of https://github.com/jlengrand/helidon.git synced 2026-03-10 08:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Brings our 1.1 and 2.0 impls in line with clarification of metric reuse (#1080)

Browse Source 
MP metrics reusability spec clarification: should apply only to annotations
This commit is contained in:
Tim Quinn
2019-10-04 09:40:04 -05:00
committed by GitHub
parent 73ae3ab671
commit 77ab6091c4
6 changed files with 94 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
metrics/metrics/src/main/java/io/helidon/metrics/Registry.java
View File

@@ -452,14 +452,6 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
.map(metric -> toType(metric, clazz));
}
private static <T extends MetricImpl> boolean enforceReusability(T metric, Metadata metadata) {
if (!(metric.isReusable() && metadata.isReusable())) {
throw new IllegalArgumentException("Attempting to re-register metric "
+ metric.getName() + " that is already registered with different reusability");
}
return true;
}
/**
* Returns an existing metric with the requested name, or if none is already
* registered registers a new metric using the name and type.
@@ -498,7 +490,6 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
Class<T> clazz) {
return getOptionalMetric(metadata.getName(), clazz)
.filter(metric -> enforceConsistentMetadata(metric, metadata))
.filter(metric -> enforceReusability(metric, metadata))
.orElseGet(() -> {
return registerMetric(metadata.getName(), metadata, metricFactory);
});

13
metrics2/metrics2/src/main/java/io/helidon/metrics/Registry.java
View File

@@ -579,8 +579,7 @@ public class Registry extends MetricRegistry implements io.helidon.common.metric
/**
* Returns an existing metric (if one is already registered with the name
* from the metadata plus the tags, and if the existing metadata is
* consistent with the new metadata, and if reuse is permitted) or a new
* metric, registered using the metadata and tags.
* consistent with the new metadata) or a new metric, registered using the metadata and tags.
*
* @param <T> type of the metric
* @param newMetadata metadata describing the metric
@@ -605,7 +604,6 @@ public class Registry extends MetricRegistry implements io.helidon.common.metric
*/
return getOptionalMetric(metricName, clazz, tags)
.filter(existingMetric -> enforceConsistentMetadata(existingMetric, newMetadata, tags))
.filter(existingMetric -> enforceReusability(existingMetric, newMetadata, tags))
.orElseGet(() -> {
final Metadata metadata = getOrRegisterMetadata(metricName, newMetadata, tags);
return registerMetric(metricName,
@@ -706,15 +704,6 @@ public class Registry extends MetricRegistry implements io.helidon.common.metric
return true;
}
private static <T extends HelidonMetric> boolean enforceReusability(T metric, Metadata metadata, Tag... tags) {
// We are here because a metric already exists during an attempt to register.
if (!metadata.isReusable()) {
throw new IllegalArgumentException("Attempting to re-register metric "
+ metric.getName() + " that is already registered as non-reusable");
}
return true;
}
private <T extends HelidonMetric, U extends HelidonMetric> U toType(T m1, Class<U> clazz) {
MetricType type1 = toType(m1);
MetricType type2 = toType(clazz);

11
metrics2/metrics2/src/test/java/io/helidon/metrics/RegistryTest.java
View File

@@ -108,15 +108,4 @@ public class RegistryTest {
() -> registry.counter(metadata2, tag1));
assertThat(ex.getMessage(), containsString("conflicts with"));
}
@Test
void testInvalidReregistration() {
Metadata metadata1 = new HelidonMetadata("counter6", "display name",
"description", MetricType.COUNTER, MetricUnits.NONE, false);
registry.counter(metadata1, tag1);
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class,
() -> registry.counter(metadata1,tag1));
assertThat(ex.getMessage(), containsString("re-register"));
assertThat(ex.getMessage(), containsString("already registered as non-reusable"));
}
}

51
microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricProducer.java
View File

@@ -16,6 +16,7 @@
package io.helidon.microprofile.metrics;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.util.Map;
import java.util.function.Function;
@@ -34,7 +35,10 @@ import org.eclipse.microprofile.metrics.MetricRegistry;
import org.eclipse.microprofile.metrics.MetricType;
import org.eclipse.microprofile.metrics.MetricUnits;
import org.eclipse.microprofile.metrics.Timer;
import org.eclipse.microprofile.metrics.annotation.Counted;
import org.eclipse.microprofile.metrics.annotation.Metered;
import org.eclipse.microprofile.metrics.annotation.Metric;
import org.eclipse.microprofile.metrics.annotation.Timed;
/**
* Class MetricProducer.
@@ -88,7 +92,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Counter produceCounter(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getCounters, registry::counter, Counter.class);
return produceMetric(registry, ip, Counted.class, registry::getCounters, registry::counter,
Counter.class);
}
@Produces
@@ -99,7 +104,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Meter produceMeter(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getMeters, registry::meter, Meter.class);
return produceMetric(registry, ip, Metered.class, registry::getMeters, registry::meter,
Meter.class);
}
@Produces
@@ -110,7 +116,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Timer produceTimer(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getTimers, registry::timer, Timer.class);
return produceMetric(registry, ip, Timed.class, registry::getTimers, registry::timer,
Timer.class);
}
@Produces
@@ -121,7 +128,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Histogram produceHistogram(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getHistograms, registry::histogram, Histogram.class);
return produceMetric(registry, ip, null, registry::getHistograms, registry::histogram,
Histogram.class);
}
/**
@@ -157,18 +165,43 @@ class MetricProducer {
return produceGauge(registry, ip);
}
private <T> T produceMetric(InjectionPoint ip, Supplier<Map<String, T>> getTypedMetricsFn,
private <T extends org.eclipse.microprofile.metrics.Metric, U extends Annotation> T produceMetric(MetricRegistry registry,
InjectionPoint ip, Class<U> annotationClass, Supplier<Map<String, T>> getTypedMetricsFn,
Function<Metadata, T> registerFn, Class<T> clazz) {
Metric metricAnno = ip.getAnnotated().getAnnotation(Metric.class);
T result = getTypedMetricsFn.get().get(getName(metricAnno, ip));
final String metricName = getName(metricAnno, ip);
T result = getTypedMetricsFn.get().get(metricName);
final Metadata newMetadata = newMetadata(ip, metricAnno, MetricType.from(clazz));
/*
* For an injection point, refer to a previously-registered metric that matches the name,
* if any. Register a new instance only if none is already present.
* If the injection point does not include the corresponding metric annotation which would
* declare the metric, then we do not need to enforce reuse restrictions because an @Inject
* or a @Metric by itself on an injection point is lookup-or-register.
*/
if (result != null) {
final Annotation specificMetricAnno = annotationClass == null ? null
: ip.getAnnotated().getAnnotation(annotationClass);
if (specificMetricAnno == null) {
return result;
}
final Metadata existingMetadata = registry.getMetadata().get(metricName);
enforceReusability(metricName, existingMetadata, newMetadata);
}
if (result == null) {
result = registerFn.apply(newMetadata(ip, metricAnno, MetricType.from(clazz)));
result = registerFn.apply(newMetadata);
}
return result;
}
private static void enforceReusability(String metricName, Metadata existingMetadata,
Metadata newMetadata) {
if (existingMetadata.isReusable() != newMetadata.isReusable()) {
throw new IllegalArgumentException("Attempt to reuse metric " + metricName
+ " with inconsistent isReusable setting");
}
if (!newMetadata.isReusable()) {
throw new IllegalArgumentException("Attempting to reuse metric "
+ metricName + " that is not reusable");
}
}
}

62
microprofile/metrics2/src/main/java/io/helidon/microprofile/metrics/MetricProducer.java
View File

@@ -16,6 +16,7 @@
package io.helidon.microprofile.metrics;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.List;
@@ -41,7 +42,10 @@ import org.eclipse.microprofile.metrics.MetricType;
import org.eclipse.microprofile.metrics.MetricUnits;
import org.eclipse.microprofile.metrics.Tag;
import org.eclipse.microprofile.metrics.Timer;
import org.eclipse.microprofile.metrics.annotation.Counted;
import org.eclipse.microprofile.metrics.annotation.Metered;
import org.eclipse.microprofile.metrics.annotation.Metric;
import org.eclipse.microprofile.metrics.annotation.Timed;
/**
* Class MetricProducer.
@@ -123,7 +127,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Counter produceCounter(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getCounters, registry::counter, Counter.class);
return produceMetric(registry, ip, Counted.class, registry::getCounters,
registry::counter, Counter.class);
}
@Produces
@@ -134,7 +139,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Meter produceMeter(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getMeters, registry::meter, Meter.class);
return produceMetric(registry, ip, Metered.class, registry::getMeters,
registry::meter, Meter.class);
}
@Produces
@@ -145,7 +151,7 @@ class MetricProducer {
@Produces
@VendorDefined
private Timer produceTimer(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getTimers, registry::timer, Timer.class);
return produceMetric(registry, ip, Timed.class, registry::getTimers, registry::timer, Timer.class);
}
@Produces
@@ -156,7 +162,8 @@ class MetricProducer {
@Produces
@VendorDefined
private Histogram produceHistogram(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getHistograms, registry::histogram, Histogram.class);
return produceMetric(registry, ip, null, registry::getHistograms,
registry::histogram, Histogram.class);
}
@Produces
@@ -167,7 +174,8 @@ class MetricProducer {
@Produces
@VendorDefined
private ConcurrentGauge produceConcurrentGauge(MetricRegistry registry, InjectionPoint ip) {
return produceMetric(ip, registry::getConcurrentGauges, registry::concurrentGauge, ConcurrentGauge.class);
return produceMetric(registry, ip, org.eclipse.microprofile.metrics.annotation.ConcurrentGauge.class,
registry::getConcurrentGauges, registry::concurrentGauge, ConcurrentGauge.class);
}
/**
@@ -204,28 +212,60 @@ class MetricProducer {
}
/**
* Returns an existing metric if one exists matching the injection point
* criteria, or if there is none registers and returns a new one using the
* caller-provided code.
* Returns an existing metric if one exists that matches the injection point
* criteria and is also reusable, or if there is none registers and returns a new one
* using the caller-provided function. If the caller refers to an existing metric that is
* not reusable then the method throws an {@code IllegalArgumentException}.
*
* @param <T> the type of the metric
* @param <U> the type of the annotation which marks a registration of the metric type
* @param registry metric registry to use
* @param ip the injection point
* @param annotationClass annotation which represents a declaration of a metric
* @param getTypedMetricsFn caller-provided factory for creating the correct
* type of metric (if there is no pre-existing one)
* @param registerFn caller-provided function for registering a newly-created metric
* @param clazz class for the metric type of interest
* @return the existing metric (if any), or the newly-created and registered one
*/
private <T> T produceMetric(InjectionPoint ip, Supplier<Map<MetricID, T>> getTypedMetricsFn,
private <T extends org.eclipse.microprofile.metrics.Metric, U extends Annotation> T produceMetric(MetricRegistry registry,
InjectionPoint ip, Class<U> annotationClass, Supplier<Map<MetricID, T>> getTypedMetricsFn,
BiFunction<Metadata, Tag[], T> registerFn, Class<T> clazz) {
final Metric metricAnno = ip.getAnnotated().getAnnotation(Metric.class);
final Tag[] tags = tags(metricAnno);
final MetricID metricID = new MetricID(getName(metricAnno, ip), tags);
T result = getTypedMetricsFn.get().get(metricID);
if (result == null) {
result = registerFn.apply(newMetadata(ip, metricAnno, MetricType.from(clazz)), tags);
final Metadata newMetadata = newMetadata(ip, metricAnno, MetricType.from(clazz));
/*
* If the injection point does not include the corresponding metric annotation which would
* declare the metric, then we do not need to enforce reuse restrictions because an @Inject
* or a @Metric by itself on an injection point is lookup-or-register.
*/
if (result != null) {
final Annotation specificMetricAnno = annotationClass == null ? null
: ip.getAnnotated().getAnnotation(annotationClass);
if (specificMetricAnno == null) {
return result;
}
final Metadata existingMetadata = registry.getMetadata().get(metricID.getName());
enforceReusability(metricID, existingMetadata, newMetadata);
} else {
result = registerFn.apply(newMetadata, tags);
}
return result;
}
private static void enforceReusability(MetricID metricID, Metadata existingMetadata,
Metadata newMetadata, Tag... tags) {
if (existingMetadata.isReusable() != newMetadata.isReusable()) {
throw new IllegalArgumentException("Attempt to reuse metric " + metricID
+ " with inconsistent isReusable setting");
}
if (!newMetadata.isReusable()) {
throw new IllegalArgumentException("Attempting to reuse metric "
+ metricID + " that is not reusable");
}
}
}

21
microprofile/metrics2/src/test/java/io/helidon/microprofile/metrics/ReusabilityMpServiceTest.java
View File

@@ -43,16 +43,6 @@ public class ReusabilityMpServiceTest {
.build();
}
@Test
public void tryToStartServerWithIllegalAnnotationReuse() throws Exception {
final DefinitionException ex = assertThrows(DefinitionException.class, () -> {
initServer(ResourceWithIllegallyReusedMetrics.class);;
});
assertThat(ex.getCause(), is(instanceOf(IllegalArgumentException.class)));
assertThat(ex.getCause().getMessage(), containsString("already registered"));
}
@Test
public void tryToStartServerWithLegalAnnotationReuse() throws Exception {
Server server = initServer(ResourceWithLegallyReusedMetrics.class);
@@ -62,15 +52,4 @@ public class ReusabilityMpServiceTest {
server.stop();
}
}
@Test
public void tryToStartServerWithMixedReuseAnnotations() throws Exception {
DefinitionException ex = assertThrows(DefinitionException.class, () -> {
initServer(ResourceWithMixedReusability.class);;
});
assertThat(ex.getCause(), is(instanceOf(IllegalArgumentException.class)));
assertThat(ex.getCause().getMessage(), containsString("already registered"));
}
}