Manual cherrypick for commits 8bbb0e2, b287318 (#4104)

open-telemetry · Jan 21, 2022 · 3938d96 · 3938d96
1 parent 8967558
commit 3938d96
Show file tree

Hide file tree

Showing 15 changed files with 180 additions and 47 deletions.
diff --git a/exporters/otlp/common/build.gradle.kts b/exporters/otlp/common/build.gradle.kts
@@ -43,6 +43,7 @@ dependencies {
   testImplementation("com.linecorp.armeria:armeria-junit5")
   testImplementation("io.opentelemetry.proto:opentelemetry-proto")
   testImplementation("org.skyscreamer:jsonassert")
+  testImplementation("org.assertj:assertj-core")
 
   testImplementation("com.google.api.grpc:proto-google-common-protos")
   testImplementation("io.grpc:grpc-testing")

diff --git a/...rc/main/java/io/opentelemetry/exporter/otlp/internal/grpc/DefaultGrpcExporterBuilder.java b/...rc/main/java/io/opentelemetry/exporter/otlp/internal/grpc/DefaultGrpcExporterBuilder.java
@@ -16,7 +16,6 @@
 import io.opentelemetry.api.metrics.MeterProvider;
 import io.opentelemetry.exporter.otlp.internal.Marshaler;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
-import java.lang.reflect.Field;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.time.Duration;
@@ -44,7 +43,7 @@ public final class DefaultGrpcExporterBuilder<T extends Marshaler>
   private boolean compressionEnabled = false;
   @Nullable private Metadata metadata;
   @Nullable private byte[] trustedCertificatesPem;
-  @Nullable private RetryPolicy retryPolicy;
+  @Nullable RetryPolicy retryPolicy;
   private MeterProvider meterProvider = MeterProvider.noop();
 
   /** Creates a new {@link DefaultGrpcExporterBuilder}. */
@@ -173,25 +172,4 @@ public GrpcExporter<T> build() {
     return new DefaultGrpcExporter<>(
         type, channel, stub, meterProvider, timeoutNanos, compressionEnabled);
   }
-
-  /**
-   * Reflectively access a {@link DefaultGrpcExporterBuilder} instance in field called "delegate" of
-   * the instance.
-   *
-   * @throws IllegalArgumentException if the instance does not contain a field called "delegate" of
-   *     type {@link DefaultGrpcExporterBuilder}
-   */
-  public static <T> DefaultGrpcExporterBuilder<?> getDelegateBuilder(Class<T> type, T instance) {
-    try {
-      Field field = type.getDeclaredField("delegate");
-      field.setAccessible(true);
-      Object value = field.get(instance);
-      if (!(value instanceof DefaultGrpcExporterBuilder)) {
-        throw new IllegalArgumentException("delegate field is not type DefaultGrpcExporterBuilder");
-      }
-      return (DefaultGrpcExporterBuilder<?>) value;
-    } catch (NoSuchFieldException | IllegalAccessException e) {
-      throw new IllegalArgumentException("Unable to access delegate reflectively.", e);
-    }
-  }
 }
diff --git a/...rs/otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/retry/RetryUtil.java b/...rs/otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/retry/RetryUtil.java
@@ -5,7 +5,10 @@
 
 package io.opentelemetry.exporter.otlp.internal.retry;
 
+import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.grpc.GrpcStatusUtil;
+import io.opentelemetry.exporter.otlp.internal.grpc.OkHttpGrpcExporterBuilder;
+import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
@@ -44,4 +47,29 @@ public static Set<String> retryableGrpcStatusCodes() {
   public static Set<Integer> retryableHttpResponseCodes() {
     return RETRYABLE_HTTP_STATUS_CODES;
   }
+
+  /**
+   * Reflectively access a {@link DefaultGrpcExporterBuilder} or {@link OkHttpGrpcExporterBuilder}
+   * instance in field called "delegate" of the instance, and set the {@link RetryPolicy}.
+   *
+   * @throws IllegalArgumentException if the instance does not contain a field called "delegate" of
+   *     type {@link DefaultGrpcExporterBuilder}
+   */
+  public static void setRetryPolicyOnDelegate(Object instance, RetryPolicy retryPolicy) {
+    try {
+      Field field = instance.getClass().getDeclaredField("delegate");
+      field.setAccessible(true);
+      Object value = field.get(instance);
+      if (value instanceof DefaultGrpcExporterBuilder) {
+        ((DefaultGrpcExporterBuilder<?>) value).setRetryPolicy(retryPolicy);
+      } else if (value instanceof OkHttpGrpcExporterBuilder) {
+        ((OkHttpGrpcExporterBuilder<?>) value).setRetryPolicy(retryPolicy);
+      } else {
+        throw new IllegalArgumentException(
+            "delegate field is not type DefaultGrpcExporterBuilder or OkHttpGrpcExporterBuilder");
+      }
+    } catch (NoSuchFieldException | IllegalAccessException e) {
+      throw new IllegalArgumentException("Unable to access delegate reflectively.", e);
+    }
+  }
 }
diff --git a/...tlp/common/src/test/java/io/opentelemetry/exporter/otlp/internal/retry/RetryUtilTest.java b/...tlp/common/src/test/java/io/opentelemetry/exporter/otlp/internal/retry/RetryUtilTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.exporter.otlp.internal.retry;
+
+import static org.assertj.core.api.Assertions.as;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
+import io.opentelemetry.exporter.otlp.internal.grpc.OkHttpGrpcExporterBuilder;
+import java.net.URI;
+import java.net.URISyntaxException;
+import org.assertj.core.api.InstanceOfAssertFactories;
+import org.junit.jupiter.api.Test;
+
+class RetryUtilTest {
+
+  @Test
+  void setRetryPolicyOnDelegate_DefaultGrpcExporterBuilder() throws URISyntaxException {
+    RetryPolicy retryPolicy = RetryPolicy.getDefault();
+    DefaultGrpcExporterBuilder<?> builder =
+        new DefaultGrpcExporterBuilder<>(
+            "test", unused -> null, 0, new URI("http://localhost"), "test");
+
+    RetryUtil.setRetryPolicyOnDelegate(new WithDelegate(builder), retryPolicy);
+
+    assertThat(builder)
+        .extracting("retryPolicy", as(InstanceOfAssertFactories.type(RetryPolicy.class)))
+        .isEqualTo(retryPolicy);
+  }
+
+  @Test
+  void setRetryPolicyOnDelegate_OkHttpGrpcExporterBuilder() throws URISyntaxException {
+    RetryPolicy retryPolicy = RetryPolicy.getDefault();
+    OkHttpGrpcExporterBuilder<?> builder =
+        new OkHttpGrpcExporterBuilder<>("test", "/test", 0, new URI("http://localhost"));
+
+    RetryUtil.setRetryPolicyOnDelegate(new WithDelegate(builder), retryPolicy);
+
+    assertThat(builder)
+        .extracting("retryPolicy", as(InstanceOfAssertFactories.type(RetryPolicy.class)))
+        .isEqualTo(retryPolicy);
+  }
+
+  @Test
+  void setRetryPolicyOnDelegate_InvalidUsage() {
+    assertThatThrownBy(
+            () -> RetryUtil.setRetryPolicyOnDelegate(new Object(), RetryPolicy.getDefault()))
+        .hasMessageContaining("Unable to access delegate reflectively");
+    assertThatThrownBy(
+            () ->
+                RetryUtil.setRetryPolicyOnDelegate(
+                    new WithDelegate(new Object()), RetryPolicy.getDefault()))
+        .hasMessageContaining(
+            "delegate field is not type DefaultGrpcExporterBuilder or OkHttpGrpcExporterBuilder");
+  }
+
+  @SuppressWarnings({"UnusedVariable", "FieldCanBeLocal"})
+  private static class WithDelegate {
+    private final Object delegate;
+
+    private WithDelegate(Object delegate) {
+      this.delegate = delegate;
+    }
+  }
+}
diff --git a/.../otlp/logs/src/test/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogExporterTest.java b/.../otlp/logs/src/test/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogExporterTest.java
@@ -6,12 +6,14 @@
 package io.opentelemetry.exporter.otlp.logs;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.exporter.otlp.internal.Marshaler;
 import io.opentelemetry.exporter.otlp.internal.grpc.OkHttpGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.logs.ResourceLogsMarshaler;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
+import io.opentelemetry.exporter.otlp.internal.retry.RetryUtil;
 import io.opentelemetry.exporter.otlp.testing.internal.AbstractGrpcTelemetryExporterTest;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporter;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporterBuilder;
@@ -33,6 +35,15 @@ class OtlpGrpcLogExporterTest extends AbstractGrpcTelemetryExporterTest<LogData,
     super("log", ResourceLogs.getDefaultInstance());
   }
 
