Skip to content

Commit

Permalink
More carefully handle re-creation of global meter registry (helidon-i…
Browse files Browse the repository at this point in the history
  • Loading branch information
tjquinno authored and hrstoyanov committed Feb 23, 2024
1 parent 1502087 commit 0532bc0
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 17 deletions.
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

0 comments on commit 0532bc0

Please sign in to comment.