Skip to content

Fix Otlp*MetricExporterBuilderTests #7313

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.

Sign up for GitHub

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

Merged
jack-berg merged 2 commits into from
May 6, 2025
+136 −41
Merged

Fix Otlp*MetricExporterBuilderTests #7313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62484da
Fix Otlp*MetricExporterBuilderTests
jack-berg May 1, 2025
9d4bde1
Don't depend on grpc stubs
jack-berg May 1, 2025
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
8 changes: 5 additions & 3 deletions exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,11 @@
}

private Meter meter() {
return meterProviderSupplier
.get()
.get("io.opentelemetry.exporters." + exporterName + "-" + transportName);
MeterProvider meterProvider = meterProviderSupplier.get();
if (meterProvider == null) {
meterProvider = MeterProvider.noop();

Check warning on line 92 in exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterMetrics.java

View check run for this annotation

Codecov / codecov/patch

exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterMetrics.java#L92 

Added line #L92 was not covered by tests
}
Copy link
Member Author

@jack-berg jack-berg May 1, 2025

Choose a reason for hiding this comment

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

This is unrelated, but I noticed that we don't protect against Supplier<MeterProvider> returning null. This is one of the edge cases which nullaway doesn't catch.

return meterProvider.get("io.opentelemetry.exporters." + exporterName + "-" + transportName);
}

private static boolean isNoop(LongCounter counter) {
Expand Down
1 change: 1 addition & 0 deletions exporters/otlp/all/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ dependencies {

testImplementation(project(":exporters:otlp:testing-internal"))
testImplementation("com.linecorp.armeria:armeria-junit5")
testImplementation("com.linecorp.armeria:armeria-grpc-protocol")
testImplementation("io.grpc:grpc-stub")

jmhImplementation(project(":sdk:testing"))
Expand Down
77 changes: 58 additions & 19 deletions ...t/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.metrics.LongCounter;
Expand All @@ -36,9 +41,13 @@
import io.opentelemetry.sdk.metrics.internal.data.ImmutableGaugeData;
import io.opentelemetry.sdk.resources.Resource;
import java.util.Collection;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.Mockito;

class OtlpHttpMetricExporterBuilderTest {
Expand All @@ -53,11 +62,31 @@ class OtlpHttpMetricExporterBuilderTest {
"test",
ImmutableGaugeData.empty()));

@RegisterExtension
private static final ServerExtension server =
new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
sb.service("/v1/metrics", (ctx, req) -> HttpResponse.of(HttpStatus.OK));
sb.http(0);
}
};

private final SdkMeterProvider meterProvider = mock(SdkMeterProvider.class);
private final Meter meter = mock(Meter.class);
private final LongCounterBuilder counterBuilder = mock(LongCounterBuilder.class);
private final LongCounter counter = mock(LongCounter.class);

@BeforeEach
void setup() {
GlobalOpenTelemetry.resetForTest();
}

@AfterEach
void cleanup() {
GlobalOpenTelemetry.resetForTest();
}

@Test
void setMeterProvider_null() {
OtlpHttpMetricExporterBuilder builder = OtlpHttpMetricExporter.builder();
Expand All @@ -74,53 +103,61 @@ void setMeterProvider_null() {
@Test
void setMeterProvider() {
when(meterProvider.get(any())).thenReturn(meter);
when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder);
when(meter.counterBuilder(any())).thenReturn(counterBuilder);
Copy link
Member Author

@jack-berg jack-berg May 1, 2025

Choose a reason for hiding this comment

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

ExporterMetrics obtains / increments multiple counters. Only accounting for one of them was causing NPE.

when(counterBuilder.build()).thenReturn(counter);

try (OtlpHttpMetricExporter exporter =
OtlpHttpMetricExporter.builder().setMeterProvider(meterProvider).build()) {
OtlpHttpMetricExporter.builder()
.setMeterProvider(meterProvider)
.setEndpoint("http://localhost:" + server.httpPort() + "/v1/metrics")
Copy link
Member Author

@jack-berg jack-berg May 1, 2025

Choose a reason for hiding this comment

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

Need to send the data to a real OTLP receiver.

.build()) {
verifyNoInteractions(meterProvider, meter, counterBuilder, counter);

// Collection before MeterProvider is initialized.
when(meterProvider.get(any())).thenReturn(MeterProvider.noop().get("test"));
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);
Copy link
Member Author

@jack-berg jack-berg May 1, 2025

Choose a reason for hiding this comment

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

The test wasn't waiting for the export to finish / fail, making it difficult to track down which test was responsible.


verifyNoInteractions(meter, counterBuilder, counter);

// Collection after MeterProvider is initialized.
when(meterProvider.get(any())).thenReturn(meter);
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verify(meter).counterBuilder(eq("otlp.exporter.seen"));
verify(counter).add(eq(1L), any());
verify(meter).counterBuilder("otlp.exporter.seen");
verify(meter).counterBuilder("otlp.exporter.exported");
verify(counter, times(2)).add(eq(1L), any());
verifyNoMoreInteractions(meter, counter);
}
}

