Skip to content

Commit

Permalink
Fix system meters provider to return builders instead of create meter…
Browse files Browse the repository at this point in the history 
…s; a few other fix-ups
  • Loading branch information
@tjquinno
tjquinno committed Aug 30, 2023
1 parent 93bbe1f commit 039c332
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 93 deletions.
2 changes: 1 addition & 1 deletion metrics/api/src/main/java/io/helidon/metrics/api/MetricsFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ static MetricsFactory getInstance(Config rootConfig) {
*
* @return metrics config used to create the metrics factory
*/
MetricsConfig.Builder metricsConfig();
MetricsConfig metricsConfig();

/**
* Creates a new {@link io.helidon.metrics.api.MeterRegistry} using the provided metrics config.
Expand Down
4 changes: 2 additions & 2 deletions metrics/api/src/main/java/io/helidon/metrics/api/NoOpMetricsFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public long monotonicTime() {
}
};
private final MeterRegistry meterRegistry = new NoOpMeterRegistry();
private final MetricsConfig.Builder metricsConfig = MetricsConfig.builder();
private final MetricsConfig metricsConfig = MetricsConfig.create();

static NoOpMetricsFactory create() {
return new NoOpMetricsFactory();
Expand All @@ -59,7 +59,7 @@ public MeterRegistry globalRegistry(Consumer<Meter> onAddListener, Consumer<Mete
}

@Override
public MetricsConfig.Builder metricsConfig() {
public MetricsConfig metricsConfig() {
return metricsConfig;
}

Expand Down
5 changes: 3 additions & 2 deletions metrics/api/src/main/java/io/helidon/metrics/spi/MetersProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@
import io.helidon.metrics.api.MetricsFactory;

/**
* Furnishes {@link io.helidon.metrics.api.Meter.Builder} instances which are supplied to each {@link io.helidon.metrics.api.MetricsFactory}
* when it is created and, in turn, are passed to each {@link io.helidon.metrics.api.MeterRegistry} that factory creates.
* Furnishes {@link io.helidon.metrics.api.Meter.Builder} instances which are supplied to each
* {@link io.helidon.metrics.api.MetricsFactory} when it is created and, in turn, are passed to each
* {@link io.helidon.metrics.api.MeterRegistry} that factory creates.
*/
public interface MetersProvider {

Expand Down
4 changes: 3 additions & 1 deletion metrics/api/src/test/java/io/helidon/metrics/api/SimpleApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package io.helidon.metrics.api;

import java.time.Duration;
import java.util.Optional;
import java.util.Set;

Expand Down Expand Up @@ -47,7 +48,8 @@ static void prep() {

timer1 = Metrics.getOrCreate(Timer.builder("timer1")
.tags(Metrics.tags("t1", "v1",
"t2", "v2")));
"t2", "v2"))
.maximumExpectedValue(Duration.ofSeconds(4)));
}

@Test
Expand Down
142 changes: 88 additions & 54 deletions ...ders/micrometer/src/main/java/io/helidon/metrics/providers/micrometer/MMeterRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
import java.util.function.Predicate;

