few fixes on the registry and setup
diff --git a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/ConcurrentGaugeImpl.java b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/ConcurrentGaugeImpl.java
index abd476f..6f3d218 100644
--- a/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/ConcurrentGaugeImpl.java
+++ b/geronimo-metrics-common/src/main/java/org/apache/geronimo/microprofile/metrics/common/ConcurrentGaugeImpl.java
@@ -16,50 +16,90 @@
*/
package org.apache.geronimo.microprofile.metrics.common;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
import java.util.concurrent.atomic.AtomicLong;
import org.eclipse.microprofile.metrics.ConcurrentGauge;
+// this minute thing is stupid but what the TCK expect...todo: move to a scheduledexecutor to avoid that stupid cost
public class ConcurrentGaugeImpl implements ConcurrentGauge {
+ private static final Clock CLOCK = Clock.tickMinutes(ZoneId.of("UTC"));
+
private final AtomicLong delegate = new AtomicLong();
private final AtomicLong min = new AtomicLong();
private final AtomicLong max = new AtomicLong();
+
+ private volatile Instant currentMinute = CLOCK.instant();
+
+ private volatile long lastMax = -1;
+ private volatile long lastMin = -1;
private final String unit;
public ConcurrentGaugeImpl(final String unit) {
this.unit = unit;
}
+ public String getUnit() {
+ return unit;
+ }
+
@Override
public void inc() {
- final long value = delegate.incrementAndGet();
- final long max = this.max.get();
- if (max < value) {
- this.max.compareAndSet(max, value);
+ maybeRotate();
+ synchronized (this) {
+ final long value = delegate.incrementAndGet();
+ final long max = this.max.get();
+ if (max < value) {
+ this.max.set(value);
+ }
}
}
@Override
public void dec() {
- final long value = delegate.decrementAndGet();
- final long min = this.min.get();
- if (min < value) {
- this.min.compareAndSet(min, value);
+ maybeRotate();
+ synchronized (this) {
+ final long value = delegate.decrementAndGet();
+ final long min = this.min.get();
+ if (min < value) {
+ this.min.set(value);
+ }
}
}
@Override
public long getCount() {
+ maybeRotate();
return delegate.get();
}
@Override
public long getMax() {
- return max.get();
+ maybeRotate();
+ return lastMax;
}
@Override
public long getMin() {
- return min.get();
+ maybeRotate();
+ return lastMin;
+ }
+
+ private void maybeRotate() {
+ final Instant now = CLOCK.instant();
+ if (now.isAfter(currentMinute)) {
+ synchronized (this) {
+ if (now.isAfter(currentMinute)) {
+ final long count = delegate.get();
+ lastMin = min.getAndSet(count);
+ lastMax = max.getAndSet(count);
+ min.set(count);
+ max.set(count);
+ currentMinute = now;
+ }
+ }
+ }
}
}
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 2945a5e..17ab35b 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
@@ -48,8 +48,9 @@
@Override
public <T extends Metric> T register(final Metadata metadata, final T metric) throws IllegalArgumentException {
+ final MetricID metricID = new MetricID(metadata.getName());
final Holder<? extends Metric> holder = metrics.putIfAbsent(
- new MetricID(metadata.getName()), new Holder<>(metric, metadata));
+ metricID, new Holder<>(metric, metadata, metricID));
if (holder != null && !metadata.isReusable() && !holder.metadata.isReusable()) {
throw new IllegalArgumentException("'" + metadata.getName() + "' metric already exists and is not reusable");
}
@@ -74,6 +75,8 @@
type = MetricType.TIMER;
} else if (Histogram.class.isInstance(metric)) {
type = MetricType.HISTOGRAM;
+ } else if (ConcurrentGauge.class.isInstance(metric)) {
+ type = MetricType.CONCURRENT_GAUGE;
} else {
type = MetricType.INVALID;
}
@@ -87,10 +90,11 @@
@Override
public Counter counter(final Metadata metadata, final Tag... tags) {
- Holder<? extends Metric> holder = metrics.get(new MetricID(metadata.getName(), tags));
+ Holder<? extends Metric> holder = metrics.get(metadata.getName());
if (holder == null) {
- holder = new Holder<>(new CounterImpl(metadata.getUnit().orElse("")), metadata);
- final Holder<? extends Metric> existing = metrics.putIfAbsent(new MetricID(metadata.getName(), tags), holder);
+ holder = new Holder<>(new CounterImpl(
+ metadata.getUnit().orElse("")), metadata, new MetricID(metadata.getName(), tags));
+ final Holder<? extends Metric> existing = metrics.putIfAbsent(holder.metricID, holder);
if (existing != null) {
holder = existing;
}
@@ -122,10 +126,12 @@
@Override
public ConcurrentGauge concurrentGauge(final Metadata metadata, final Tag... tags) {
- Holder<? extends Metric> holder = metrics.get(new MetricID(metadata.getName(), tags));
+ final MetricID metricID = new MetricID(metadata.getName(), tags);
+ Holder<? extends Metric> holder = metrics.get(metricID);
if (holder == null) {
- holder = new Holder<>(new ConcurrentGaugeImpl(metadata.getUnit().orElse("")), metadata);
- final Holder<? extends Metric> existing = metrics.putIfAbsent(new MetricID(metadata.getName(), tags), holder);
+ holder = new Holder<>(new ConcurrentGaugeImpl(
+ metadata.getUnit().orElse("")), metadata, metricID);
+ final Holder<? extends Metric> existing = metrics.putIfAbsent(holder.metricID, holder);
if (existing != null) {
holder = existing;
}
@@ -147,11 +153,11 @@
@Override
public Histogram histogram(final Metadata metadata, final Tag... tags) {
-
- Holder<? extends Metric> holder = metrics.get(new MetricID(metadata.getName(), tags));
+ final MetricID metricID = new MetricID(metadata.getName(), tags);
+ Holder<? extends Metric> holder = metrics.get(metricID);
if (holder == null) {
- holder = new Holder<>(new HistogramImpl(metadata.getUnit().orElse("")), metadata);
- final Holder<? extends Metric> existing = metrics.putIfAbsent(new MetricID(metadata.getName(), tags), holder);
+ holder = new Holder<>(new HistogramImpl(metadata.getUnit().orElse("")), metadata, metricID);
+ final Holder<? extends Metric> existing = metrics.putIfAbsent(metricID, holder);
if (existing != null) {
holder = existing;
}
@@ -173,10 +179,11 @@
@Override
public Meter meter(final Metadata metadata, final Tag... tags) {
- Holder<? extends Metric> holder = metrics.get(new MetricID(metadata.getName(), tags));
+ final MetricID metricID = new MetricID(metadata.getName(), tags);
+ Holder<? extends Metric> holder = metrics.get(metricID);
if (holder == null) {
- holder = new Holder<>(new MeterImpl(metadata.getUnit().orElse("")), metadata);
- final Holder<? extends Metric> existing = metrics.putIfAbsent(new MetricID(metadata.getName(), tags), holder);
+ holder = new Holder<>(new MeterImpl(metadata.getUnit().orElse("")), metadata, metricID);
+ final Holder<? extends Metric> existing = metrics.putIfAbsent(metricID, holder);
if (existing != null) {
holder = existing;
}
@@ -198,10 +205,11 @@
@Override
public Timer timer(final Metadata metadata, final Tag... tags) {
- Holder<? extends Metric> holder = metrics.get(new MetricID(metadata.getName(), tags));
+ final MetricID metricID = new MetricID(metadata.getName(), tags);
+ Holder<? extends Metric> holder = metrics.get(metricID);
if (holder == null) {
- holder = new Holder<>(new TimerImpl(metadata.getUnit().orElse("")), metadata);
- final Holder<? extends Metric> existing = metrics.putIfAbsent(new MetricID(metadata.getName(), tags), holder);
+ holder = new Holder<>(new TimerImpl(metadata.getUnit().orElse("")), metadata, metricID);
+ final Holder<? extends Metric> existing = metrics.putIfAbsent(metricID, holder);
if (existing != null) {
holder = existing;
}
@@ -348,7 +356,8 @@
@Override
public Map<String, Metadata> getMetadata() {
- return metrics.entrySet().stream().collect(toMap(e -> e.getKey().getName(), e -> e.getValue().metadata));
+ return metrics.entrySet().stream()
+ .collect(toMap(e -> e.getKey().getName(), e -> e.getValue().metadata, (a, b) -> a));
}
private <T extends Metric> SortedMap<MetricID, T> filterByType(final MetricFilter filter, final Class<T> type) {
@@ -363,10 +372,12 @@
private static final class Holder<T extends Metric> {
private final T metric;
private final Metadata metadata;
+ private final MetricID metricID;
- private Holder(final T metric, final Metadata metadata) {
+ private Holder(final T metric, final Metadata metadata, final MetricID metricID) {
this.metric = metric;
this.metadata = Metadata.builder(metadata).build();
+ this.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 170e1ae..c0a7598 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
@@ -99,7 +99,6 @@
.map(it -> new Entry(metadatas.get(it.getKey()), registryKey + ':' + toPrometheusKey(metadatas.get(it.getKey())), it.getValue()))
.filter(it -> prefixFilter == null || prefixFilter.test(it.prometheusKey))
.map(entry -> {
-
switch (entry.metadata.getTypeRaw()) {
case COUNTER:
case CONCURRENT_GAUGE: {
diff --git a/geronimo-metrics/pom.xml b/geronimo-metrics/pom.xml
index 9345a67..e3e09f5 100644
--- a/geronimo-metrics/pom.xml
+++ b/geronimo-metrics/pom.xml
@@ -85,6 +85,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
+ <reuseForks>false</reuseForks> <!-- FIXME: should be true indeed but easier to debug tcks when upgrading from v1 to v2 -->
<dependenciesToScan>
<dependency>org.eclipse.microprofile.metrics:microprofile-metrics-api-tck</dependency>
<dependency>org.eclipse.microprofile.metrics:microprofile-metrics-rest-tck</dependency>
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 2825f35..bbb6967 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
@@ -151,7 +151,8 @@
}
final Annotated annotated = injectionPoint.getAnnotated();
- final org.eclipse.microprofile.metrics.annotation.Metric config = annotated.getAnnotation(org.eclipse.microprofile.metrics.annotation.Metric.class);
+ final org.eclipse.microprofile.metrics.annotation.Metric config = annotated.getAnnotation(
+ org.eclipse.microprofile.metrics.annotation.Metric.class);
final MetricType type = findType(clazz);
if (config != null) {
@@ -432,6 +433,8 @@
type = MetricType.TIMER;
} else if (Histogram.class.isAssignableFrom(clazz)) {
type = MetricType.HISTOGRAM;
+ } else if (org.eclipse.microprofile.metrics.ConcurrentGauge.class.isAssignableFrom(clazz)) {
+ type = MetricType.CONCURRENT_GAUGE;
} else {
type = MetricType.INVALID;
}
diff --git a/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/TckContainer.java b/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/TckContainer.java
index 3079ceb..d680419 100644
--- a/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/TckContainer.java
+++ b/geronimo-metrics/src/test/java/org/apache/geronimo/microprofile/metrics/test/TckContainer.java
@@ -19,20 +19,22 @@
import static java.lang.ClassLoader.getSystemClassLoader;
import static java.lang.String.format;
import static java.util.Optional.of;
-import static java.util.Optional.ofNullable;
import java.io.File;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;
+import java.util.logging.Logger;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.loader.WebappClassLoaderBase;
import org.apache.catalina.loader.WebappLoader;
import org.apache.meecrowave.Meecrowave;
+import org.apache.meecrowave.arquillian.MeecrowaveConfiguration;
import org.apache.meecrowave.arquillian.MeecrowaveContainer;
import org.apache.meecrowave.io.IO;
+import org.jboss.arquillian.container.spi.client.protocol.ProtocolDescription;
import org.jboss.arquillian.container.spi.client.protocol.metadata.HTTPContext;
import org.jboss.arquillian.container.spi.client.protocol.metadata.ProtocolMetaData;
import org.jboss.arquillian.container.spi.client.protocol.metadata.Servlet;
@@ -43,6 +45,12 @@
private final Map<Archive<?>, Runnable> onUnDeploy = new HashMap<>();
@Override
+ public void setup(final MeecrowaveConfiguration configuration) {
+ super.setup(configuration);
+ getConfiguration().setWatcherBouncing(-1);
+ }
+
+ @Override
public ProtocolMetaData deploy(final Archive<?> archive) {
final File dump = toArchiveDump(archive);
archive.as(ZipExporter.class).exportTo(dump, true);
@@ -77,7 +85,27 @@
@Override
public void undeploy(final Archive<?> archive) { // we rename the archive so the context so we must align the undeploy
- ofNullable(onUnDeploy.remove(archive)).ifPresent(Runnable::run);
+ Runnable remove = onUnDeploy.remove(archive);
+ if (remove == null && onUnDeploy.size() == 1) { // assume it is the one
+ final Archive<?> key = onUnDeploy.keySet().iterator().next();
+ remove = onUnDeploy.remove(key);
+ }
+ if (remove != null) {
+ remove.run();
+ } else {
+ Logger.getLogger(getClass().getName())
+ .warning("Can't find " + archive + " to undeploy it, it can break next tests");
+ }
+ }
+
+ private Meecrowave.Builder getConfiguration() {
+ try {
+ final Field field = getClass().getSuperclass().getDeclaredField("configuration");
+ field.setAccessible(true);
+ return Meecrowave.Builder.class.cast(field.get(this));
+ } catch (final Exception e) {
+ throw new IllegalStateException(e);
+ }
}
private Meecrowave getContainer() {