@Test
void setMeterProvider_supplier() {
when(meterProvider.get(any())).thenReturn(meter);
when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder);
when(meter.counterBuilder(any())).thenReturn(counterBuilder);
when(counterBuilder.build()).thenReturn(counter);

@SuppressWarnings("unchecked")
Supplier<MeterProvider> provider = mock(Supplier.class);
try (OtlpHttpMetricExporter exporter =
OtlpHttpMetricExporter.builder().setMeterProvider(provider).build()) {
OtlpHttpMetricExporter.builder()
.setMeterProvider(provider)
.setEndpoint("http://localhost:" + server.httpPort() + "/v1/metrics")
.build()) {
verifyNoInteractions(provider, meterProvider, meter, counterBuilder, counter);

// Collection before MeterProvider is initialized.
when(provider.get()).thenReturn(MeterProvider.noop());
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verifyNoInteractions(meterProvider, meter, counterBuilder, counter);

// Collection after MeterProvider is initialized.
when(provider.get()).thenReturn(meterProvider);
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verify(meter).counterBuilder(eq("otlp.exporter.seen"));
verify(counter).add(eq(1L), any());
verify(meter).counterBuilder("otlp.exporter.seen");
verify(meter).counterBuilder("otlp.exporter.exported");
verify(counter, times(2)).add(eq(1L), any());
verifyNoMoreInteractions(meter, counter);
}
}
Expand All @@ -145,19 +182,21 @@ public ContextPropagators getPropagators() {
}
});
when(meterProvider.get(any())).thenReturn(meter);
when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder);
when(meter.counterBuilder(any())).thenReturn(counterBuilder);
when(counterBuilder.build()).thenReturn(counter);

try (OtlpHttpMetricExporter exporter = OtlpHttpMetricExporter.builder().build()) {
try (OtlpHttpMetricExporter exporter =
OtlpHttpMetricExporter.builder()
.setEndpoint("http://localhost:" + server.httpPort() + "/v1/metrics")
.build()) {
verifyNoInteractions(meterProvider, meter, counterBuilder, counter);

exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verify(meter).counterBuilder(eq("otlp.exporter.seen"));
verify(counter).add(eq(1L), any());
verify(meter).counterBuilder("otlp.exporter.seen");
verify(meter).counterBuilder("otlp.exporter.exported");
verify(counter, times(2)).add(eq(1L), any());
verifyNoMoreInteractions(meter, counter);
} finally {
GlobalOpenTelemetry.resetForTest();
}
}

Expand Down
91 changes: 72 additions & 19 deletions ...c/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,17 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.grpc.protocol.AbstractUnaryGrpcService;
import com.linecorp.armeria.server.logging.LoggingService;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.metrics.LongCounter;
Expand All @@ -25,6 +31,7 @@
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse;
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
import io.opentelemetry.sdk.common.export.MemoryMode;
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
Expand All @@ -36,9 +43,15 @@
import io.opentelemetry.sdk.metrics.internal.data.ImmutableGaugeData;
import io.opentelemetry.sdk.resources.Resource;
import java.util.Collection;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.Mockito;

class OtlpGrpcMetricExporterBuilderTest {
Expand All @@ -53,11 +66,41 @@ class OtlpGrpcMetricExporterBuilderTest {
"test",
ImmutableGaugeData.empty()));

@RegisterExtension
private static final ServerExtension server =
new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
sb.service(
"/opentelemetry.proto.collector.metrics.v1.MetricsService/Export",
new AbstractUnaryGrpcService() {
@Override
protected CompletionStage<byte[]> handleMessage(
ServiceRequestContext ctx, byte[] message) {
return CompletableFuture.completedFuture(
ExportMetricsServiceResponse.getDefaultInstance().toByteArray());
}
});
sb.http(0);
sb.decorator(LoggingService.newDecorator());
}
};