import io.helidon.metrics.api.Clock;
import io.helidon.metrics.api.FunctionalCounter;
import io.helidon.metrics.api.MetricsConfig;
import io.helidon.metrics.api.MetricsFactory;
import io.helidon.metrics.api.ScopingConfig;
import io.helidon.metrics.api.SystemTagsManager;
import io.helidon.metrics.api.Tag;
Expand Down Expand Up @@ -86,6 +86,7 @@ class MMeterRegistry implements io.helidon.metrics.api.MeterRegistry {
* Helidon API clock to be returned by the {@link #clock()} method.
*/
private final Clock clock;
private final MicrometerMetricsFactory metricsFactory;
private final MetricsConfig metricsConfig;

private final ScopingConfig scopingConfig;
Expand All @@ -105,10 +106,11 @@ class MMeterRegistry implements io.helidon.metrics.api.MeterRegistry {

private MMeterRegistry(MeterRegistry delegate,
Clock clock,
MetricsConfig metricsConfig) {
MicrometerMetricsFactory metricsFactory) {
this.delegate = delegate;
this.clock = clock;
this.metricsConfig = metricsConfig;
this.metricsFactory = metricsFactory;
metricsConfig = metricsFactory.metricsConfig();
scopingConfig = this.metricsConfig.scoping();
delegate.config()
.onMeterAdded(this::onMeterAdded)
Expand All @@ -128,19 +130,17 @@ private MMeterRegistry(MeterRegistry delegate,
* global tags.
* </p>
*
* @param meterRegistry existing Micrometer meter registry to wrap
* @param metricsConfig metrics config
* @param initialMeterBuilders builders for initial meters
* @param meterRegistry existing Micrometer meter registry to wrap
* @param metricsFactory metrics factory
* @return new wrapper around the specified Micrometer meter registry
*/
static MMeterRegistry create(MeterRegistry meterRegistry,
MetricsConfig metricsConfig,
Collection<io.helidon.metrics.api.Meter.Builder<?, ?>> initialMeterBuilders) {
MicrometerMetricsFactory metricsFactory) {
// The caller passed a pre-existing meter registry, with its own clock, so wrap that clock
// with a Helidon clock adapter (MClock).
return new MMeterRegistry(ensurePrometheusRegistryIsPresent(meterRegistry, metricsConfig),
return new MMeterRegistry(ensurePrometheusRegistryIsPresent(meterRegistry, metricsFactory.metricsConfig()),
MClock.create(meterRegistry.config().clock()),
metricsConfig);
metricsFactory);
}

/**
Expand All @@ -151,8 +151,7 @@ static MMeterRegistry create(MeterRegistry meterRegistry,
* @param metricsConfig metrics config
* @return new wrapper around a new Micrometer composite meter registry
*/
static MMeterRegistry create(MetricsConfig metricsConfig,
Collection<io.helidon.metrics.api.Meter.Builder<?, ?>> initialMeterBuilders) {
static MMeterRegistry create(MetricsConfig metricsConfig) {
CompositeMeterRegistry delegate = new CompositeMeterRegistry();
return create(ensurePrometheusRegistryIsPresent(delegate, metricsConfig),
MClock.create(delegate.config().clock()),
Expand Down Expand Up @@ -211,49 +210,12 @@ public Clock clock() {
}

@Override
public <HB extends io.helidon.metrics.api.Meter.Builder<HB, HM>,
HM extends io.helidon.metrics.api.Meter> HM getOrCreate(HB builder) {

// The Micrometer builders do not have a shared inherited declaration of the register method.
// Each type of builder declares its own so we need to decide here which specific one to invoke.
// That's so we can invoke the Micrometer builder's register method, which acts as
// get-or-create.
// Micrometer's register methods will throw an IllegalArgumentException if the caller specifies a builder that finds
// a previously-registered meter of a different type from that implied by the builder.

lock.lock();

try {

if (!isMeterEnabled(builder.name(), builder.tags(), builder.scope())) {
return (HM) MetricsFactory.getInstance().noOpMeter(builder);
}

io.helidon.metrics.api.Meter helidonMeter = null;

if (builder instanceof MCounter.Builder cBuilder) {
helidonMeter = getOrCreate(cBuilder, cBuilder::addTag, cBuilder.delegate()::register);
} else if (builder instanceof MFunctionalCounter.Builder<?> fcBuilder) {
helidonMeter = getOrCreate(fcBuilder, fcBuilder::addTag, fcBuilder.delegate()::register);
} else if (builder instanceof MDistributionSummary.Builder sBuilder) {
helidonMeter = getOrCreate(sBuilder, sBuilder::addTag, sBuilder.delegate()::register);
} else if (builder instanceof MGauge.Builder gBuilder) {
helidonMeter = getOrCreate(gBuilder, gBuilder::addTag, ((MGauge.Builder<?, ?>) gBuilder).delegate()::register);
} else if (builder instanceof MTimer.Builder tBuilder) {
helidonMeter = getOrCreate(tBuilder, tBuilder::addTag, tBuilder.delegate()::register);
} else {
throw new IllegalArgumentException(String.format("Unexpected builder type %s, expected one of %s",
builder.getClass().getName(),
List.of(MCounter.Builder.class.getName(),
MFunctionalCounter.Builder.class.getName(),
MDistributionSummary.Builder.class.getName(),
MGauge.Builder.class.getName(),
MTimer.Builder.class.getName())));
}
return (HM) helidonMeter;
} finally {
lock.unlock();
public <HB extends io.helidon.metrics.api.Meter.Builder<HB, HM>, HM extends io.helidon.metrics.api.Meter> HM getOrCreate(HB builder) {
// Just cast the builder if it "one of ours."
if (builder instanceof MMeter.Builder<?, ?, ?, ?> mBuilder) {
return getOrCreateTyped((HB) mBuilder);
}
return getOrCreateTyped(convertNeutralBuilder(builder));
}

@Override
Expand Down Expand Up @@ -384,6 +346,51 @@ private static MeterRegistry ensurePrometheusRegistryIsPresent(MeterRegistry met
return meterRegistry;
}

private <HB extends io.helidon.metrics.api.Meter.Builder<HB, HM>, HM extends io.helidon.metrics.api.Meter>
HM getOrCreateTyped(HB builder) {

// The Micrometer builders do not have a shared inherited declaration of the register method.
// Each type of builder declares its own so we need to decide here which specific one to invoke.
// That's so we can invoke the Micrometer builder's register method, which acts as
// get-or-create.
// Micrometer's register methods will throw an IllegalArgumentException if the caller specifies a builder that finds
// a previously-registered meter of a different type from that implied by the builder.

lock.lock();

try {

if (!isMeterEnabled(builder.name(), builder.tags(), builder.scope())) {
return (HM) metricsFactory.noOpMeter(builder);
}

io.helidon.metrics.api.Meter helidonMeter = null;

if (builder instanceof MCounter.Builder cBuilder) {
helidonMeter = getOrCreate(cBuilder, cBuilder::addTag, cBuilder.delegate()::register);
} else if (builder instanceof MFunctionalCounter.Builder<?> fcBuilder) {
helidonMeter = getOrCreate(fcBuilder, fcBuilder::addTag, fcBuilder.delegate()::register);
} else if (builder instanceof MDistributionSummary.Builder sBuilder) {
helidonMeter = getOrCreate(sBuilder, sBuilder::addTag, sBuilder.delegate()::register);
} else if (builder instanceof MGauge.Builder gBuilder) {
helidonMeter = getOrCreate(gBuilder, gBuilder::addTag, ((MGauge.Builder<?, ?>) gBuilder).delegate()::register);
} else if (builder instanceof MTimer.Builder tBuilder) {
helidonMeter = getOrCreate(tBuilder, tBuilder::addTag, tBuilder.delegate()::register);
} else {
throw new IllegalArgumentException(String.format("Unexpected builder type %s, expected one of %s",
builder.getClass().getName(),
List.of(MCounter.Builder.class.getName(),
MFunctionalCounter.Builder.class.getName(),
MDistributionSummary.Builder.class.getName(),
MGauge.Builder.class.getName(),
MTimer.Builder.class.getName())));
}
return (HM) helidonMeter;
} finally {
lock.unlock();
}
}

private <M extends Meter, HB extends MMeter.Builder<?, M, HB, HM>, HM extends MMeter<M>> HM getOrCreate(
HB mBuilder,
Function<Tag, ?> builderTagSetter,
Expand Down Expand Up @@ -470,6 +477,33 @@ HM extends MMeter<M>> HM wrapMeter(io.helidon.metrics.api.Meter.Id id,
return (HM) helidonMeter;
}

private <M extends Meter,
HB extends MMeter.Builder<?, M, HB, HM>,
HM extends MMeter<M>> HB convertNeutralBuilder(io.helidon.metrics.api.Meter.Builder<?, ?> builder) {
if (builder instanceof io.helidon.metrics.api.Counter.Builder cBuilder) {
return (HB) MMeter.fillInBuilder(MCounter.builder(cBuilder.name()), builder);
}
if (builder instanceof FunctionalCounter.Builder fcBuilder) {
return (HB) MMeter.fillInBuilder(MFunctionalCounter.builder(fcBuilder.name(),
fcBuilder.stateObject(),
fcBuilder.fn()), builder);
}
if (builder instanceof io.helidon.metrics.api.Gauge.Builder<?> gBuilder) {
return (HB) MMeter.fillInBuilder(MGauge.builder(gBuilder.name(), gBuilder.valueSupplier());
}
if (builder instanceof io.helidon.metrics.api.DistributionSummary.Builder sBuilder) {
MDistributionStatisticsConfig.Builder configBuilder = MDistributionStatisticsConfig.builder();
sBuilder.distributionStatisticsConfig().ifPresent(statsBuilder -> {
statsBuilder.
});

MDistributionSummary.Builder b = MDistributionSummary.builder(sBuilder.name());

sBuilder.distributionStatisticsConfig().ifPresent(b::distributionStatisticsConfig);
return (HB) MMeter.fillInBuilder(MDistributionSummary.builder(sBuilder.name(), sBuilder.distributionStatisticsConfig()));
}
}

private Optional<String> chooseScope(Meter meter, Optional<String> reliableScope) {
// The so-called "reliable" scope typically comes from a Helidon meter builder if a builder is available to the caller.
// IF the reliable scope is present then it is reliable because the developer set it.
Expand Down
2 changes: 1 addition & 1 deletion ...meter/src/main/java/io/helidon/metrics/providers/micrometer/MicrometerMetricsFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public MeterRegistry globalRegistry() {
}

@Override
public MetricsConfig.Builder metricsConfig() {
public MetricsConfig metricsConfig() {
return metricsConfigBuilder;
}

Expand Down
Loading

0 comments on commit 039c332

Please sign in to comment.