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 http and rpc metrics when advice can not be applied #10671

Merged
merged 1 commit into from
Feb 27, 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.
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 @@ -17,6 +17,7 @@
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil;
import java.util.logging.Logger;

/**
Expand All @@ -42,7 +43,8 @@ public final class HttpClientExperimentalMetrics implements OperationListener {
* io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder}.
*/
public static OperationMetrics get() {
return HttpClientExperimentalMetrics::new;
return OperationMetricsUtil.create(
"experimental http client", HttpClientExperimentalMetrics::new);
}

private final LongHistogram requestSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil;
import java.util.logging.Logger;

/**
Expand Down Expand Up @@ -46,7 +47,8 @@ public final class HttpServerExperimentalMetrics implements OperationListener {
* io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder}.
*/
public static OperationMetrics get() {
return HttpServerExperimentalMetrics::new;
return OperationMetricsUtil.create(
"experimental http server", HttpServerExperimentalMetrics::new);
}

private final LongUpDownCounter activeRequests;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;

Expand Down Expand Up @@ -51,7 +52,7 @@ private RpcClientMetrics(Meter meter) {
* io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder}.
*/
public static OperationMetrics get() {
return RpcClientMetrics::new;
return OperationMetricsUtil.create("rpc client", RpcClientMetrics::new);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;

Expand Down Expand Up @@ -51,7 +52,7 @@ private RpcServerMetrics(Meter meter) {
* io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder}.
*/
public static OperationMetrics get() {
return RpcServerMetrics::new;
return OperationMetricsUtil.create("rpc server", RpcServerMetrics::new);
}

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

package io.opentelemetry.instrumentation.api.internal;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.context.Context;
import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import java.util.function.BiConsumer;
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public class OperationMetricsUtil {
private static final Logger logger = Logger.getLogger(OperationMetricsUtil.class.getName());
private static final OperationListener NOOP_OPERATION_LISTENER =
new OperationListener() {

@Override
public Context onStart(Context context, Attributes startAttributes, long startNanos) {
return context;
}

@Override
public void onEnd(Context context, Attributes endAttributes, long endNanos) {}
};

public static OperationMetrics create(
String description, Function<Meter, OperationListener> factory) {
return create(
description,
factory,
(s, histogramBuilder) ->
logger.log(
Level.WARNING,
"Disabling {0} metrics because {1} does not implement {2}. This prevents using "
+ "metrics advice, which could result in {0} having high cardinality attributes.",
new Object[] {
description,
histogramBuilder.getClass().getName(),
ExtendedDoubleHistogramBuilder.class.getName()
}));
}

// visible for testing
static OperationMetrics create(
String description,
Function<Meter, OperationListener> factory,
BiConsumer<String, DoubleHistogramBuilder> warningEmitter) {
return meter -> {
DoubleHistogramBuilder histogramBuilder = meter.histogramBuilder("compatibility-test");
if (!(histogramBuilder instanceof ExtendedDoubleHistogramBuilder)
&& !histogramBuilder.getClass().getName().contains("NoopDoubleHistogram")) {
warningEmitter.accept(description, histogramBuilder);
return NOOP_OPERATION_LISTENER;
}
return factory.apply(meter);
};
}

private OperationMetricsUtil() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;

Expand All @@ -43,7 +44,7 @@ public final class HttpClientMetrics implements OperationListener {
* @see InstrumenterBuilder#addOperationMetrics(OperationMetrics)
*/
public static OperationMetrics get() {
return HttpClientMetrics::new;
return OperationMetricsUtil.create("http client", HttpClientMetrics::new);
}

private final DoubleHistogram duration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
import io.opentelemetry.instrumentation.api.instrumenter.OperationListener;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;

Expand All @@ -43,7 +44,7 @@ public final class HttpServerMetrics implements OperationListener {
* @see InstrumenterBuilder#addOperationMetrics(OperationMetrics)
*/
public static OperationMetrics get() {
return HttpServerMetrics::new;
return OperationMetricsUtil.create("http server", HttpServerMetrics::new);
}

private final DoubleHistogram duration;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.internal;

import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
import java.lang.reflect.Proxy;
import java.util.concurrent.atomic.AtomicBoolean;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

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

@Test
void noWarning() {
AtomicBoolean warning = new AtomicBoolean(false);
OperationMetrics operationMetrics =
OperationMetricsUtil.create(
"test metrics", meter -> null, (s, doubleHistogramBuilder) -> warning.set(true));
operationMetrics.create(testing.getOpenTelemetry().getMeter("test"));

Assertions.assertThat(warning).isFalse();
}

@Test
void noWarningWithNoopMetrics() {
AtomicBoolean warning = new AtomicBoolean(false);
OperationMetrics operationMetrics =
OperationMetricsUtil.create(
"test metrics", meter -> null, (s, doubleHistogramBuilder) -> warning.set(true));
operationMetrics.create(MeterProvider.noop().get("test"));

Assertions.assertThat(warning).isFalse();
}

@Test
void warning() {
AtomicBoolean warning = new AtomicBoolean(false);
OperationMetrics operationMetrics =
OperationMetricsUtil.create(
"test metrics", meter -> null, (s, doubleHistogramBuilder) -> warning.set(true));
Meter defaultMeter = MeterProvider.noop().get("test");
Meter meter =
(Meter)
Proxy.newProxyInstance(
Meter.class.getClassLoader(),
new Class<?>[] {Meter.class},
(proxy, method, args) -> {
if ("histogramBuilder".equals(method.getName())) {
// proxy the histogram builder so that the builder instance does not implement
// ExtendedDoubleHistogramBuilder which will trigger the warning
return proxyDoubleHistogramBuilder(defaultMeter);
}
return method.invoke(defaultMeter, args);
});
operationMetrics.create(meter);

Assertions.assertThat(warning).isTrue();
}

private static DoubleHistogramBuilder proxyDoubleHistogramBuilder(Meter meter) {
return (DoubleHistogramBuilder)
Proxy.newProxyInstance(
DoubleHistogramBuilder.class.getClassLoader(),
new Class<?>[] {DoubleHistogramBuilder.class},
(proxy1, method1, args1) -> meter.histogramBuilder((String) args1[0]));
}
}
Loading