Dismantle AbstractInstrumentBuilder inheritance hierarchy

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java → sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/InstrumentBuilder.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.sdk.metrics;
 
+import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
 import io.opentelemetry.api.metrics.ObservableLongMeasurement;
 import io.opentelemetry.sdk.metrics.internal.descriptor.Advice;
@@ -15,95 +16,72 @@
 import io.opentelemetry.sdk.metrics.internal.state.SdkObservableMeasurement;
 import io.opentelemetry.sdk.metrics.internal.state.WriteableMetricStorage;
 import java.util.Collections;
+import java.util.List;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
+import javax.annotation.Nullable;
 
 /** Helper to make implementing builders easier. */
-abstract class AbstractInstrumentBuilder<BuilderT extends AbstractInstrumentBuilder<?>> {
-
-  static final String DEFAULT_UNIT = "";
+final class InstrumentBuilder {
 
+  private final String name;
   private final MeterProviderSharedState meterProviderSharedState;
-  private final InstrumentType type;
-  private final InstrumentValueType valueType;
-  private String description;
-  private String unit;
-
-  protected final MeterSharedState meterSharedState;
-  protected final String instrumentName;
-  protected final Advice.AdviceBuilder adviceBuilder;
-
-  AbstractInstrumentBuilder(
-      MeterProviderSharedState meterProviderSharedState,
-      MeterSharedState meterSharedState,
-      InstrumentType type,
-      InstrumentValueType valueType,
+  private final MeterSharedState meterSharedState;
+  @Nullable private InstrumentType type = null;
+  @Nullable private InstrumentValueType valueType;
+  private Advice.AdviceBuilder adviceBuilder = Advice.builder();
+  private String description = "";
+  private String unit = "";
+
+  InstrumentBuilder(
       String name,
-      String description,
-      String unit) {
-    this(
-        meterProviderSharedState,
-        meterSharedState,
-        type,
-        valueType,
-        name,
-        description,
-        unit,
-        Advice.builder());
-  }
-
-  AbstractInstrumentBuilder(
       MeterProviderSharedState meterProviderSharedState,
-      MeterSharedState meterSharedState,
-      InstrumentType type,
-      InstrumentValueType valueType,
-      String name,
-      String description,
-      String unit,
-      Advice.AdviceBuilder adviceBuilder) {
-    this.type = type;
-    this.valueType = valueType;
-    this.instrumentName = name;
-    this.description = description;
-    this.unit = unit;
+      MeterSharedState meterSharedState) {
+    this.name = name;
     this.meterProviderSharedState = meterProviderSharedState;
     this.meterSharedState = meterSharedState;
-    this.adviceBuilder = adviceBuilder;
   }
 
-  protected abstract BuilderT getThis();
-
-  public BuilderT setUnit(String unit) {
+  public InstrumentBuilder setUnit(String unit) {
     this.unit = unit;
-    return getThis();
+    return this;
+  }
+
+  public InstrumentBuilder setType(InstrumentType type) {
+    this.type = type;
+    return this;
   }
 
-  public BuilderT setDescription(String description) {
+  public InstrumentBuilder setValueType(InstrumentValueType valueType) {
+    this.valueType = valueType;
+    return this;
+  }
+
+  public InstrumentBuilder setAdviceBuilder(Advice.AdviceBuilder adviceBuilder) {
+    this.adviceBuilder = adviceBuilder;
+    return this;
+  }
+
+  public InstrumentBuilder setDescription(String description) {
     this.description = description;
-    return getThis();
+    return this;
   }
 
-  protected <T> T swapBuilder(SwapBuilder<T> swapper) {
+  <T> T swapBuilder(SwapBuilder<T> swapper) {
     return swapper.newBuilder(
-        meterProviderSharedState,
-        meterSharedState,
-        instrumentName,
-        description,
-        unit,
-        adviceBuilder);
+        meterProviderSharedState, meterSharedState, name, description, unit, adviceBuilder);
   }
 
-  final <I extends AbstractInstrument> I buildSynchronousInstrument(
+  <I extends AbstractInstrument> I buildSynchronousInstrument(
       BiFunction<InstrumentDescriptor, WriteableMetricStorage, I> instrumentFactory) {
-    InstrumentDescriptor descriptor =
-        InstrumentDescriptor.create(
-            instrumentName, description, unit, type, valueType, adviceBuilder.build());
+    validateType();
+    InstrumentDescriptor descriptor = newDescriptor();
     WriteableMetricStorage storage =
         meterSharedState.registerSynchronousMetricStorage(descriptor, meterProviderSharedState);
     return instrumentFactory.apply(descriptor, storage);
   }
 
-  final SdkObservableInstrument registerDoubleAsynchronousInstrument(
+  SdkObservableInstrument buildDoubleAsynchronousInstrument(
       InstrumentType type, Consumer<ObservableDoubleMeasurement> updater) {
     SdkObservableMeasurement sdkObservableMeasurement = buildObservableMeasurement(type);
     Runnable runnable = () -> updater.accept(sdkObservableMeasurement);
@@ -113,7 +91,7 @@
     return new SdkObservableInstrument(meterSharedState, callbackRegistration);
   }
 
-  final SdkObservableInstrument registerLongAsynchronousInstrument(
+  SdkObservableInstrument buildLongAsynchronousInstrument(
       InstrumentType type, Consumer<ObservableLongMeasurement> updater) {
     SdkObservableMeasurement sdkObservableMeasurement = buildObservableMeasurement(type);
     Runnable runnable = () -> updater.accept(sdkObservableMeasurement);
@@ -123,20 +101,36 @@
     return new SdkObservableInstrument(meterSharedState, callbackRegistration);
   }
 
-  final SdkObservableMeasurement buildObservableMeasurement(InstrumentType type) {
-    InstrumentDescriptor descriptor =
-        InstrumentDescriptor.create(
-            instrumentName, description, unit, type, valueType, adviceBuilder.build());
+  SdkObservableMeasurement buildObservableMeasurement(InstrumentType type) {
+    this.type = type;
+    validateType();
+    InstrumentDescriptor descriptor = newDescriptor();
     return meterSharedState.registerObservableMeasurement(descriptor);
   }
 
+  @SuppressWarnings("NullAway")
+  private InstrumentDescriptor newDescriptor() {
+    validateType();
+    return InstrumentDescriptor.create(
+        name, description, unit, type, valueType, adviceBuilder.build());
+  }
+
+  private void validateType() {
+    if (type == null) {
+      throw new IllegalStateException("The instrument type must be configured before build().");
+    }
+    if (valueType == null) {
+      throw new IllegalStateException("The valueType must be configured before build().");
+    }
+  }
+
   @Override
   public String toString() {
-    return this.getClass().getSimpleName()
-        + "{descriptor="
-        + InstrumentDescriptor.create(
-            instrumentName, description, unit, type, valueType, adviceBuilder.build())
-        + "}";
+    return toStringHelper(getClass().getSimpleName());
+  }
+
+  String toStringHelper(String className) {
+    return className + "{descriptor=" + newDescriptor() + "}";
   }
 
   @FunctionalInterface
@@ -149,4 +143,12 @@
         String unit,
         Advice.AdviceBuilder adviceBuilder);
   }
+
+  void setAdviceAttributes(List<AttributeKey<?>> attributes) {
+    adviceBuilder.setAttributes(attributes);
+  }
+
+  void setExplicitBucketBoundaries(List<Double> bucketBoundaries) {
+    adviceBuilder.setExplicitBucketBoundaries(bucketBoundaries);
+  }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleCounter.java

@@ -8,6 +8,7 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.metrics.DoubleCounter;
+import io.opentelemetry.api.metrics.DoubleCounterBuilder;
 import io.opentelemetry.api.metrics.ObservableDoubleCounter;
 import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
 import io.opentelemetry.context.Context;
@@ -57,9 +58,9 @@
     add(increment, Attributes.empty());
   }
 
-  static final class SdkDoubleCounterBuilder
-      extends AbstractInstrumentBuilder<SdkDoubleCounterBuilder>
-      implements ExtendedDoubleCounterBuilder {
+  static final class SdkDoubleCounterBuilder implements ExtendedDoubleCounterBuilder {
+
+    private final InstrumentBuilder builder;
 
     SdkDoubleCounterBuilder(
         MeterProviderSharedState meterProviderSharedState,
@@ -68,42 +69,52 @@
         String description,
         String unit,
         Advice.AdviceBuilder adviceBuilder) {
-      super(
-          meterProviderSharedState,
-          sharedState,
-          InstrumentType.COUNTER,
-          InstrumentValueType.DOUBLE,
-          name,
-          description,
-          unit,
-          adviceBuilder);
+      this.builder =
+          new InstrumentBuilder(name, meterProviderSharedState, sharedState)
+              .setType(InstrumentType.COUNTER)
+              .setValueType(InstrumentValueType.DOUBLE)
+              .setUnit(unit)
+              .setDescription(description)
+              .setAdviceBuilder(adviceBuilder);
+    }
+
+    @Override
+    public SdkDoubleCounter build() {
+      return builder.buildSynchronousInstrument(SdkDoubleCounter::new);
     }
 
     @Override
-    protected SdkDoubleCounterBuilder getThis() {
+    public DoubleCounterBuilder setDescription(String description) {
+      builder.setDescription(description);
       return this;
     }
 
     @Override
-    public SdkDoubleCounter build() {
-      return buildSynchronousInstrument(SdkDoubleCounter::new);
+    public DoubleCounterBuilder setUnit(String unit) {
+      builder.setUnit(unit);
+      return this;
     }
 
     @Override
     public ObservableDoubleCounter buildWithCallback(
         Consumer<ObservableDoubleMeasurement> callback) {
-      return registerDoubleAsynchronousInstrument(InstrumentType.OBSERVABLE_COUNTER, callback);
+      return builder.buildDoubleAsynchronousInstrument(InstrumentType.OBSERVABLE_COUNTER, callback);
     }
 
     @Override
     public ObservableDoubleMeasurement buildObserver() {
-      return buildObservableMeasurement(InstrumentType.OBSERVABLE_COUNTER);
+      return builder.buildObservableMeasurement(InstrumentType.OBSERVABLE_COUNTER);
     }
 
     @Override
     public ExtendedDoubleCounterBuilder setAttributesAdvice(List<AttributeKey<?>> attributes) {
-      adviceBuilder.setAttributes(attributes);
+      builder.setAdviceAttributes(attributes);
       return this;
     }
+
+    @Override
+    public String toString() {
+      return builder.toStringHelper(getClass().getSimpleName());
+    }
   }
 }

sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleGauge.java

@@ -7,6 +7,7 @@
 
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.metrics.DoubleGaugeBuilder;
 import io.opentelemetry.api.metrics.LongGaugeBuilder;
 import io.opentelemetry.api.metrics.ObservableDoubleGauge;
 import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
@@ -39,55 +40,64 @@
     set(increment, Attributes.empty());
   }
 
-  static final class SdkDoubleGaugeBuilder extends AbstractInstrumentBuilder<SdkDoubleGaugeBuilder>
-      implements ExtendedDoubleGaugeBuilder {
+  static final class SdkDoubleGaugeBuilder implements ExtendedDoubleGaugeBuilder {
+    private final InstrumentBuilder builder;
 
     SdkDoubleGaugeBuilder(
         MeterProviderSharedState meterProviderSharedState,
         MeterSharedState meterSharedState,
         String name) {
-      super(
-          meterProviderSharedState,
-          meterSharedState,
-          // TODO: use InstrumentType.GAUGE when available
-          InstrumentType.OBSERVABLE_GAUGE,
-          InstrumentValueType.DOUBLE,
-          name,
-          "",
-          DEFAULT_UNIT);
+
+      builder =
+          new InstrumentBuilder(name, meterProviderSharedState, meterSharedState)
+              // TODO: use InstrumentType.GAUGE when available
+              .setType(InstrumentType.OBSERVABLE_GAUGE)
+              .setValueType(InstrumentValueType.DOUBLE);
+    }
+
+    @Override
+    public DoubleGaugeBuilder setDescription(String description) {
+      builder.setDescription(description);
+      return this;
     }
 
     @Override
-    protected SdkDoubleGaugeBuilder getThis() {
+    public DoubleGaugeBuilder setUnit(String unit) {
+      builder.setUnit(unit);
       return this;
     }
 
     @Override
     public SdkDoubleGauge build() {
-      return buildSynchronousInstrument(SdkDoubleGauge::new);
+      return builder.buildSynchronousInstrument(SdkDoubleGauge::new);
     }
 
     @Override
     public ExtendedDoubleGaugeBuilder setAttributesAdvice(List<AttributeKey<?>> attributes) {
-      adviceBuilder.setAttributes(attributes);
+      builder.setAdviceAttributes(attributes);
       return this;
     }
 
     @Override
     public LongGaugeBuilder ofLongs() {
-      return swapBuilder(SdkLongGauge.SdkLongGaugeBuilder::new);
+      return builder.swapBuilder(SdkLongGauge.SdkLongGaugeBuilder::new);
     }
 
     @Override
     public ObservableDoubleGauge buildWithCallback(Consumer<ObservableDoubleMeasurement> callback) {
       // TODO: use InstrumentType.GAUGE when available
-      return registerDoubleAsynchronousInstrument(InstrumentType.OBSERVABLE_GAUGE, callback);
+      return builder.buildDoubleAsynchronousInstrument(InstrumentType.OBSERVABLE_GAUGE, callback);
     }
 
     @Override
     public ObservableDoubleMeasurement buildObserver() {
       // TODO: use InstrumentType.GAUGE when available
-      return buildObservableMeasurement(InstrumentType.OBSERVABLE_GAUGE);
+      return builder.buildObservableMeasurement(InstrumentType.OBSERVABLE_GAUGE);
+    }
+
+    @Override
+    public String toString() {
+      return builder.toStringHelper(getClass().getSimpleName());
     }
   }
 }