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

Fully enforce reusability in MP Metrics 1.1. support (#1048)

Browse Source 
* Fix for incomplete reusability enforcement in MP Metrics 1.1. support
This commit is contained in:
Tim Quinn
2019-09-23 16:09:49 -05:00
committed by GitHub
parent 16a0ae048b
commit 719aa163f0
9 changed files with 217 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
metrics/metrics/src/main/java/io/helidon/metrics/InternalMetadataBuilderImpl.java
View File

@@ -104,7 +104,7 @@ class InternalMetadataBuilderImpl implements InternalBridge.Metadata.MetadataBui
if (name == null) {
throw new IllegalStateException("name must be assigned");
}
return new InternalMetadataImpl(name, displayName, description, type, unit);
return new InternalMetadataImpl(name, displayName, description, type, unit, reusable, null);
}
static class FactoryImpl implements Metadata.MetadataBuilder.Factory {

15
metrics/metrics/src/main/java/io/helidon/metrics/MetricsSupport.java
View File

@@ -269,25 +269,25 @@ public final class MetricsSupport implements Service {
String metricPrefix = (null == routingName ? "" : routingName + ".") + "requests.";
Registry vendor = rf.getARegistry(MetricRegistry.Type.VENDOR);
Counter totalCount = vendor.counter(new Metadata(metricPrefix + "count",
Counter totalCount = vendor.counter(reusableMetadata(metricPrefix + "count",
"Total number of HTTP requests",
"Each request (regardless of HTTP method) will increase this counter",
MetricType.COUNTER,
MetricUnits.NONE));
Meter totalMeter = vendor.meter(new Metadata(metricPrefix + "meter",
Meter totalMeter = vendor.meter(reusableMetadata(metricPrefix + "meter",
"Meter for overall HTTP requests",
"Each request will mark the meter to see overall throughput",
MetricType.METERED,
MetricUnits.NONE));
vendor.counter(new Metadata("grpc.requests.count",
vendor.counter(reusableMetadata("grpc.requests.count",
"Total number of gRPC requests",
"Each gRPC request (regardless of the method) will increase this counter",
MetricType.COUNTER,
MetricUnits.NONE));
vendor.meter(new Metadata("grpc.requests.meter",
vendor.meter(reusableMetadata("grpc.requests.meter",
"Meter for overall gRPC requests",
"Each gRPC request will mark the meter to see overall throughput",
MetricType.METERED,
@@ -353,6 +353,13 @@ public final class MetricsSupport implements Service {
configureEndpoint(rules);
}
private Metadata reusableMetadata(String name, String displayName, String description,
MetricType type, String units) {
Metadata result = new Metadata(name, displayName, description, type, units);
result.setReusable(true);
return result;
}
private void getOne(ServerRequest req, ServerResponse res, Registry registry) {
String metricName = req.path().param("metric");

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

@@ -20,13 +20,14 @@ import java.util.AbstractMap;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.function.BiFunction;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -49,6 +50,9 @@ import org.eclipse.microprofile.metrics.Timer;
* Metrics registry.
*/
class Registry extends MetricRegistry implements io.helidon.common.metrics.InternalBridge.MetricRegistry {
private static final Map<Class<? extends HelidonMetric>, MetricType> METRIC_TO_TYPE_MAP = prepareMetricToTypeMap();
private final Type type;
private final Map<String, MetricImpl> allMetrics = new ConcurrentHashMap<>();
@@ -65,9 +69,11 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
}
static Metadata toMetadata(io.helidon.common.metrics.InternalBridge.Metadata metadata, Map<String, String> tags) {
return new Metadata(metadata.getName(), metadata.getDisplayName(),
Metadata result = new Metadata(metadata.getName(), metadata.getDisplayName(),
metadata.getDescription().orElse(null), metadata.getTypeRaw(),
metadata.getUnit().orElse(null), tagsAsString(tags));
result.setReusable(metadata.isReusable());
return result;
}
Optional<HelidonMetric> getMetric(String metricName) {
@@ -114,12 +120,12 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
@Override
public Counter counter(String name) {
return counter(new Metadata(name, MetricType.COUNTER));
return getOrRegisterMetric(name, HelidonCounter::create, HelidonCounter.class);
}
@Override
public Counter counter(Metadata metadata) {
return getMetric(metadata, Counter.class, (name) -> HelidonCounter.create(type.getName(), metadata));
return getOrRegisterMetric(metadata, HelidonCounter::create, HelidonCounter.class);
}
@Override
@@ -134,12 +140,12 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
@Override
public Histogram histogram(String name) {
return histogram(new Metadata(name, MetricType.HISTOGRAM));
return getOrRegisterMetric(name, HelidonHistogram::create, HelidonHistogram.class);
}
@Override
public Histogram histogram(Metadata metadata) {
return getMetric(metadata, Histogram.class, (name) -> HelidonHistogram.create(type.getName(), metadata));
return getOrRegisterMetric(metadata, HelidonHistogram::create, HelidonHistogram.class);
}
@Override
@@ -154,12 +160,12 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
@Override
public Meter meter(String name) {
return meter(new Metadata(name, MetricType.METERED));
return getOrRegisterMetric(name, HelidonMeter::create, HelidonMeter.class);
}
@Override
public Meter meter(Metadata metadata) {
return getMetric(metadata, Meter.class, (name) -> HelidonMeter.create(type.getName(), metadata));
return getOrRegisterMetric(metadata, HelidonMeter::create, HelidonMeter.class);
}
@Override
@@ -174,12 +180,12 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
@Override
public Timer timer(String name) {
return timer(new Metadata(name, MetricType.TIMER));
return getOrRegisterMetric(name, HelidonTimer::create, HelidonTimer.class);
}
@Override
public Timer timer(Metadata metadata) {
return getMetric(metadata, Timer.class, (name) -> HelidonTimer.create(type.getName(), metadata));
return getOrRegisterMetric(metadata, HelidonTimer::create, HelidonTimer.class);
}
@Override
@@ -409,32 +415,139 @@ class Registry extends MetricRegistry implements io.helidon.common.metrics.Inter
return new TreeMap<>(collected);
}
private <T extends Metric, I extends MetricImpl> T getMetric(Metadata metadata,
Class<T> type,
Function<String, I> newInstanceCreator) {
MetricImpl metric = allMetrics.get(metadata.getName());
if (metric != null) {
if (metric.isReusable() != metadata.isReusable()) {
throw new IllegalArgumentException("Metadata not re-usable for metric " + metadata.getName());
}
} else {
metric = newInstanceCreator.apply(metadata.getName());
metric.setReusable(metadata.isReusable());
allMetrics.put(metadata.getName(), metric);
}
if (!(type.isAssignableFrom(metric.getClass()))) {
throw new IllegalArgumentException("Attempting to get " + metadata.getType()
+ ", but metric registered under this name is "
+ metric);
}
return type.cast(metric);
}
static String tagsAsString(Map<String, String> tags) {
return tags.entrySet().stream()
.map(entry -> String.format("%s=%s", entry.getKey(), entry.getValue()))
.collect(Collectors.joining(","));
}
static <T extends Metadata, U extends Metadata> boolean metadataMatches(T a, U b) {
if ((a == null && b == null) || (a == b)) {
return true;
}
if (a == null || b == null) {
return false;
}
return a.getName().equals(b.getName())
&& a.getTypeRaw().equals(b.getTypeRaw())
&& Objects.equals(a.getUnit(), b.getUnit())
&& a.getTags().equals(b.getTags())
&& (a.isReusable() == b.isReusable());
}
private static <T extends MetricImpl> T enforceConsistentMetadata(T metric, Metadata metadata) {
// Check that metadata is compatible.
if (!metadataMatches(metric, metadata)) {
throw new IllegalArgumentException("New metric " + metric.getName()
+ " with metadata " + metric
+ " conflicts with a metric already registered with metadata "
+ metadata);
}
return metric;
}
<T extends HelidonMetric> Optional<T> getOptionalMetric(String metricName, Class<T> clazz) {
return Optional.ofNullable(allMetrics.get(metricName))
.map(metric -> toType(metric, clazz));
}
private static <T extends MetricImpl> T 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 metric;
}
/**
* Returns an existing metric with the requested name, or if none is already
* registered registers a new metric using the name and type.
*
* @param <T> type of the metric
* @param metricName name of the metric
* @param metricFactory method to create a new instance of the metric type
* @param clazz class of the metric to find or create
* @return the existing metric (if any) or a newly-registered one
*/
private <T extends MetricImpl> T getOrRegisterMetric(String metricName,
BiFunction<String, Metadata, T> metricFactory,
Class<T> clazz) {
final T metric = getOptionalMetric(metricName, clazz)
.orElseGet(() -> {
return registerMetric(metricName,
new Metadata(metricName, MetricType.from(clazz)), metricFactory);
});
return metric;
}
/**
* Returns an existing metric if it matches the provided metadata, or if no
* same-named metric exists registers and returns a new metric using the metadata.
*
* @param <T> type of the metric
* @param metadata metadata describing the metric
* @param metricFactory method to create a new instance of the metric type
* @param clazz class of the metric to find or create
* @return the existing metric (if matching the metadata) or a newly-registered one
*/
private <T extends MetricImpl> T getOrRegisterMetric(Metadata metadata,
BiFunction<String, Metadata, T> metricFactory,
Class<T> clazz) {
return getOptionalMetric(metadata.getName(), clazz)
.map(metric -> enforceConsistentMetadata(metric, metadata))
.map(metric -> enforceReusability(metric, metadata))
.orElseGet(() -> {
return registerMetric(metadata.getName(), metadata, metricFactory);
});
}
private <T extends MetricImpl> T registerMetric(String metricName, Metadata metadata,
BiFunction<String, Metadata, T> metricFactory) {
T newMetric = metricFactory.apply(type.getName(), metadata);
allMetrics.put(metricName, newMetric);
return newMetric;
}
private static <T extends HelidonMetric, U extends HelidonMetric> U toType(T m1, Class<U> clazz) {
MetricType type1 = toType(m1);
MetricType type2 = toType(clazz);
if (type1 == type2) {
return clazz.cast(m1);
}
throw new IllegalArgumentException("Metric types " + type1.toString()
+ " and " + type2.toString() + " do not match");
}
private static MetricType toType(Metric metric) {
// Find subtype of Metric, needed for user-defined metrics
Class<?> clazz = metric.getClass();
do {
Optional<Class<?>> optionalClass = Arrays.stream(clazz.getInterfaces())
.filter(Metric.class::isAssignableFrom)
.findFirst();
if (optionalClass.isPresent()) {
clazz = optionalClass.get();
break;
}
clazz = clazz.getSuperclass();
} while (clazz != null);
return MetricType.from(clazz == null ? metric.getClass() : clazz);
}
private static <T extends HelidonMetric> MetricType toType(Class<T> clazz) {
return METRIC_TO_TYPE_MAP.getOrDefault(clazz, MetricType.INVALID);
}
private static Map<Class<? extends HelidonMetric>, MetricType> prepareMetricToTypeMap() {
final Map<Class<? extends HelidonMetric>, MetricType> result = new HashMap<>();
result.put(HelidonCounter.class, MetricType.COUNTER);
result.put(HelidonGauge.class, MetricType.GAUGE);
result.put(HelidonHistogram.class, MetricType.HISTOGRAM);
result.put(HelidonMeter.class, MetricType.METERED);
result.put(HelidonTimer.class, MetricType.TIMER);
return result;
}
}

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

@@ -23,6 +23,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.SortedMap;
import java.util.SortedSet;
@@ -544,8 +545,8 @@ public class Registry extends MetricRegistry implements io.helidon.common.metric
&& a.getTypeRaw().equals(b.getTypeRaw())
&& (((isFlexible(a) || isFlexible(b))
|| (a.getDisplayName().equals(b.getDisplayName())
&& a.getDescription().equals(b.getDescription())
&& a.getUnit().equals(b.getUnit())
&& Objects.equals(a.getDescription(), b.getDescription())
&& Objects.equals(a.getUnit(), b.getUnit())
&& (a.isReusable() == b.isReusable())
))
);

10
microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/FaultToleranceMetrics.java
View File

@@ -17,6 +17,7 @@
package io.helidon.microprofile.faulttolerance;
import java.lang.reflect.Method;
import java.util.Collections;
import javax.enterprise.inject.spi.CDI;
@@ -303,7 +304,8 @@ class FaultToleranceMetrics {
*/
private static Counter registerCounter(String name, String description) {
return getMetricRegistry().counter(
newMetadata(name, name, description, MetricType.COUNTER, MetricUnits.NONE));
newMetadata(name, name, description, MetricType.COUNTER, MetricUnits.NONE,
true, Collections.emptyMap()));
}
/**
@@ -315,7 +317,8 @@ class FaultToleranceMetrics {
*/
static Histogram registerHistogram(String name, String description) {
return getMetricRegistry().histogram(
newMetadata(name, name, description, MetricType.HISTOGRAM, MetricUnits.NANOSECONDS));
newMetadata(name, name, description, MetricType.HISTOGRAM, MetricUnits.NANOSECONDS,
true, Collections.emptyMap()));
}
/**
@@ -335,7 +338,8 @@ class FaultToleranceMetrics {
Gauge<T> existing = getMetricRegistry().getBridgeGauges().get(metricID);
if (existing == null) {
getMetricRegistry().register(
newMetadata(metricID.getName(), metricID.getName(), description, MetricType.GAUGE, MetricUnits.NANOSECONDS),
newMetadata(metricID.getName(), metricID.getName(), description, MetricType.GAUGE, MetricUnits.NANOSECONDS,
true, Collections.emptyMap()),
gauge);
}
return existing;

1
microprofile/fault-tolerance/src/test/java/io/helidon/microprofile/faulttolerance/MetricsTest.java
View File

@@ -21,6 +21,7 @@ import java.util.concurrent.Future;
import io.helidon.common.metrics.InternalBridge;
import io.helidon.common.metrics.InternalBridge.Metadata.MetadataBuilder;
import io.helidon.common.metrics.InternalBridge.MetricID;
import io.helidon.common.metrics.InternalBridge.MetricRegistry;
import org.eclipse.microprofile.faulttolerance.exceptions.CircuitBreakerOpenException;

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

@@ -17,6 +17,9 @@
package io.helidon.microprofile.metrics;
import java.lang.reflect.Constructor;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Supplier;
import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.inject.Produces;
@@ -69,17 +72,12 @@ public class MetricProducer {
}
private static String getName(Metric metric, InjectionPoint ip) {
StringBuilder fullName =
new StringBuilder(metric.absolute() ? "" : ip.getMember().getDeclaringClass().getName() + ".");
if (metric.name().isEmpty()) {
fullName.append(ip.getMember().getName());
if (ip.getMember() instanceof Constructor) {
fullName.append(".new");
}
} else {
fullName.append(metric.name());
}
return fullName.toString();
boolean isAbsolute = metric != null && metric.absolute();
String prefix = isAbsolute ? "" : ip.getMember().getDeclaringClass().getName() + ".";
String shortName = metric != null && !metric.name().isEmpty() ? metric.name() : ip.getMember().getName();
String ctorSuffix = ip.getMember() instanceof Constructor ? ".new" : "";
String fullName = prefix + shortName + ctorSuffix;
return fullName;
}
@Produces
@@ -90,8 +88,7 @@ public class MetricProducer {
@Produces
@VendorDefined
private Counter produceCounter(MetricRegistry registry, InjectionPoint ip) {
Metric metric = ip.getAnnotated().getAnnotation(Metric.class);
return registry.counter(newMetadata(ip, metric, MetricType.COUNTER));
return produceMetric(ip, registry::getCounters, registry::counter, Counter.class);
}
@Produces
@@ -102,8 +99,7 @@ public class MetricProducer {
@Produces
@VendorDefined
private Meter produceMeter(MetricRegistry registry, InjectionPoint ip) {
Metric metric = ip.getAnnotated().getAnnotation(Metric.class);
return registry.meter(newMetadata(ip, metric, MetricType.METERED));
return produceMetric(ip, registry::getMeters, registry::meter, Meter.class);
}
@Produces
@@ -114,8 +110,7 @@ public class MetricProducer {
@Produces
@VendorDefined
private Timer produceTimer(MetricRegistry registry, InjectionPoint ip) {
Metric metric = ip.getAnnotated().getAnnotation(Metric.class);
return registry.timer(newMetadata(ip, metric, MetricType.TIMER));
return produceMetric(ip, registry::getTimers, registry::timer, Timer.class);
}
@Produces
@@ -126,8 +121,7 @@ public class MetricProducer {
@Produces
@VendorDefined
private Histogram produceHistogram(MetricRegistry registry, InjectionPoint ip) {
Metric metric = ip.getAnnotated().getAnnotation(Metric.class);
return registry.histogram(newMetadata(ip, metric, MetricType.HISTOGRAM));
return produceMetric(ip, registry::getHistograms, registry::histogram, Histogram.class);
}
/**
@@ -162,4 +156,19 @@ public class MetricProducer {
private <T> Gauge<T> produceGaugeDefault(MetricRegistry registry, InjectionPoint ip) {
return produceGauge(registry, ip);
}
private <T> T produceMetric(InjectionPoint ip, 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));
/*
* 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 (result == null) {
result = registerFn.apply(newMetadata(ip, metricAnno, MetricType.from(clazz)));
}
return result;
}
}

24
microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsCdiExtension.java
View File

@@ -196,13 +196,19 @@ public class MetricsCdiExtension implements Extension {
configurator.filterMethods(method -> !Modifier.isPrivate(method.getJavaMember().getModifiers()))
.forEach(method -> {
METRIC_ANNOTATIONS.forEach(annotation -> {
LookupResult<? extends Annotation> lookupResult
= lookupAnnotation(method.getAnnotated().getJavaMember(), annotation, clazz);
if (lookupResult != null) {
registerMetric(method.getAnnotated().getJavaMember(), clazz, lookupResult);
}
Method m = method.getAnnotated().getJavaMember();
LookupResult<? extends Annotation> lookupResult = lookupAnnotation(m, annotation, clazz);
// For methods, register the metric only on the declaring
// class, not subclasses per the MP Metrics TCK
// VisibilityTimedMethodBeanTest.
if (lookupResult != null
&& (lookupResult.getType() != MetricUtil.MatchingType.METHOD
|| clazz.equals(m.getDeclaringClass()))) {
registerMetric(method.getAnnotated().getJavaMember(), clazz, lookupResult);
}
});
});
});
// Process constructors
configurator.filterConstructors(constructor -> !Modifier.isPrivate(constructor.getJavaMember().getModifiers()))
@@ -232,7 +238,8 @@ public class MetricsCdiExtension implements Extension {
* @param ppf Producer field.
*/
private void recordProducerFields(@Observes ProcessProducerField<? extends org.eclipse.microprofile.metrics.Metric, ?> ppf) {
LOGGER.log(Level.FINE, () -> "### recordProducerFields " + ppf.getBean().getBeanClass());
LOGGER.log(Level.FINE, () -> "### recordProducerFields " + ppf.getBean().getBeanClass()
+ ", field: " + ppf.getAnnotatedProducerField());
if (!MetricProducer.class.equals(ppf.getBean().getBeanClass())) {
Metric metric = ppf.getAnnotatedProducerField().getAnnotation(Metric.class);
if (metric != null) {
@@ -260,7 +267,8 @@ public class MetricsCdiExtension implements Extension {
*/
private void recordProducerMethods(@Observes
ProcessProducerMethod<? extends org.eclipse.microprofile.metrics.Metric, ?> ppm) {
LOGGER.log(Level.FINE, () -> "### recordProducerMethods " + ppm.getBean().getBeanClass());
LOGGER.log(Level.FINE, () -> "### recordProducerMethods " + ppm.getBean().getBeanClass()
+ ", method: " + ppm.getAnnotatedProducerMethod());
if (!MetricProducer.class.equals(ppm.getBean().getBeanClass())) {
Metric metric = ppm.getAnnotatedProducerMethod().getAnnotation(Metric.class);
if (metric != null) {

11
microprofile/metrics/src/test/java/io/helidon/microprofile/metrics/MetricsMpServiceTest.java
View File

@@ -19,13 +19,14 @@ package io.helidon.microprofile.metrics;
import javax.ws.rs.client.Client;
import javax.ws.rs.client.ClientBuilder;
import io.helidon.common.metrics.InternalBridge.MetricRegistry;
import io.helidon.config.Config;
import io.helidon.metrics.RegistryFactory;
import io.helidon.microprofile.config.MpConfig;
import io.helidon.microprofile.server.Server;
import org.eclipse.microprofile.metrics.Counter;
import org.eclipse.microprofile.metrics.MetricRegistry.Type;
import org.eclipse.microprofile.metrics.Metadata;
import org.eclipse.microprofile.metrics.MetricRegistry;
import org.eclipse.microprofile.metrics.MetricType;
import org.eclipse.microprofile.metrics.MetricUnits;
import org.junit.jupiter.api.AfterAll;
@@ -53,8 +54,7 @@ public class MetricsMpServiceTest {
.build();
server.start();
registry = io.helidon.common.metrics.InternalBridge.INSTANCE
.getRegistryFactory().getBridgeRegistry(Type.APPLICATION);
registry = registry = RegistryFactory.getInstance().getRegistry(MetricRegistry.Type.APPLICATION);
port = server.port();
baseUri = "http://localhost:" + port;
@@ -70,8 +70,7 @@ public class MetricsMpServiceTest {
}
protected static void registerCounter(String name) {
io.helidon.common.metrics.InternalBridge.Metadata meta =
io.helidon.common.metrics.InternalBridge.Metadata.newMetadata(name,
Metadata meta = new Metadata(name,
name,
name,
MetricType.COUNTER,