Skip to content

Commit e136f05

Browse files
committed
Refactor slightly for readability; add locking to avoid very unlikely but very confusing corruption of the static data we use to display the multiple instantiations
1 parent 8a382a5 commit e136f05

File tree

1 file changed

+44
-17
lines changed
  • metrics/providers/micrometer/src/main/java/io/helidon/metrics/providers/micrometer

1 file changed

+44
-17
lines changed

metrics/providers/micrometer/src/main/java/io/helidon/metrics/providers/micrometer/MMeterRegistry.java

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,18 @@
1717

1818
import java.lang.System.Logger.Level;
1919
import java.util.ArrayList;
20-
import java.util.Arrays;
2120
import java.util.Collection;
2221
import java.util.HashMap;
2322
import java.util.HashSet;
2423
import java.util.List;
2524
import java.util.Map;
2625
import java.util.Optional;
2726
import java.util.Set;
27+
import java.util.StringJoiner;
2828
import java.util.concurrent.CopyOnWriteArrayList;
29+
import java.util.concurrent.locks.Lock;
2930
import java.util.concurrent.locks.ReadWriteLock;
31+
import java.util.concurrent.locks.ReentrantLock;
3032
import java.util.concurrent.locks.ReentrantReadWriteLock;
3133
import java.util.function.Consumer;
3234
import java.util.function.Function;
@@ -83,6 +85,7 @@ class MMeterRegistry implements io.helidon.metrics.api.MeterRegistry {
8385

8486
private static volatile StackTraceElement[] originalCreationStackTrace;
8587
private static volatile boolean hasLoggedFirstMultiInstantiationWarning = false;
88+
private static final Lock WARNING_INFO_LOCK = new ReentrantLock();
8689

8790
private final io.micrometer.core.instrument.MeterRegistry delegate;
8891

@@ -123,22 +126,7 @@ private MMeterRegistry(io.micrometer.core.instrument.MeterRegistry delegate,
123126
this.clock = clock;
124127
this.metricsFactory = metricsFactory;
125128
this.metricsConfig = metricsConfig;
126-
if (originalCreationStackTrace == null) {
127-
originalCreationStackTrace = Thread.currentThread().getStackTrace();
128-
} else if (metricsConfig.warnOnMultipleRegistries()) {
129-
if (!hasLoggedFirstMultiInstantiationWarning) {
130-
hasLoggedFirstMultiInstantiationWarning = true;
131-
LOGGER.log(Level.WARNING,
132-
"Unexpected duplicate instantiation\nOriginal instantiation from:\n{0}\n\nAdditional instantiation "
133-
+ "from:\n{1}",
134-
Arrays.toString(originalCreationStackTrace),
135-
Arrays.toString(Thread.currentThread().getStackTrace()));
136-
} else {
137-
LOGGER.log(Level.WARNING,
138-
"Unexpected additional instantiation from:\n{0}",
139-
Arrays.toString(Thread.currentThread().getStackTrace()));
140-
}
141-
}
129+
checkMultipleInstantiations(metricsConfig);
142130
}
143131

144132
static Builder builder(
@@ -583,6 +571,45 @@ static void clearMultipleInstantiationInfo() {
583571
originalCreationStackTrace = null;
584572
}
585573

574+
private static void checkMultipleInstantiations(MetricsConfig metricsConfig) {
575+
/*
576+
We have not seen cases where multiple instantiations occur concurrently, but it's possible. Locking guards against the
577+
very rare but probably very confusing possibility of our attempt at clarifying where multiple instantiations are occurring
578+
actually getting corrupted by concurrent access to the static fields.
579+
*/
580+
WARNING_INFO_LOCK.lock();
581+
try {
582+
if (originalCreationStackTrace == null) {
583+
originalCreationStackTrace = Thread.currentThread().getStackTrace();
584+
} else if (metricsConfig.warnOnMultipleRegistries()) {
585+
if (!hasLoggedFirstMultiInstantiationWarning) {
586+
hasLoggedFirstMultiInstantiationWarning = true;
587+
LOGGER.log(Level.WARNING,
588+
"Unexpected duplicate instantiation\n"
589+
+ "Original instantiation from:\n{0}\n\n"
590+
+ "Additional instantiation from:\n{1}\n",
591+
592+
stackTraceToString(originalCreationStackTrace),
593+
stackTraceToString(Thread.currentThread().getStackTrace()));
594+
} else {
595+
LOGGER.log(Level.WARNING,
596+
"Unexpected additional instantiation from:\n{0}\n",
597+
stackTraceToString(Thread.currentThread().getStackTrace()));
598+
}
599+
}
600+
} finally {
601+
WARNING_INFO_LOCK.unlock();
602+
}
603+
}
604+
605+
private static String stackTraceToString(StackTraceElement[] stackTraceElements) {
606+
StringJoiner joiner = new StringJoiner("\n");
607+
for (StackTraceElement element : stackTraceElements) {
608+
joiner.add(element.toString());
609+
}
610+
return joiner.toString();
611+
}
612+
586613
private io.helidon.metrics.api.Meter noopMeterIfDisabled(io.helidon.metrics.api.Meter.Builder<?, ?> builder) {
587614
if (!isMeterEnabled(builder.name(), builder.tags(), builder.scope())) {
588615

0 commit comments

Comments
 (0)