Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More carefully handle re-creation of global meter registry #8389

Merged
merged 1 commit into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ static List<Tag> createTags(String pairs) {
*
* @return metrics configuration
*/
@Option.Redundant
Config config();

/**
Expand Down
4 changes: 4 additions & 0 deletions metrics/providers/micrometer/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
<groupId>io.micrometer</groupId>
<artifactId>micrometer-registry-prometheus</artifactId>
</dependency>
<dependency>
<groupId>io.helidon.config</groupId>
<artifactId>helidon-config</artifactId>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
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, 2024 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 @@ -217,6 +217,7 @@ static MMeterRegistry applyMetersProvidersToRegistry(MetricsFactory factory,
public void close() {
onAddListeners.clear();
onRemoveListeners.clear();
List.copyOf(meters.values()).forEach(this::remove);
meters.clear();
buildersByPromMeterId.clear();
scopeMembership.clear();
Expand Down Expand Up @@ -472,7 +473,7 @@ <HM extends MMeter<M>, M extends Meter, B, HB extends MMeter.Builder<B, M, HB, H
if (builder == null) {
if (scope.isEmpty()) {
LOGGER.log(Level.DEBUG, "Processing meter creation with no scope from the meter or configuration: "
+ addedMeter);
+ addedMeter.getId());
}
id = neutralIdForAddedMeter;
mMeter = MMeter.create(id, addedMeter, scope);
Expand Down Expand Up @@ -506,7 +507,7 @@ void onMeterRemoved(Meter removedMeter) {
try {
MMeter<?> removedHelidonMeter = meters.remove(removedMeter);
if (removedHelidonMeter == null) {
LOGGER.log(Level.WARNING, "No matching neutral meter for implementation meter " + removedMeter);
LOGGER.log(Level.WARNING, "No matching neutral meter for implementation meter " + removedMeter.getId());
} else {
recordRemove(removedHelidonMeter);
}
Expand Down Expand Up @@ -586,7 +587,7 @@ but we have no record of that meter being linked to one of ours. This is surpris
*/

LOGGER.log(Level.WARNING,
"Unexpected discovery of unknown previously-created meter; creating wrapper for " + meter);
"Unexpected discovery of unknown previously-created meter; creating wrapper for " + meter.getId());
result = wrapMeter(id, meter, mBuilder.scope());
recordNewMeter(id, result, meter, effectiveScope);
}
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, 2024 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 @@ -27,7 +27,7 @@

