Skip to content

Commit

Permalink
Reject null instrumentationName for Meters and Tracers
Browse files Browse the repository at this point in the history
The parameters were already marked as being @nonnull. However, the SDK implementation failed when null was passed, while the API allowed null. The result is that one may publish a library that compiles and tests fine, but fails when the SDK is applied by an application developer.

This change adds test to verify the non-null behavior is consistently enforced, and adds checks to the API to enforce non-null. These checks will prevent library authors from creating libraries that crash at runtime.

Fixes open-telemetry#1879
  • Loading branch information
MariusVolkhart committed Nov 6, 2020
1 parent 1a7a117 commit d05c042
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.api.metrics;

import javax.annotation.concurrent.ThreadSafe;
import java.util.Objects;

@ThreadSafe
final class DefaultMeterProvider implements MeterProvider {
Expand All @@ -23,6 +24,7 @@ public Meter get(String instrumentationName) {

@Override
public Meter get(String instrumentationName, String instrumentationVersion) {
Objects.requireNonNull(instrumentationName);
return Meter.getDefault();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.api.trace;

import javax.annotation.concurrent.ThreadSafe;
import java.util.Objects;

@ThreadSafe
class DefaultTracerProvider implements TracerProvider {
Expand All @@ -23,6 +24,7 @@ public Tracer get(String instrumentationName) {

@Override
public Tracer get(String instrumentationName, String instrumentationVersion) {
Objects.requireNonNull(instrumentationName);
return Tracer.getDefault();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.opentelemetry.api.metrics;

import static org.junit.jupiter.api.Assertions.assertThrows;

import org.junit.jupiter.api.Test;

class DefaultMeterProviderTest {

@Test
void rejectsNullInstrumentationName() {
assertThrows(NullPointerException.class, () -> DefaultMeterProvider.getInstance().get(null));
assertThrows(NullPointerException.class, () -> DefaultMeterProvider.getInstance().get(null, "1.0.0"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.opentelemetry.api.trace;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertThrows;

class DefaultTracerProviderTest {

@Test
void rejectsNullInstrumentationName() {
assertThrows(NullPointerException.class, () -> DefaultTracerProvider.getInstance().get(null));
assertThrows(NullPointerException.class, () -> DefaultTracerProvider.getInstance().get(null, "1.0.0"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,10 @@ void metricProducer_GetAllMetrics() {
Collections.singletonList(
LongPoint.create(testClock.now(), testClock.now(), Labels.empty(), 10))));
}

@Test
void rejectsNullInstrumentationName() {
assertThrows(NullPointerException.class, () -> meterProvider.get(null));
assertThrows(NullPointerException.class, () -> meterProvider.get(null, "1.0.0"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,10 @@ void returnNoopSpanAfterShutdown() {
assertThat(span.getSpanContext().isValid()).isFalse();
span.end();
}

@Test
void rejectsNullInstrumentationName() {
assertThrows(NullPointerException.class, () -> tracerFactory.get(null));
assertThrows(NullPointerException.class, () -> tracerFactory.get(null, "1.0.0"));
}
}

0 comments on commit d05c042

Please sign in to comment.