Fix Otlp*MetricExporterBuilderTests#7313
Conversation
| MeterProvider meterProvider = meterProviderSupplier.get(); | ||
| if (meterProvider == null) { | ||
| meterProvider = MeterProvider.noop(); | ||
| } |
There was a problem hiding this comment.
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.
| void setMeterProvider() { | ||
| when(meterProvider.get(any())).thenReturn(meter); | ||
| when(meter.counterBuilder(eq("otlp.exporter.seen"))).thenReturn(counterBuilder); | ||
| when(meter.counterBuilder(any())).thenReturn(counterBuilder); |
There was a problem hiding this comment.
ExporterMetrics obtains / increments multiple counters. Only accounting for one of them was causing NPE.
| // 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); |
There was a problem hiding this comment.
The test wasn't waiting for the export to finish / fail, making it difficult to track down which test was responsible.
| OtlpHttpMetricExporter.builder().setMeterProvider(meterProvider).build()) { | ||
| OtlpHttpMetricExporter.builder() | ||
| .setMeterProvider(meterProvider) | ||
| .setEndpoint("http://localhost:" + server.httpPort() + "/v1/metrics") |
There was a problem hiding this comment.
Need to send the data to a real OTLP receiver.
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (50.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7313 +/- ##
============================================
- Coverage 90.03% 90.02% -0.01%
Complexity 6915 6915
============================================
Files 787 787
Lines 20864 20865 +1
Branches 2023 2024 +1
============================================
- Hits 18784 18783 -1
- Misses 1441 1442 +1
- Partials 639 640 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
trask
left a comment
trask
left a comment
There was a problem hiding this comment.
(thanks for reviewing @jaydeluca)
3 participants
I ran the build locally for the first time after merging #7255 and noticed a variety of issues:
Fixing things up here.