import io.helidon.common.HelidonServiceLoader;
import io.helidon.common.LazyValue;
import io.helidon.common.config.Config;
import io.helidon.config.Config;
import io.helidon.metrics.api.Clock;
import io.helidon.metrics.api.Counter;
import io.helidon.metrics.api.DistributionStatisticsConfig;
Expand Down Expand Up @@ -56,7 +56,6 @@
*/
class MicrometerMetricsFactory implements MetricsFactory {

private final MetricsConfig metricsConfig;
private final Collection<MetersProvider> metersProviders;

private final Collection<MMeterRegistry> meterRegistries = new ConcurrentLinkedQueue<>();
Expand All @@ -74,14 +73,15 @@ class MicrometerMetricsFactory implements MetricsFactory {
.next());

private MMeterRegistry globalMeterRegistry;
private MetricsConfig metricsConfig;

private MicrometerMetricsFactory(MetricsConfig metricsConfig,
Collection<MetersProvider> metersProviders) {
this.metricsConfig = metricsConfig;
this.metersProviders = metersProviders;
}

static MicrometerMetricsFactory create(Config rootConfig,
static MicrometerMetricsFactory create(io.helidon.common.config.Config rootConfig,
MetricsConfig metricsConfig,
Collection<MetersProvider> metersProviders) {

Expand Down Expand Up @@ -114,7 +114,7 @@ public MMeterRegistry.Builder meterRegistryBuilder() {

@Override
public MeterRegistry createMeterRegistry(MetricsConfig metricsConfig) {
return save(MMeterRegistry.builder(Metrics.globalRegistry, this)
return save(metricsConfig, MMeterRegistry.builder(Metrics.globalRegistry, this)
.metricsConfig(metricsConfig)
.build());
}
Expand All @@ -124,7 +124,7 @@ public MeterRegistry createMeterRegistry(MetricsConfig metricsConfig) {
public MeterRegistry createMeterRegistry(MetricsConfig metricsConfig,
Consumer<Meter> onAddListener,
Consumer<Meter> onRemoveListener) {
return save(MMeterRegistry.builder(Metrics.globalRegistry,
return save(metricsConfig, MMeterRegistry.builder(Metrics.globalRegistry,
this)
.metricsConfig(metricsConfig)
.onMeterAdded(onAddListener)
Expand All @@ -139,7 +139,7 @@ public MeterRegistry createMeterRegistry(Clock clock,
Consumer<Meter> onAddListener,
Consumer<Meter> onRemoveListener) {

return save(MMeterRegistry.builder(Metrics.globalRegistry,
return save(metricsConfig, MMeterRegistry.builder(Metrics.globalRegistry,
this)
.metricsConfig(metricsConfig)
.clock(clock)
Expand All @@ -150,7 +150,7 @@ public MeterRegistry createMeterRegistry(Clock clock,

@Override
public MeterRegistry createMeterRegistry(Clock clock, MetricsConfig metricsConfig) {
return save(MMeterRegistry.builder(Metrics.globalRegistry, this)
return save(metricsConfig, MMeterRegistry.builder(Metrics.globalRegistry, this)
.clock(clock)
.metricsConfig(metricsConfig)
.build());
Expand All @@ -160,19 +160,26 @@ public MeterRegistry createMeterRegistry(Clock clock, MetricsConfig metricsConfi
public MeterRegistry globalRegistry() {
return globalMeterRegistry != null
? globalMeterRegistry
: globalRegistry(MetricsConfig.create());
: globalRegistry(MetricsConfig.create(Config.global().get(MetricsConfig.METRICS_CONFIG_KEY)));
}

@Override
public MeterRegistry globalRegistry(MetricsConfig metricsConfig) {
lock.lock();
try {
if (globalMeterRegistry != null) {
if (metricsConfig.equals(this.metricsConfig)) {
return globalMeterRegistry;
}
// Ideally this method will be invoked once with the proper MetricsConfig settings.
// But it's possible for it to be invoked more than once with different
// settings. In such a case we need to clear the old global registry and create a new one because
// the new settings might affect its behavior.
globalMeterRegistry.close();
meterRegistries.remove(globalMeterRegistry);
}
ensurePrometheusRegistry(Metrics.globalRegistry, metricsConfig);
globalMeterRegistry = save(MMeterRegistry.builder(Metrics.globalRegistry, this)
globalMeterRegistry = save(metricsConfig, MMeterRegistry.builder(Metrics.globalRegistry, this)
.metricsConfig(metricsConfig)
.build());

Expand Down Expand Up @@ -325,7 +332,8 @@ private <B extends Meter.Builder<B, M>, M extends Meter> MeterRegistry applyMete
return registry;
}

private MMeterRegistry save(MMeterRegistry meterRegistry) {
private MMeterRegistry save(MetricsConfig metricsConfig, MMeterRegistry meterRegistry) {
this.metricsConfig = metricsConfig;
meterRegistries.add(meterRegistry);
return meterRegistry;
}
Expand Down
4 changes: 2 additions & 2 deletions metrics/providers/micrometer/src/main/java/module-info.java
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, 2024 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 @@ -22,8 +22,8 @@
requires micrometer.core;
requires static micrometer.registry.prometheus;
requires io.helidon.common;
requires io.helidon.common.config;
requires io.helidon.common.media.type;
requires io.helidon.config;
requires simpleclient.common;
requires simpleclient.tracer.common;
requires simpleclient;
Expand Down