Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,19 @@ static List<Tag> createTags(String pairs) {
@Option.Configured("timers.json-units-default")
Optional<TimeUnit> jsonUnitsDefault();

/**
* Whether to log warnings when multiple registries are created.
* <p>
* By far most applications use a single meter registry, but certain app or library programming errors can cause Helidon to
* create more than one. By default, Helidon logs warning messages for each additional meter registry created. This
* setting allows users with apps that <em>need</em> multiple meter registries to suppress those warnings.
*
* @return whether to log warnings upon creation of multiple meter registries
*/
@Option.Configured
@Option.DefaultBoolean(true)
boolean warnOnMultipleRegistries();

/**
* Reports whether the specified scope is enabled, according to any scope configuration that
* is part of this metrics configuration.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, 2024 Oracle and/or its affiliates.
* Copyright (c) 2023, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,8 +24,11 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.StringJoiner;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.function.Consumer;
import java.util.function.Function;
Expand Down Expand Up @@ -79,6 +82,11 @@
class MMeterRegistry implements io.helidon.metrics.api.MeterRegistry {

private static final System.Logger LOGGER = System.getLogger(MMeterRegistry.class.getName());

private static StackTraceElement[] originalCreationStackTrace;
private static boolean hasLoggedFirstMultiInstantiationWarning = false;
private static final Lock WARNING_INFO_LOCK = new ReentrantLock();

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

/*
Expand Down Expand Up @@ -118,6 +126,7 @@ private MMeterRegistry(io.micrometer.core.instrument.MeterRegistry delegate,
this.clock = clock;
this.metricsFactory = metricsFactory;
this.metricsConfig = metricsConfig;
checkMultipleInstantiations(metricsConfig);
}

static Builder builder(
Expand Down Expand Up @@ -556,6 +565,51 @@ void onMeterRemoved(Meter removedMeter) {
}
}

// For testing.
static void clearMultipleInstantiationInfo() {
hasLoggedFirstMultiInstantiationWarning = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field are not volatile and not guarded by a lock - this may not work as expected.

originalCreationStackTrace = null;
}

private static void checkMultipleInstantiations(MetricsConfig metricsConfig) {
/*
We have not seen cases where multiple instantiations occur concurrently, but it's possible. Locking guards against the
very rare but probably very confusing possibility of our attempt at clarifying where multiple instantiations are occurring
actually getting corrupted by concurrent access to the static fields.
*/
WARNING_INFO_LOCK.lock();
try {
if (originalCreationStackTrace == null) {
originalCreationStackTrace = Thread.currentThread().getStackTrace();
} else if (metricsConfig.warnOnMultipleRegistries()) {
if (!hasLoggedFirstMultiInstantiationWarning) {
hasLoggedFirstMultiInstantiationWarning = true;
LOGGER.log(Level.WARNING,
"Unexpected duplicate instantiation\n"
+ "Original instantiation from:\n{0}\n\n"
+ "Additional instantiation from:\n{1}\n",

stackTraceToString(originalCreationStackTrace),
stackTraceToString(Thread.currentThread().getStackTrace()));
} else {
LOGGER.log(Level.WARNING,
"Unexpected additional instantiation from:\n{0}\n",
stackTraceToString(Thread.currentThread().getStackTrace()));
}
}
} finally {
WARNING_INFO_LOCK.unlock();
}
}

private static String stackTraceToString(StackTraceElement[] stackTraceElements) {
StringJoiner joiner = new StringJoiner("\n");
for (StackTraceElement element : stackTraceElements) {
joiner.add(element.toString());
}
return joiner.toString();
}

private io.helidon.metrics.api.Meter noopMeterIfDisabled(io.helidon.metrics.api.Meter.Builder<?, ?> builder) {
if (!isMeterEnabled(builder.name(), builder.tags(), builder.scope())) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
* Copyright (c) 2023, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -53,6 +53,7 @@ public MetricsFactory create(Config rootConfig, MetricsConfig metricsConfig, Col
public void close() {
metricsFactories.forEach(MetricsFactory::close);
metricsFactories.clear();
MMeterRegistry.clearMultipleInstantiationInfo();
List<Meter> meters = List.copyOf(Metrics.globalRegistry.getMeters());
meters.forEach(Metrics.globalRegistry::remove);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright (c) 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.helidon.metrics.providers.micrometer;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.PrintStream;

import io.helidon.metrics.api.Metrics;
import io.helidon.metrics.api.MetricsConfig;
import io.helidon.metrics.api.MetricsFactory;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.isEmptyString;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.not;

class TestMultipleRegistryLogging {

private static ByteArrayOutputStream baos;
private static PrintStream originalErr;
private static PrintStream testPrintStream;

@BeforeAll
static void beforeAll() {
baos = new ByteArrayOutputStream();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stealing the system error output will also disable output of Junit framework itself.
I think we are using a custom in-memory logging appender in some other test, which is a better approach to check this.

originalErr = System.err;
testPrintStream = new PrintStream(baos);
System.setErr(testPrintStream);
}

@BeforeEach
void setUp() {
MetricsFactory.closeAll();
}

@AfterEach
void tearDown() {
baos.reset();
}

@AfterAll
static void afterAll() {
MMeterRegistry.clearMultipleInstantiationInfo();
System.setErr(originalErr);
}

@Test
void testSingleRegistry() throws IOException {
Metrics.globalRegistry();
testPrintStream.flush();
String output = baos.toString();
assertThat("Single meter registry", output, isEmptyString());
}

@Test
void testTwoRegistries() throws IOException {
Metrics.globalRegistry();
Metrics.createMeterRegistry();
testPrintStream.flush();

String output = baos.toString();
assertThat("Two meter registries", output, allOf(
containsString("Unexpected duplicate"),
containsString("Original instantiation"),
containsString("Additional instantiation")));
}

@Test
void testThreeRegistries() throws IOException {
Metrics.globalRegistry();
Metrics.createMeterRegistry();
Metrics.createMeterRegistry();
testPrintStream.flush();

String output = baos.toString();
assertThat("Three meter registries", output, allOf(
containsString("Unexpected duplicate"),
containsString("Original instantiation"),
containsString("Additional instantiation"),
containsString("Unexpected additional instantiation")));
}

@Test
void testTwoRegistriesWithWarningDisabled() {
MetricsConfig configWithWarningsSuppressed = MetricsConfig.builder().warnOnMultipleRegistries(false).build();
Metrics.createMeterRegistry(configWithWarningsSuppressed);
Metrics.createMeterRegistry(configWithWarningsSuppressed);
testPrintStream.flush();

String output = baos.toString();
assertThat("Two meter registrations with warnings suppressed", output, allOf(
not(containsString("Unexpected duplicate")),
not(containsString("Original instantiation")),
not(containsString("Additional instantiation")),
not(containsString("Unexpected additional instantiation"))));

}

}