+  @Test
+  void testBuilderDelegate() {
+    assertThatCode(
+            () ->
+                RetryUtil.setRetryPolicyOnDelegate(
+                    OtlpGrpcLogExporter.builder(), RetryPolicy.getDefault()))
+        .doesNotThrowAnyException();
+  }
+
   @Test
   void usingOkHttp() {
     assertThat(OtlpGrpcLogExporter.builder().delegate)

diff --git a/.../testGrpcNetty/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcNettyLogExporterTest.java b/.../testGrpcNetty/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcNettyLogExporterTest.java
@@ -13,6 +13,7 @@
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.logs.ResourceLogsMarshaler;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
+import io.opentelemetry.exporter.otlp.internal.retry.RetryUtil;
 import io.opentelemetry.exporter.otlp.testing.internal.AbstractGrpcTelemetryExporterTest;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporter;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporterBuilder;
@@ -39,9 +40,8 @@ class OtlpGrpcNettyLogExporterTest
   void testBuilderDelegate() {
     assertThatCode(
             () ->
-                DefaultGrpcExporterBuilder.getDelegateBuilder(
-                        OtlpGrpcLogExporterBuilder.class, OtlpGrpcLogExporter.builder())
-                    .setRetryPolicy(RetryPolicy.getDefault()))
+                RetryUtil.setRetryPolicyOnDelegate(
+                    OtlpGrpcLogExporter.builder(), RetryPolicy.getDefault()))
         .doesNotThrowAnyException();
   }
 

