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

Disable by default gauge-based histograms in the Micrometer bridge #8360

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions instrumentation/micrometer/micrometer-1.5/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Settings for the Micrometer bridge instrumentation

| System property | Type | Default | Description |
|---|---|---|---|
| `otel.instrumentation.micrometer.base-time-unit` | String | `ms` | Set the base time unit for the OpenTelemetry `MeterRegistry` implementation. <details><summary>Valid values</summary>`ns`, `nanoseconds`, `us`, `microseconds`, `ms`, `microseconds`, `s`, `seconds`, `min`, `minutes`, `h`, `hours`, `d`, `days`</details> |
| `otel.instrumentation.micrometer.prometheus-mode.enabled` | boolean | false | Enable the "Prometheus mode" this will simulate the behavior of Micrometer's PrometheusMeterRegistry. The instruments will be renamed to match Micrometer instrument naming, and the base time unit will be set to seconds. |
| System property | Type | Default | Description |
|------------------------------------------------------------|---------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `otel.instrumentation.micrometer.base-time-unit` | String | `ms` | Set the base time unit for the OpenTelemetry `MeterRegistry` implementation. <details><summary>Valid values</summary>`ns`, `nanoseconds`, `us`, `microseconds`, `ms`, `microseconds`, `s`, `seconds`, `min`, `minutes`, `h`, `hours`, `d`, `days`</details> |
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to change the default to s (in the same release as #8435)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, definitely! I've been waiting for the other two micrometer PRs to get merged first though, because of a large number of conflicts (especially in test code)

| `otel.instrumentation.micrometer.prometheus-mode.enabled` | boolean | false | Enable the "Prometheus mode" this will simulate the behavior of Micrometer's PrometheusMeterRegistry. The instruments will be renamed to match Micrometer instrument naming, and the base time unit will be set to seconds. |
| `otel.instrumentation.micrometer.histogram-gauges.enabled` | boolean | false | Enables the generation of gauge-based Micrometer histograms for `DistributionSummary` and `Timer` instruments. |
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,25 @@ tasks {
jvmArgs("-Dotel.instrumentation.micrometer.base-time-unit=seconds")
}

val testHistogramGauges by registering(Test::class) {
filter {
includeTestsMatching("*HistogramGaugesTest")
}
include("**/*HistogramGaugesTest.*")
jvmArgs("-Dotel.instrumentation.micrometer.histogram-gauges.enabled=true")
}

test {
filter {
excludeTestsMatching("*TimerSecondsTest")
excludeTestsMatching("*PrometheusModeTest")
excludeTestsMatching("*HistogramGaugesTest")
}
}

check {
dependsOn(testBaseTimeUnit)
dependsOn(testPrometheusMode)
dependsOn(testHistogramGauges)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ public final class MicrometerSingletons {
.setBaseTimeUnit(
TimeUnitParser.parseConfigValue(
config.getString("otel.instrumentation.micrometer.base-time-unit")))
.setMicrometerHistogramGaugesEnabled(
config.getBoolean(
"otel.instrumentation.micrometer.histogram-gauges.enabled", false))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5;

import io.micrometer.core.instrument.Metrics;
import io.opentelemetry.instrumentation.micrometer.v1_5.AbstractDistributionSummaryHistogramGaugesTest;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.extension.RegisterExtension;

class DistributionSummaryHistogramGaugesTest
extends AbstractDistributionSummaryHistogramGaugesTest {

@RegisterExtension
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();

@AfterEach
public void cleanup() {
Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove);
}

@Override
protected InstrumentationExtension testing() {
return testing;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5;

import io.micrometer.core.instrument.Metrics;
import io.opentelemetry.instrumentation.micrometer.v1_5.AbstractTimerHistogramGaugesTest;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.extension.RegisterExtension;

class TimerHistogramGaugesTest extends AbstractTimerHistogramGaugesTest {

@RegisterExtension
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();

@AfterEach
public void cleanup() {
Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove);
}

@Override
protected InstrumentationExtension testing() {
return testing;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.micrometer.v1_5;

import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;

@SuppressWarnings("CanIgnoreReturnValueSuggester")
enum DistributionStatisticConfigModifier {
DISABLE_HISTOGRAM_GAUGES {
@Override
DistributionStatisticConfig modify(DistributionStatisticConfig config) {
return DistributionStatisticConfig.builder()
// disable all percentile and slo related options
.percentilesHistogram(null)
.percentiles((double[]) null)
.serviceLevelObjectives((double[]) null)
.percentilePrecision(null)
// and keep the rest
.minimumExpectedValue(config.getMinimumExpectedValueAsDouble())
.maximumExpectedValue(config.getMaximumExpectedValueAsDouble())
.expiry(config.getExpiry())
.bufferLength(config.getBufferLength())
.build();
}
},
IDENTITY {
@Override
DistributionStatisticConfig modify(DistributionStatisticConfig config) {
return config;
}
};

abstract DistributionStatisticConfig modify(DistributionStatisticConfig config);
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ final class OpenTelemetryDistributionSummary extends AbstractDistributionSummary
NamingConvention namingConvention,
Clock clock,
DistributionStatisticConfig distributionStatisticConfig,
DistributionStatisticConfigModifier modifier,
double scale,
Meter otelMeter) {
super(id, clock, distributionStatisticConfig, scale, false);
super(id, clock, modifier.modify(distributionStatisticConfig), scale, false);

if (isUsingMicrometerHistograms()) {
measurements = new MicrometerHistogramMeasurements();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,18 @@ public static OpenTelemetryMeterRegistryBuilder builder(OpenTelemetry openTeleme
}

private final TimeUnit baseTimeUnit;
private final DistributionStatisticConfigModifier distributionStatisticConfigModifier;
private final io.opentelemetry.api.metrics.Meter otelMeter;

OpenTelemetryMeterRegistry(
Clock clock,
TimeUnit baseTimeUnit,
NamingConvention namingConvention,
DistributionStatisticConfigModifier distributionStatisticConfigModifier,
io.opentelemetry.api.metrics.Meter otelMeter) {
super(clock);
this.baseTimeUnit = baseTimeUnit;
this.distributionStatisticConfigModifier = distributionStatisticConfigModifier;
this.otelMeter = otelMeter;

this.config()
Expand Down Expand Up @@ -104,6 +107,7 @@ protected Timer newTimer(
config().namingConvention(),
clock,
distributionStatisticConfig,
distributionStatisticConfigModifier,
pauseDetector,
getBaseTimeUnit(),
otelMeter);
Expand All @@ -118,7 +122,13 @@ protected DistributionSummary newDistributionSummary(
Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, double scale) {
OpenTelemetryDistributionSummary distributionSummary =
new OpenTelemetryDistributionSummary(
id, config().namingConvention(), clock, distributionStatisticConfig, scale, otelMeter);
id,
config().namingConvention(),
clock,
distributionStatisticConfig,
distributionStatisticConfigModifier,
scale,
otelMeter);
if (distributionSummary.isUsingMicrometerHistograms()) {
HistogramGauges.registerWithCommonFormat(distributionSummary, this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import io.micrometer.core.instrument.Clock;
import io.micrometer.core.instrument.DistributionSummary;
import io.micrometer.core.instrument.LongTaskTimer;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Timer;
import io.micrometer.core.instrument.config.NamingConvention;
import io.opentelemetry.api.OpenTelemetry;
import java.util.concurrent.TimeUnit;
Expand All @@ -22,6 +25,7 @@ public final class OpenTelemetryMeterRegistryBuilder {
private Clock clock = Clock.SYSTEM;
private TimeUnit baseTimeUnit = TimeUnit.MILLISECONDS;
private boolean prometheusMode = false;
private boolean histogramGaugesEnabled = false;

OpenTelemetryMeterRegistryBuilder(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
Expand Down Expand Up @@ -54,6 +58,27 @@ public OpenTelemetryMeterRegistryBuilder setPrometheusMode(boolean prometheusMod
return this;
}

/**
* Enables the generation of gauge-based Micrometer histograms. While the Micrometer bridge is
* able to map Micrometer's {@link DistributionSummary} and {@link Timer} service level objectives
* to OpenTelemetry histogram buckets, it might not cover all cases that are normally supported by
* Micrometer (e.g. the bridge is not able to translate percentiles). With this setting enabled,
* the Micrometer bridge will additionally emit Micrometer service level objectives and
* percentiles as separate gauges.
*
* <p>Note that this setting does not concern the {@link LongTaskTimer}, as it is not bridged to
* an OpenTelemetry histogram.
*
* <p>This is disabled by default, set this to {@code true} to enable gauge-based Micrometer
* histograms.
*/
@CanIgnoreReturnValue
public OpenTelemetryMeterRegistryBuilder setMicrometerHistogramGaugesEnabled(
boolean histogramGaugesEnabled) {
this.histogramGaugesEnabled = histogramGaugesEnabled;
return this;
}

/**
* Returns a new {@link OpenTelemetryMeterRegistry} with the settings of this {@link
* OpenTelemetryMeterRegistryBuilder}.
Expand All @@ -63,11 +88,16 @@ public MeterRegistry build() {
TimeUnit baseTimeUnit = prometheusMode ? TimeUnit.SECONDS : this.baseTimeUnit;
NamingConvention namingConvention =
prometheusMode ? PrometheusModeNamingConvention.INSTANCE : NamingConvention.identity;
DistributionStatisticConfigModifier modifier =
histogramGaugesEnabled
? DistributionStatisticConfigModifier.IDENTITY
: DistributionStatisticConfigModifier.DISABLE_HISTOGRAM_GAUGES;

return new OpenTelemetryMeterRegistry(
clock,
baseTimeUnit,
namingConvention,
modifier,
openTelemetry.getMeterProvider().get(INSTRUMENTATION_NAME));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,17 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter {
NamingConvention namingConvention,
Clock clock,
DistributionStatisticConfig distributionStatisticConfig,
DistributionStatisticConfigModifier modifier,
PauseDetector pauseDetector,
TimeUnit baseTimeUnit,
Meter otelMeter) {
super(id, clock, distributionStatisticConfig, pauseDetector, TimeUnit.MILLISECONDS, false);
super(
id,
clock,
modifier.modify(distributionStatisticConfig),
pauseDetector,
baseTimeUnit,
false);

if (isUsingMicrometerHistograms()) {
measurements = new MicrometerHistogramMeasurements();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.micrometer.v1_5;

import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
import org.junit.jupiter.api.extension.RegisterExtension;

class DistributionSummaryHistogramGaugesTest
extends AbstractDistributionSummaryHistogramGaugesTest {

@RegisterExtension
static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();

@RegisterExtension
static final MicrometerTestingExtension micrometerExtension =
new MicrometerTestingExtension(testing) {
@Override
OpenTelemetryMeterRegistryBuilder configureOtelRegistry(
OpenTelemetryMeterRegistryBuilder registry) {
return registry.setMicrometerHistogramGaugesEnabled(true);
}
};

@Override
protected InstrumentationExtension testing() {
return testing;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
import org.junit.jupiter.api.extension.RegisterExtension;

public class NamingConventionTest extends AbstractNamingConventionTest {
class NamingConventionTest extends AbstractNamingConventionTest {

@RegisterExtension
static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.micrometer.v1_5;

import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
import org.junit.jupiter.api.extension.RegisterExtension;

class TimerHistogramGaugesTest extends AbstractTimerHistogramGaugesTest {

@RegisterExtension
static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();

@RegisterExtension
static final MicrometerTestingExtension micrometerExtension =
new MicrometerTestingExtension(testing) {
@Override
OpenTelemetryMeterRegistryBuilder configureOtelRegistry(
OpenTelemetryMeterRegistryBuilder registry) {
return registry.setMicrometerHistogramGaugesEnabled(true);
}
};

@Override
protected InstrumentationExtension testing() {
return testing;
}
}
Loading