Closing a kafka producer/consumer should not disable metrics from other consumers/producers (#11975)

This commit is contained in:
Lauri Tulmin 2024-08-15 01:50:54 +03:00 committed by GitHub
parent f7f7d39c10
commit 31e5eea3bc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 46 additions and 9 deletions

View File

@ -32,6 +32,7 @@ import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.Random; import java.util.Random;
import java.util.Set; import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.kafka.clients.CommonClientConfigs; import org.apache.kafka.clients.CommonClientConfigs;
import org.apache.kafka.clients.consumer.ConsumerConfig; import org.apache.kafka.clients.consumer.ConsumerConfig;
import org.apache.kafka.clients.consumer.KafkaConsumer; import org.apache.kafka.clients.consumer.KafkaConsumer;
@ -70,6 +71,13 @@ public abstract class AbstractOpenTelemetryMetricsReporterTest {
private static KafkaProducer<byte[], byte[]> producer; private static KafkaProducer<byte[], byte[]> producer;
private static KafkaConsumer<byte[], byte[]> consumer; private static KafkaConsumer<byte[], byte[]> consumer;
private static final List<OpenTelemetryMetricsReporter> metricsReporters =
new CopyOnWriteArrayList<>();
static {
OpenTelemetryMetricsReporter.setListener(metricsReporters::add);
}
@BeforeEach @BeforeEach
void beforeAll() { void beforeAll() {
// only start the kafka container the first time this runs // only start the kafka container the first time this runs
@ -90,14 +98,16 @@ public abstract class AbstractOpenTelemetryMetricsReporterTest {
@AfterAll @AfterAll
static void afterAll() { static void afterAll() {
kafka.stop();
producer.close(); producer.close();
consumer.close(); consumer.close();
kafka.stop();
} }
@AfterEach @AfterEach
void tearDown() { void tearDown() {
OpenTelemetryMetricsReporter.resetForTest(); for (OpenTelemetryMetricsReporter metricsReporter : metricsReporters) {
metricsReporter.resetForTest();
}
} }
protected abstract InstrumentationExtension testing(); protected abstract InstrumentationExtension testing();
@ -186,6 +196,14 @@ public abstract class AbstractOpenTelemetryMetricsReporterTest {
@Test @Test
void observeMetrics() { void observeMetrics() {
// Firstly create new producer and consumer and close them. This is done tp verify that metrics
// are still produced after closing one producer/consumer. See
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/11880
KafkaProducer<byte[], byte[]> producer2 = new KafkaProducer<>(producerConfig());
KafkaConsumer<byte[], byte[]> consumer2 = new KafkaConsumer<>(consumerConfig());
producer2.close();
consumer2.close();
produceRecords(); produceRecords();
consumeRecords(); consumeRecords();
@ -405,7 +423,9 @@ public abstract class AbstractOpenTelemetryMetricsReporterTest {
Map<String, List<KafkaMetricId>> kafkaMetricsByGroup = Map<String, List<KafkaMetricId>> kafkaMetricsByGroup =
TestMetricsReporter.seenMetrics.stream().collect(groupingBy(KafkaMetricId::getGroup)); TestMetricsReporter.seenMetrics.stream().collect(groupingBy(KafkaMetricId::getGroup));
List<RegisteredObservable> registeredObservables = List<RegisteredObservable> registeredObservables =
OpenTelemetryMetricsReporter.getRegisteredObservables(); metricsReporters.stream()
.flatMap(metricsReporter -> metricsReporter.getRegisteredObservables().stream())
.collect(toList());
// Iterate through groups in alpha order // Iterate through groups in alpha order
for (String group : kafkaMetricsByGroup.keySet().stream().sorted().collect(toList())) { for (String group : kafkaMetricsByGroup.keySet().stream().sorted().collect(toList())) {
List<KafkaMetricId> kafkaMetricIds = List<KafkaMetricId> kafkaMetricIds =

View File

@ -41,28 +41,35 @@ public final class OpenTelemetryMetricsReporter implements MetricsReporter {
private static final Logger logger = private static final Logger logger =
Logger.getLogger(OpenTelemetryMetricsReporter.class.getName()); Logger.getLogger(OpenTelemetryMetricsReporter.class.getName());
private volatile Meter meter; private static volatile Listener listener;
private static final Object lock = new Object(); private volatile Meter meter;
private final Object lock = new Object();
@GuardedBy("lock") @GuardedBy("lock")
private static final List<RegisteredObservable> registeredObservables = new ArrayList<>(); private final List<RegisteredObservable> registeredObservables = new ArrayList<>();
/** /**
* Reset for test by resetting the {@link #meter} to {@code null} and closing all registered * Reset for test by resetting the {@link #meter} to {@code null} and closing all registered
* instruments. * instruments.
*/ */
static void resetForTest() { void resetForTest() {
closeAllInstruments(); closeAllInstruments();
} }
// Visible for test // Visible for test
static List<RegisteredObservable> getRegisteredObservables() { List<RegisteredObservable> getRegisteredObservables() {
synchronized (lock) { synchronized (lock) {
return new ArrayList<>(registeredObservables); return new ArrayList<>(registeredObservables);
} }
} }
public OpenTelemetryMetricsReporter() {
if (listener != null) {
listener.metricsReporterCreated(this);
}
}
@Override @Override
public void init(List<KafkaMetric> metrics) { public void init(List<KafkaMetric> metrics) {
metrics.forEach(this::metricChange); metrics.forEach(this::metricChange);
@ -131,7 +138,7 @@ public final class OpenTelemetryMetricsReporter implements MetricsReporter {
closeAllInstruments(); closeAllInstruments();
} }
private static void closeAllInstruments() { private void closeAllInstruments() {
synchronized (lock) { synchronized (lock) {
for (Iterator<RegisteredObservable> it = registeredObservables.iterator(); it.hasNext(); ) { for (Iterator<RegisteredObservable> it = registeredObservables.iterator(); it.hasNext(); ) {
closeInstrument(it.next().getObservable()); closeInstrument(it.next().getObservable());
@ -177,4 +184,14 @@ public final class OpenTelemetryMetricsReporter implements MetricsReporter {
} }
return (T) value; return (T) value;
} }
// Visible for test
static void setListener(Listener listener) {
OpenTelemetryMetricsReporter.listener = listener;
}
// used for testing
interface Listener {
void metricsReporterCreated(OpenTelemetryMetricsReporter metricsReporter);
}
} }