diff --git a/...rics/src/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterTest.java b/...rics/src/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterTest.java
@@ -7,12 +7,14 @@
 
 import static io.opentelemetry.api.common.AttributeKey.stringKey;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.exporter.otlp.internal.Marshaler;
 import io.opentelemetry.exporter.otlp.internal.grpc.OkHttpGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.metrics.ResourceMetricsMarshaler;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
+import io.opentelemetry.exporter.otlp.internal.retry.RetryUtil;
 import io.opentelemetry.exporter.otlp.testing.internal.AbstractGrpcTelemetryExporterTest;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporter;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporterBuilder;
@@ -36,6 +38,15 @@ class OtlpGrpcMetricExporterTest
     super("metric", ResourceMetrics.getDefaultInstance());
   }
 
+  @Test
+  void testBuilderDelegate() {
+    assertThatCode(
+            () ->
+                RetryUtil.setRetryPolicyOnDelegate(
+                    OtlpGrpcMetricExporter.builder(), RetryPolicy.getDefault()))
+        .doesNotThrowAnyException();
+  }
+
   @Test
   void usingOkHttp() {
     assertThat(OtlpGrpcMetricExporter.builder().delegate)

diff --git a/...rpcNetty/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcNettyMetricExporterTest.java b/...rpcNetty/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcNettyMetricExporterTest.java
@@ -14,6 +14,7 @@
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.metrics.ResourceMetricsMarshaler;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
+import io.opentelemetry.exporter.otlp.internal.retry.RetryUtil;
 import io.opentelemetry.exporter.otlp.testing.internal.AbstractGrpcTelemetryExporterTest;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporter;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporterBuilder;
@@ -41,9 +42,8 @@ class OtlpGrpcNettyMetricExporterTest
   void testBuilderDelegate() {
     assertThatCode(
             () ->
-                DefaultGrpcExporterBuilder.getDelegateBuilder(
-                        OtlpGrpcMetricExporterBuilder.class, OtlpGrpcMetricExporter.builder())
-                    .setRetryPolicy(RetryPolicy.getDefault()))
+                RetryUtil.setRetryPolicyOnDelegate(
+                    OtlpGrpcMetricExporter.builder(), RetryPolicy.getDefault()))
         .doesNotThrowAnyException();
   }
 

diff --git a/...lp/trace/src/test/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterTest.java b/...lp/trace/src/test/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterTest.java
@@ -6,6 +6,7 @@
 package io.opentelemetry.exporter.otlp.trace;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.api.trace.SpanKind;
@@ -14,6 +15,7 @@
 import io.opentelemetry.exporter.otlp.internal.Marshaler;
 import io.opentelemetry.exporter.otlp.internal.grpc.OkHttpGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
+import io.opentelemetry.exporter.otlp.internal.retry.RetryUtil;
 import io.opentelemetry.exporter.otlp.internal.traces.ResourceSpansMarshaler;
 import io.opentelemetry.exporter.otlp.testing.internal.AbstractGrpcTelemetryExporterTest;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporter;