private final MeterProvider meterProvider = mock(MeterProvider.class);
private final Meter meter = mock(Meter.class);
private final LongCounterBuilder counterBuilder = mock(LongCounterBuilder.class);
private final LongCounter counter = mock(LongCounter.class);

@BeforeEach
void setup() {
GlobalOpenTelemetry.resetForTest();
}

@AfterEach
void cleanup() {
GlobalOpenTelemetry.resetForTest();
}

@Test
void setMeterProvider_null() {
OtlpGrpcMetricExporterBuilder builder = OtlpGrpcMetricExporter.builder();
Expand All @@ -74,53 +117,61 @@ void setMeterProvider_null() {
@Test
void setMeterProvider() {
when(meterProvider.get(any())).thenReturn(meter);
when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder);
when(meter.counterBuilder(any())).thenReturn(counterBuilder);
when(counterBuilder.build()).thenReturn(counter);

try (OtlpGrpcMetricExporter exporter =
OtlpGrpcMetricExporter.builder().setMeterProvider(meterProvider).build()) {
OtlpGrpcMetricExporter.builder()
.setMeterProvider(meterProvider)
.setEndpoint("http://localhost:" + server.httpPort())
.build()) {
verifyNoInteractions(meterProvider, meter, counterBuilder, counter);

// Collection before MeterProvider is initialized.
when(meterProvider.get(any())).thenReturn(MeterProvider.noop().get("test"));
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verifyNoInteractions(meter, counterBuilder, counter);

// Collection after MeterProvider is initialized.
when(meterProvider.get(any())).thenReturn(meter);
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verify(meter).counterBuilder(eq("otlp.exporter.seen"));
verify(counter).add(eq(1L), any());
verify(meter).counterBuilder("otlp.exporter.seen");
verify(meter).counterBuilder("otlp.exporter.exported");
verify(counter, times(2)).add(eq(1L), any());
verifyNoMoreInteractions(meter, counter);
}
}

@Test
void setMeterProvider_supplier() {
when(meterProvider.get(any())).thenReturn(meter);
when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder);
when(meter.counterBuilder(any())).thenReturn(counterBuilder);
when(counterBuilder.build()).thenReturn(counter);

@SuppressWarnings("unchecked")
Supplier<MeterProvider> provider = mock(Supplier.class);
try (OtlpGrpcMetricExporter exporter =
OtlpGrpcMetricExporter.builder().setMeterProvider(provider).build()) {
OtlpGrpcMetricExporter.builder()
.setMeterProvider(provider)
.setEndpoint("http://localhost:" + server.httpPort())
.build()) {
verifyNoInteractions(provider, meterProvider, meter, counterBuilder, counter);

// Collection before MeterProvider is initialized.
when(provider.get()).thenReturn(MeterProvider.noop());
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verifyNoInteractions(meterProvider, meter, counterBuilder, counter);

// Collection after MeterProvider is initialized.
when(provider.get()).thenReturn(meterProvider);
exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verify(meter).counterBuilder(eq("otlp.exporter.seen"));
verify(counter).add(eq(1L), any());
verify(meter).counterBuilder("otlp.exporter.seen");
verify(meter).counterBuilder("otlp.exporter.exported");
verify(counter, times(2)).add(eq(1L), any());
verifyNoMoreInteractions(meter, counter);
}
}
Expand All @@ -145,19 +196,21 @@ public ContextPropagators getPropagators() {
}
});
when(meterProvider.get(any())).thenReturn(meter);
when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder);
when(meter.counterBuilder(any())).thenReturn(counterBuilder);
when(counterBuilder.build()).thenReturn(counter);

try (OtlpGrpcMetricExporter exporter = OtlpGrpcMetricExporter.builder().build()) {
try (OtlpGrpcMetricExporter exporter =
OtlpGrpcMetricExporter.builder()
.setEndpoint("http://localhost:" + server.httpPort())
.build()) {
verifyNoInteractions(meterProvider, meter, counterBuilder, counter);

exporter.export(DATA_SET);
exporter.export(DATA_SET).join(10, TimeUnit.SECONDS);

verify(meter).counterBuilder(eq("otlp.exporter.seen"));
verify(counter).add(eq(1L), any());
verify(meter).counterBuilder("otlp.exporter.seen");
verify(meter).counterBuilder("otlp.exporter.exported");
verify(counter, times(2)).add(eq(1L), any());
verifyNoMoreInteractions(meter, counter);
} finally {
GlobalOpenTelemetry.resetForTest();
}
}

Expand Down
Loading