better tag handling for all metric types
diff --git a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/RegistryImpl.java b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/RegistryImpl.java
index 61cb4eb..469af63 100644
--- a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/RegistryImpl.java
+++ b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/RegistryImpl.java
@@ -244,7 +244,7 @@
@Override
public Histogram histogram(final String name, final Tag... tags) {
- return histogram(Metadata.builder().withName(name).withType(MetricType.HISTOGRAM).build());
+ return histogram(Metadata.builder().withName(name).withType(MetricType.HISTOGRAM).build(), tags);
}
@Override
@@ -254,7 +254,7 @@
@Override
public Meter meter(final String name, final Tag... tags) {
- return meter(Metadata.builder().withName(name).withType(MetricType.METERED).build());
+ return meter(Metadata.builder().withName(name).withType(MetricType.METERED).build(), tags);
}
@Override
@@ -264,7 +264,7 @@
@Override
public Timer timer(final String name, final Tag... tags) {
- return timer(Metadata.builder().withName(name).withType(MetricType.TIMER).build());
+ return timer(Metadata.builder().withName(name).withType(MetricType.TIMER).build(), tags);
}
@Override
diff --git a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
index 7aec70a..47f0c40 100644
--- a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
+++ b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/jaxrs/MetricsEndpoints.java
@@ -24,6 +24,7 @@
import static java.util.stream.Collectors.toMap;
import java.util.Collections;
+import java.util.HashMap;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Stream;
@@ -41,12 +42,18 @@
import javax.ws.rs.core.UriInfo;
import org.apache.geronimo.microprofile.metrics.common.prometheus.PrometheusFormatter;
+import org.eclipse.microprofile.metrics.ConcurrentGauge;
import org.eclipse.microprofile.metrics.Counter;
import org.eclipse.microprofile.metrics.Gauge;
+import org.eclipse.microprofile.metrics.Histogram;
import org.eclipse.microprofile.metrics.Metadata;
+import org.eclipse.microprofile.metrics.Meter;
+import org.eclipse.microprofile.metrics.Metered;
import org.eclipse.microprofile.metrics.Metric;
import org.eclipse.microprofile.metrics.MetricID;
import org.eclipse.microprofile.metrics.MetricRegistry;
+import org.eclipse.microprofile.metrics.Snapshot;
+import org.eclipse.microprofile.metrics.Timer;
@Path("metrics")
public class MetricsEndpoints {
@@ -89,7 +96,7 @@
securityValidator.checkSecurity(securityContext, uriInfo);
return Stream.of(MetricRegistry.Type.values())
.collect(toMap(MetricRegistry.Type::getName, it -> findRegistry(it.getName()).getMetrics().entrySet().stream()
- .collect(toMap(Map.Entry::getKey, m -> map(m.getValue())))));
+ .collect(toMap(this::getKey, m -> toJson(map(m.getValue()), formatTags(m.getKey())), (a, b) -> a))));
}
@GET
@@ -114,7 +121,7 @@
@Context final UriInfo uriInfo) {
securityValidator.checkSecurity(securityContext, uriInfo);
return findRegistry(registry).getMetrics().entrySet().stream()
- .collect(toMap(Map.Entry::getKey, it -> map(it.getValue())));
+ .collect(toMap(this::getKey, it -> toJson(map(it.getValue()), formatTags(it.getKey())), (a, b) -> a));
}
@GET
@@ -174,12 +181,12 @@
@Context final UriInfo uriInfo) {
securityValidator.checkSecurity(securityContext, uriInfo);
return findRegistry(registry).getMetadata().entrySet().stream()
- .collect(toMap(Map.Entry::getKey, e -> mapMeta(e.getValue(), new MetricID(e.getKey()))));
+ .collect(toMap(Map.Entry::getKey, e -> mapMeta(e.getValue(), new MetricID(e.getKey())), (a, b) -> a));
}
private Map<String, Metric> metrics(final MetricRegistry metricRegistry) {
return metricRegistry.getMetrics().entrySet().stream()
- .collect(toMap(e -> e.getKey().getName(), Map.Entry::getValue));
+ .collect(toMap(this::getKey, Map.Entry::getValue, (a, b) -> a));
}
private <T> Map<String, T> singleEntry(final String id, final MetricRegistry metricRegistry,
@@ -203,6 +210,69 @@
return metric;
}
+ private String getKey(final Map.Entry<MetricID, Metric> e) {
+ if (Counter.class.isInstance(e.getValue()) || Gauge.class.isInstance(e.getValue())) {
+ return e.getKey().getName() + formatTags(e.getKey());
+ }
+ return e.getKey().getName();
+ }
+
+ // https://github.com/eclipse/microprofile-metrics/issues/508
+ private Object toJson(final Object metric, final String nameSuffix) {
+ if (Timer.class.isInstance(metric)) {
+ final Timer meter = Timer.class.cast(metric);
+ final Map<Object, Object> map = new HashMap<>(15);
+ map.putAll(snapshot(meter.getSnapshot(), nameSuffix));
+ map.putAll(meter(meter, nameSuffix));
+ return map;
+ }
+ if (Meter.class.isInstance(metric)) {
+ return meter(Meter.class.cast(metric), nameSuffix);
+ }
+ if (Histogram.class.isInstance(metric)) {
+ final Histogram histogram = Histogram.class.cast(metric);
+ final Map<Object, Object> map = new HashMap<>(11);
+ map.putAll(snapshot(histogram.getSnapshot(), nameSuffix));
+ map.put("count" + nameSuffix, histogram.getCount());
+ return map;
+ }
+ if (ConcurrentGauge.class.isInstance(metric)) {
+ final ConcurrentGauge concurrentGauge = ConcurrentGauge.class.cast(metric);
+ final Map<Object, Object> map = new HashMap<>(3);
+ map.put("min" + nameSuffix, concurrentGauge.getMin());
+ map.put("current" + nameSuffix, concurrentGauge.getCount());
+ map.put("max" + nameSuffix, concurrentGauge.getMax());
+ return map;
+ }
+ // counters and gauges are unwrapped so skip it
+ return metric;
+ }
+
+ private Map<String, Object> meter(final Metered metered, final String nameSuffix) {
+ final Map<String, Object> map = new HashMap<>(5);
+ map.put("count" + nameSuffix, metered.getCount());
+ map.put("meanRate" + nameSuffix, metered.getMeanRate());
+ map.put("oneMinRate" + nameSuffix, metered.getOneMinuteRate());
+ map.put("fiveMinRate" + nameSuffix, metered.getFiveMinuteRate());
+ map.put("fifteenMinRate" + nameSuffix, metered.getFifteenMinuteRate());
+ return map;
+ }
+
+ private Map<String, Object> snapshot(final Snapshot snapshot, final String nameSuffix) {
+ final Map<String, Object> map = new HashMap<>(10);
+ map.put("p50" + nameSuffix, snapshot.getMedian());
+ map.put("p75" + nameSuffix, snapshot.get75thPercentile());
+ map.put("p95" + nameSuffix, snapshot.get95thPercentile());
+ map.put("p98" + nameSuffix, snapshot.get98thPercentile());
+ map.put("p99" + nameSuffix, snapshot.get99thPercentile());
+ map.put("p999" + nameSuffix, snapshot.get999thPercentile());
+ map.put("min" + nameSuffix, snapshot.getMin());
+ map.put("mean" + nameSuffix, snapshot.getMean());
+ map.put("max" + nameSuffix, snapshot.getMax());
+ map.put("stddev" + nameSuffix, snapshot.getStdDev());
+ return map;
+ }
+
private MetricRegistry findRegistry(final String registry) {
switch (Stream.of(MetricRegistry.Type.values()).filter(it -> it.getName().equals(registry)).findFirst()
.orElseThrow(() -> new WebApplicationException(Response.Status.NOT_FOUND))) {
@@ -215,6 +285,12 @@
}
}
+ private String formatTags(final MetricID id) {
+ return id.getTags().isEmpty() ? "" : (';' + id.getTags().entrySet().stream()
+ .map(e -> e.getKey() + "=" + e.getValue())
+ .collect(joining(",")));
+ }
+
public static class Meta {
private final Metadata value;
private final MetricID metricID;
diff --git a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/prometheus/PrometheusFormatter.java b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/prometheus/PrometheusFormatter.java
index c0a7598..0bf382d 100644
--- a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/prometheus/PrometheusFormatter.java
+++ b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/prometheus/PrometheusFormatter.java
@@ -32,13 +32,13 @@
import java.util.function.Predicate;
import java.util.stream.Stream;
+import org.eclipse.microprofile.metrics.ConcurrentGauge;
import org.eclipse.microprofile.metrics.Counter;
import org.eclipse.microprofile.metrics.Gauge;
import org.eclipse.microprofile.metrics.Histogram;
import org.eclipse.microprofile.metrics.Metadata;
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.Snapshot;
@@ -96,16 +96,28 @@
final Map<String, Metric> entries) {
final Map<String, Metadata> metadatas = registry.getMetadata();
return entries.entrySet().stream()
- .map(it -> new Entry(metadatas.get(it.getKey()), registryKey + ':' + toPrometheusKey(metadatas.get(it.getKey())), it.getValue()))
+ .map(it -> {
+ String key = it.getKey();
+ final int tagSep = key.indexOf(';');
+ if (tagSep > 0) {
+ key = key.substring(0, tagSep);
+ }
+ final Metadata metadata = metadatas.get(key);
+ return new Entry(metadata, registryKey + ':' + toPrometheusKey(metadata), it.getValue());
+ })
.filter(it -> prefixFilter == null || prefixFilter.test(it.prometheusKey))
.map(entry -> {
switch (entry.metadata.getTypeRaw()) {
- case COUNTER:
- case CONCURRENT_GAUGE: {
+ case COUNTER: {
final String key = toPrometheusKey(entry.metadata);
return new StringBuilder()
.append(value(registryKey, key, Counter.class.cast(entry.metric).getCount(), entry.metadata));
}
+ case CONCURRENT_GAUGE: {
+ final String key = toPrometheusKey(entry.metadata);
+ return new StringBuilder()
+ .append(value(registryKey, key, ConcurrentGauge.class.cast(entry.metric).getCount(), entry.metadata));
+ }
case GAUGE: {
final Object val = Gauge.class.cast(entry.metric).getValue();
if (Number.class.isInstance(val)) {
@@ -183,7 +195,7 @@
}
private String toUnitSuffix(final Metadata metadata) {
- return metadata.getUnit().orElse("").equalsIgnoreCase("none") ?
+ return metadata.getUnit().orElse("none").equalsIgnoreCase("none") ?
"" : ("_" + toPrometheusUnit(metadata.getUnit().orElse("")));
}
@@ -290,8 +302,8 @@
}
}
- private String toPrometheus(final Metadata metadata) {
- return metadata.getName()
+ private String toPrometheus(final Metadata id) {
+ return id.getName()
.replaceAll("[^\\w]+", "_")
.replaceAll("(.)(\\p{Upper})", "$1_$2")
.replace("__", "_")
diff --git a/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java b/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
index b5fe0de..cb50015 100644
--- a/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
+++ b/geronimo-metrics/src/main/java/org/apache/geronimo/microprofile/metrics/cdi/MetricsExtension.java
@@ -60,6 +60,7 @@
import org.apache.geronimo.microprofile.metrics.common.BaseMetrics;
import org.apache.geronimo.microprofile.metrics.common.GaugeImpl;
import org.apache.geronimo.microprofile.metrics.common.RegistryImpl;
+import org.apache.geronimo.microprofile.metrics.common.jaxrs.MetricsEndpoints;
import org.apache.geronimo.microprofile.metrics.jaxrs.CdiMetricsEndpoints;
import org.eclipse.microprofile.metrics.Counter;
import org.eclipse.microprofile.metrics.Gauge;
@@ -86,11 +87,13 @@
private final MetricRegistry vendorRegistry = new RegistryImpl();
private final Map<MetricID, Metadata> registrations = new HashMap<>();
- private final Map<String, Function<BeanManager, Gauge<?>>> gaugeFactories = new HashMap<>();
+ private final Map<MetricID, Function<BeanManager, Gauge<?>>> gaugeFactories = new HashMap<>();
private final Collection<Runnable> producersRegistrations = new ArrayList<>();
private final Collection<CreationalContext<?>> creationalContexts = new ArrayList<>();
- void letOtherExtensionsUseRegistries(@Observes final ProcessAnnotatedType<CdiMetricsEndpoints> processAnnotatedType) {
+ private Map<String, String> environmentalTags;
+
+ void vetoEndpointIfNotActivated(@Observes final ProcessAnnotatedType<CdiMetricsEndpoints> processAnnotatedType) {
if ("false".equalsIgnoreCase(System.getProperty("geronimo.metrics.jaxrs.activated"))) { // default is secured so deploy
processAnnotatedType.veto();
}
@@ -101,6 +104,13 @@
processAnnotatedType.veto();
}
+ // can happen in shades
+ void vetoNonCdiEndpoint(@Observes final ProcessAnnotatedType<MetricsEndpoints> processAnnotatedType) {
+ if (processAnnotatedType.getAnnotatedType().getJavaClass() == MetricsEndpoints.class) { // not subclasses
+ processAnnotatedType.veto();
+ }
+ }
+
void letOtherExtensionsUseRegistries(@Observes final BeforeBeanDiscovery beforeBeanDiscovery, final BeanManager beanManager) {
beforeBeanDiscovery.addQualifier(RegistryType.class);
beanManager.fireEvent(applicationRegistry);
@@ -114,6 +124,9 @@
final String name = method.getAnnotated().getJavaMember().getName();
return name.equals("name") || name.equals("tags");
}).forEach(method -> method.remove(a -> a.annotationType() == Nonbinding.class));
+
+ // we must update @Metrics with that if it exists for the cdi resolution ot match since we make it binding
+ environmentalTags = new MetricID("foo").getTags();
}
void onMetric(@Observes final ProcessProducerField<? extends Metric, ?> processProducerField, final BeanManager beanManager) {
@@ -132,7 +145,7 @@
}
void onMetric(@Observes ProcessProducerMethod<? extends Metric, ?> processProducerMethod,
- final BeanManager beanManager) {
+ final BeanManager beanManager) {
final org.eclipse.microprofile.metrics.annotation.Metric config = processProducerMethod.getAnnotated()
.getAnnotation(org.eclipse.microprofile.metrics.annotation.Metric.class);
if (config == null) {
@@ -160,7 +173,7 @@
final MetricType type = findType(clazz);
if (config != null) {
- String name = Names.findName(injectionPoint.getMember().getDeclaringClass(), injectionPoint.getMember(),
+ final String name = Names.findName(injectionPoint.getMember().getDeclaringClass(), injectionPoint.getMember(),
of(config.name()).filter(it -> !it.isEmpty()).orElseGet(injectionPoint.getMember()::getName), config.absolute(),
"");
final Metadata metadata = Metadata.builder()
@@ -173,7 +186,7 @@
final MetricID id = new MetricID(name, createTags(config.tags()));
addRegistration(metadata, id, true);
- if (!name.equals(config.name())) {
+ if (!name.equals(config.name()) || !environmentalTags.isEmpty()) {
final Annotation[] newQualifiers = Stream.concat(metricInjectionPointProcessEvent.getInjectionPoint().getQualifiers().stream()
.filter(it -> it.annotationType() != org.eclipse.microprofile.metrics.annotation.Metric.class),
Stream.of(new MetricImpl(metadata, id)))
@@ -317,7 +330,7 @@
.build();
final MetricID metricID = new MetricID(name, createTags(gauge.tags()));
addRegistration(metadata, metricID, false);
- gaugeFactories.put(name, beanManager -> {
+ gaugeFactories.put(metricID, beanManager -> {
final Object reference = getInstance(javaClass, beanManager);
final Method mtd = Method.class.cast(javaMember);
return new GaugeImpl<>(reference, mtd);
@@ -378,11 +391,11 @@
void afterDeploymentValidation(@Observes final AfterDeploymentValidation afterDeploymentValidation,
final BeanManager beanManager) {
- registrations.values().stream()
- .filter(m -> m.getTypeRaw() == MetricType.GAUGE)
- .forEach(metadata -> {
- final Gauge<?> gauge = gaugeFactories.get(metadata.getName()).apply(beanManager);
- applicationRegistry.register(metadata, gauge);
+ registrations.entrySet().stream()
+ .filter(e -> e.getValue().getTypeRaw() == MetricType.GAUGE)
+ .forEach(entry -> {
+ final Gauge<?> gauge = gaugeFactories.get(entry.getKey()).apply(beanManager);
+ applicationRegistry.register(entry.getValue(), gauge, entry.getKey().getTagsAsList().toArray(NO_TAG));
});
producersRegistrations.forEach(Runnable::run);
@@ -409,7 +422,9 @@
beanClass = javaMember.getDeclaringClass();
}
final Metadata metadata = createMetadata(config, clazz, javaMember, beanClass);
- applicationRegistry.register(metadata, Metric.class.cast(getInstance(clazz, beanManager, bean)));
+ applicationRegistry.register(
+ metadata, Metric.class.cast(getInstance(clazz, beanManager, bean)),
+ createTags(config.tags()));
}
private Metadata createMetadata(final org.eclipse.microprofile.metrics.annotation.Metric config,
diff --git a/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/ArquillianSetup.java b/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/ArquillianSetup.java
index 7b8ab56..2e86ae3 100644
--- a/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/ArquillianSetup.java
+++ b/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/ArquillianSetup.java
@@ -23,10 +23,14 @@
import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.CDI;
+import javax.enterprise.util.AnnotationLiteral;
import org.apache.catalina.Context;
import org.apache.meecrowave.Meecrowave;
import org.apache.meecrowave.arquillian.MeecrowaveContainer;
+import org.eclipse.microprofile.metrics.MetricID;
+import org.eclipse.microprofile.metrics.Tag;
+import org.eclipse.microprofile.metrics.annotation.Metric;
import org.jboss.arquillian.container.spi.client.container.DeployableContainer;
import org.jboss.arquillian.container.spi.context.annotation.ContainerScoped;
import org.jboss.arquillian.container.spi.context.annotation.DeploymentScoped;
@@ -132,9 +136,9 @@
try {
final CDI<Object> cdi = CDI.current();
final Annotation[] qualifiers = Stream.of(p.getAnnotations()).filter(it -> cdi.getBeanManager().isQualifier(it.annotationType())).toArray(Annotation[]::new);
- return cdi.select(p.getType(), qualifiers).get();
+ return cdi.select(p.getType(), fixQualifiers(qualifiers)).get();
} catch (final RuntimeException re) {
- re.printStackTrace();
+ re.printStackTrace(); // easier to debug when some test fail since TCK inject metrics as params
return null;
} finally {
thread.setContextClassLoader(classLoader);
@@ -142,6 +146,61 @@
})
.toArray();
}
+
+ private Annotation[] fixQualifiers(final Annotation[] qualifiers) {
+ return Stream.of(qualifiers)
+ .map(it -> {
+ if (Metric.class == it.annotationType()) { // we make tags and name binding so ensure it uses the right values
+ final Metric delegate = Metric.class.cast(it);
+ return new MetricLiteral(delegate, new MetricID(delegate.name(), Stream.of(delegate.tags()).filter(tag -> tag.contains("=")).map(tag -> {
+ final int sep = tag.indexOf("=");
+ return new Tag(tag.substring(0, sep), tag.substring(sep + 1));
+ }).toArray(Tag[]::new)).getTagsAsList().stream().map(t -> t.getTagName() + '=' + t.getTagValue()).toArray(String[]::new));
+ }
+ return it;
+ })
+ .toArray(Annotation[]::new);
+ }
+ }
+
+ private static class MetricLiteral extends AnnotationLiteral<Metric> implements Metric {
+ private final Metric delegate;
+ private final String[] tags;
+
+ private MetricLiteral(final Metric delegate, final String[] tags) {
+ this.delegate = delegate;
+ this.tags = tags;
+ }
+
+ @Override
+ public String name() {
+ return delegate.name();
+ }
+
+ @Override
+ public String[] tags() {
+ return tags;
+ }
+
+ @Override
+ public boolean absolute() {
+ return delegate.absolute();
+ }
+
+ @Override
+ public String displayName() {
+ return delegate.displayName();
+ }
+
+ @Override
+ public String description() {
+ return delegate.description();
+ }
+
+ @Override
+ public String unit() {
+ return delegate.unit();
+ }
}
public static class ForceWarProtocol extends LocalProtocol {
diff --git a/pom.xml b/pom.xml
index eeebdb2..e33d4c5 100644
--- a/pom.xml
+++ b/pom.xml
@@ -62,7 +62,7 @@
<dependency>
<groupId>org.apache.geronimo.config</groupId>
<artifactId>geronimo-config-impl</artifactId>
- <version>1.1</version>
+ <version>1.2.2</version>
<scope>test</scope>
</dependency>