@@ -38,6 +40,15 @@ class OtlpGrpcSpanExporterTest extends AbstractGrpcTelemetryExporterTest<SpanDat
     super("span", ResourceSpans.getDefaultInstance());
   }
 
+  @Test
+  void testBuilderDelegate() {
+    assertThatCode(
+            () ->
+                RetryUtil.setRetryPolicyOnDelegate(
+                    OtlpGrpcSpanExporter.builder(), RetryPolicy.getDefault()))
+        .doesNotThrowAnyException();
+  }
+
   @Test
   void usingOkHttp() {
     assertThat(OtlpGrpcSpanExporter.builder().delegate)

diff --git a/...estGrpcNetty/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcNettySpanExporterTest.java b/...estGrpcNetty/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcNettySpanExporterTest.java
@@ -15,6 +15,7 @@
 import io.opentelemetry.exporter.otlp.internal.Marshaler;
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
 import io.opentelemetry.exporter.otlp.internal.retry.RetryPolicy;
+import io.opentelemetry.exporter.otlp.internal.retry.RetryUtil;
 import io.opentelemetry.exporter.otlp.internal.traces.ResourceSpansMarshaler;
 import io.opentelemetry.exporter.otlp.testing.internal.AbstractGrpcTelemetryExporterTest;
 import io.opentelemetry.exporter.otlp.testing.internal.TelemetryExporter;
@@ -44,9 +45,8 @@ class OtlpGrpcNettySpanExporterTest
   void builderDelegate() {
     assertThatCode(
             () ->
-                DefaultGrpcExporterBuilder.getDelegateBuilder(
-                        OtlpGrpcSpanExporterBuilder.class, OtlpGrpcSpanExporter.builder())
-                    .setRetryPolicy(RetryPolicy.getDefault()))
+                RetryUtil.setRetryPolicyOnDelegate(
+                    OtlpGrpcSpanExporter.builder(), RetryPolicy.getDefault()))
         .doesNotThrowAnyException();
   }
 

diff --git a/...rs/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusCollector.java b/...rs/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusCollector.java
@@ -25,13 +25,31 @@
  */
 public final class PrometheusCollector extends Collector implements MetricReader {
   private final MetricProducer metricProducer;
+  private volatile boolean registered = false;
 
   PrometheusCollector(MetricProducer metricProducer) {
     this.metricProducer = metricProducer;
   }
 
+  /**
+   * This method is called in {@link Factory#apply(MetricProducer)}. {@link Collector#register()}
+   * triggers a call to {@link #collect()}, which throws an error because {@link
+   * MetricProducer#collectAllMetrics()} is not yet read to accept calls. To get around this, we
+   * have {@link #collect()} exit early until registration is complete.
+   */
+  @SuppressWarnings("TypeParameterUnusedInFormals")
+  @Override
+  public <T extends Collector> T register() {
+    T result = super.register();
+    this.registered = true;
+    return result;
+  }
+
   @Override
   public List<MetricFamilySamples> collect() {
+    if (!registered) {
+      return Collections.emptyList();
+    }
     Collection<MetricData> allMetrics = metricProducer.collectAllMetrics();
     List<MetricFamilySamples> allSamples = new ArrayList<>(allMetrics.size());
     for (MetricData metricData : allMetrics) {

diff --git a/...rometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusCollectorTest.java b/...rometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusCollectorTest.java
@@ -7,11 +7,13 @@
 
 import static io.opentelemetry.api.common.AttributeKey.stringKey;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableList;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
 import io.opentelemetry.sdk.metrics.data.DoublePointData;
 import io.opentelemetry.sdk.metrics.data.DoubleSumData;
@@ -25,6 +27,7 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.Collections;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -43,6 +46,18 @@ void setUp() {
     prometheusCollector.register();
   }
 
+  @Test
+  void registerWithSdkMeterProvider() {
+    assertThatCode(
+            () ->
+                SdkMeterProvider.builder()
+                    .registerMetricReader(PrometheusCollector.create())
+                    .build()
+                    .forceFlush()
+                    .join(10, TimeUnit.SECONDS))
+        .doesNotThrowAnyException();
+  }
+
   @Test
   void registerToDefault() throws IOException {
     when(metricProducer.collectAllMetrics()).thenReturn(generateTestData());