Consolidate request listener lists (#5667)

open-telemetry · Mar 23, 2022 · bb677a0 · bb677a0
1 parent f2587ba
commit bb677a0
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 24 deletions.
diff --git a/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java
@@ -18,6 +18,7 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
 
@@ -100,7 +101,6 @@ public static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> builder
  attributesExtractors;
  private final List<? extends ContextCustomizer<? super REQUEST>> contextCustomizers;
  private final List<? extends RequestListener> requestListeners;
- private final List<? extends RequestListener> requestMetricListeners;
  private final ErrorCauseExtractor errorCauseExtractor;
  @Nullable private final TimeExtractor<REQUEST, RESPONSE> timeExtractor;
  private final boolean disabled;
@@ -117,7 +117,6 @@ public static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> builder
  this.attributesExtractors = new ArrayList<>(builder.attributesExtractors);
  this.contextCustomizers = new ArrayList<>(builder.contextCustomizers);
  this.requestListeners = new ArrayList<>(builder.requestListeners);
- this.requestMetricListeners = new ArrayList<>(builder.requestMetricListeners);
  this.errorCauseExtractor = builder.errorCauseExtractor;
  this.timeExtractor = builder.timeExtractor;
  this.disabled = builder.disabled;
@@ -177,6 +176,10 @@ public Context start(Context parentContext, REQUEST request) {
 
  Context context = parentContext;
 
+ spanBuilder.setAllAttributes(attributes);
+ Span span = spanBuilder.startSpan();
+ context = context.with(span);
+
  for (ContextCustomizer<? super REQUEST> contextCustomizer : contextCustomizers) {
  context = contextCustomizer.start(context, request, attributes);
  }
@@ -188,19 +191,6 @@ public Context start(Context parentContext, REQUEST request) {
  }
  }
 
- spanBuilder.setAllAttributes(attributes);
- Span span = spanBuilder.startSpan();
- context = context.with(span);
-
- // request metric listeners need to run after the span has been added to the context in order
- // for them to generate exemplars
- if (!requestMetricListeners.isEmpty()) {
- long startNanos = getNanos(startTime);
- for (RequestListener requestListener : requestMetricListeners) {
- context = requestListener.start(context, attributes, startNanos);
- }
- }
-
  return spanSuppressionStrategy.storeInContext(context, spanKind, span);
  }
 
@@ -230,14 +220,12 @@ public void end(
  endTime = timeExtractor.extractEndTime(request, response, error);
  }
 
- if (!requestListeners.isEmpty() || !requestMetricListeners.isEmpty()) {
+ if (!requestListeners.isEmpty()) {
  long endNanos = getNanos(endTime);
- // TODO (trask) call end in the reverse order that start was called?
- for (RequestListener requestListener : requestMetricListeners) {
- requestListener.end(context, attributes, endNanos);
- }
- for (RequestListener requestListener : requestListeners) {
- requestListener.end(context, attributes, endNanos);
+ ListIterator<? extends RequestListener> i =
+ requestListeners.listIterator(requestListeners.size());
+ while (i.hasPrevious()) {
+ i.previous().end(context, attributes, endNanos);
  }
  }
 

diff --git a/.../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java b/.../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java
@@ -53,7 +53,6 @@ public final class InstrumenterBuilder<REQUEST, RESPONSE> {
  new ArrayList<>();
  final List<ContextCustomizer<? super REQUEST>> contextCustomizers = new ArrayList<>();
  final List<RequestListener> requestListeners = new ArrayList<>();
- final List<RequestListener> requestMetricListeners = new ArrayList<>();
 
  SpanKindExtractor<? super REQUEST> spanKindExtractor = SpanKindExtractor.alwaysInternal();
  SpanStatusExtractor<? super REQUEST, ? super RESPONSE> spanStatusExtractor =
@@ -134,7 +133,7 @@ public InstrumenterBuilder<REQUEST, RESPONSE> addRequestListener(RequestListener
  /** Adds a {@link RequestMetrics} whose metrics will be recorded for request start and end. */
  @UnstableApi
  public InstrumenterBuilder<REQUEST, RESPONSE> addRequestMetrics(RequestMetrics factory) {
- requestMetricListeners.add(factory.create(meter));
+ requestListeners.add(factory.create(meter));
  return this;
  }