From c43769da5198cffac1528d1b6f5d76de8c27c9a9 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 16 Oct 2023 14:53:48 +0200 Subject: [PATCH 01/24] Created InstrumentationUtil --- .../internal/InstrumentationUtil.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java new file mode 100644 index 00000000000..c07e86a3416 --- /dev/null +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -0,0 +1,36 @@ +package io.opentelemetry.exporter.internal; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import java.util.Objects; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +public class InstrumentationUtil { + private static final ContextKey SUPPRESS_INSTRUMENTATION_KEY = ContextKey.named("suppress_instrumentation"); + + private InstrumentationUtil() { + } + + /** + * Adds a Context boolean key that will allow to identify HTTP calls coming from OTel exporters. The key + * later be checked by an automatic instrumentation to avoid tracing OTel exporter's calls. + */ + public static void suppressInstrumentation(Runnable runnable) { + Context.current().with(SUPPRESS_INSTRUMENTATION_KEY, true).wrap(runnable).run(); + } + + /** + * Checks if an automatic instrumentation should be suppressed with the provided Context. + * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the instrumentation. + */ + public static boolean shouldSuppressInstrumentation(Context context) { + return Objects.equals(context.get(SUPPRESS_INSTRUMENTATION_KEY), true); + } + + public static boolean shouldSuppressInstrumentation() { + return shouldSuppressInstrumentation(Context.current()); + } +} From 908e04fdd74d6223bb579ba601b92b2df62d13c8 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 16 Oct 2023 14:56:52 +0200 Subject: [PATCH 02/24] Suppressing automatic instrumentation for OTel exporters using OkHttp --- .../exporter/sender/okhttp/internal/OkHttpGrpcSender.java | 5 +++-- .../exporter/sender/okhttp/internal/OkHttpHttpSender.java | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java index 5da750edead..e9b3a42d5b3 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java @@ -23,6 +23,7 @@ package io.opentelemetry.exporter.sender.okhttp.internal; +import io.opentelemetry.exporter.internal.InstrumentationUtil; import io.opentelemetry.exporter.internal.RetryUtil; import io.opentelemetry.exporter.internal.grpc.GrpcExporterUtil; import io.opentelemetry.exporter.internal.grpc.GrpcResponse; @@ -112,7 +113,7 @@ public void send(T request, Runnable onSuccess, BiConsumer client .newCall(requestBuilder.build()) .enqueue( new Callback() { @@ -156,7 +157,7 @@ public void onResponse(Call call, Response response) { GrpcResponse.create(statusCode, errorMessage), new IllegalStateException(errorMessage)); } - }); + })); } @Nullable diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java index 306c7b97163..8cf567f590e 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java @@ -5,6 +5,7 @@ package io.opentelemetry.exporter.sender.okhttp.internal; +import io.opentelemetry.exporter.internal.InstrumentationUtil; import io.opentelemetry.exporter.internal.RetryUtil; import io.opentelemetry.exporter.internal.auth.Authenticator; import io.opentelemetry.exporter.internal.http.HttpSender; @@ -101,7 +102,7 @@ public void send( requestBuilder.post(body); } - client + InstrumentationUtil.suppressInstrumentation(()-> client .newCall(requestBuilder.build()) .enqueue( new Callback() { @@ -132,7 +133,7 @@ public byte[] responseBody() throws IOException { }); } } - }); + })); } @Override From f00b5b733c260fbbec5e243ece96d8bd9d99a11e Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 16 Oct 2023 15:01:21 +0200 Subject: [PATCH 03/24] Adding InstrumentationUtilTest validations --- .../internal/InstrumentationUtilTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java new file mode 100644 index 00000000000..1b57e1cf40c --- /dev/null +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java @@ -0,0 +1,20 @@ +package io.opentelemetry.exporter.internal; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class InstrumentationUtilTest { + @Test + void verifySuppressInstrumentation() { + // Should be false by default. + assertFalse(InstrumentationUtil.shouldSuppressInstrumentation()); + + // Should be true inside the Runnable passed to InstrumentationUtil.suppressInstrumentation. + InstrumentationUtil.suppressInstrumentation(() -> assertTrue(InstrumentationUtil.shouldSuppressInstrumentation())); + + // Should be false after the runnable finishes. + assertFalse(InstrumentationUtil.shouldSuppressInstrumentation()); + } +} From c10cf0f8f55cbbf457e2aa258c3c104074615704 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 16 Oct 2023 15:04:40 +0200 Subject: [PATCH 04/24] Spotless --- .../internal/InstrumentationUtil.java | 20 ++-- .../internal/InstrumentationUtilTest.java | 12 ++- .../okhttp/internal/OkHttpGrpcSender.java | 92 ++++++++++--------- .../okhttp/internal/OkHttpHttpSender.java | 64 ++++++------- 4 files changed, 103 insertions(+), 85 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index c07e86a3416..9b5e3b10308 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.exporter.internal; import io.opentelemetry.context.Context; @@ -9,14 +14,15 @@ * any time. */ public class InstrumentationUtil { - private static final ContextKey SUPPRESS_INSTRUMENTATION_KEY = ContextKey.named("suppress_instrumentation"); + private static final ContextKey SUPPRESS_INSTRUMENTATION_KEY = + ContextKey.named("suppress_instrumentation"); - private InstrumentationUtil() { - } + private InstrumentationUtil() {} /** - * Adds a Context boolean key that will allow to identify HTTP calls coming from OTel exporters. The key - * later be checked by an automatic instrumentation to avoid tracing OTel exporter's calls. + * Adds a Context boolean key that will allow to identify HTTP calls coming from OTel exporters. + * The key later be checked by an automatic instrumentation to avoid tracing OTel exporter's + * calls. */ public static void suppressInstrumentation(Runnable runnable) { Context.current().with(SUPPRESS_INSTRUMENTATION_KEY, true).wrap(runnable).run(); @@ -24,7 +30,9 @@ public static void suppressInstrumentation(Runnable runnable) { /** * Checks if an automatic instrumentation should be suppressed with the provided Context. - * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the instrumentation. + * + * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the + * instrumentation. */ public static boolean shouldSuppressInstrumentation(Context context) { return Objects.equals(context.get(SUPPRESS_INSTRUMENTATION_KEY), true); diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java index 1b57e1cf40c..328fabb2326 100644 --- a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java @@ -1,10 +1,15 @@ -package io.opentelemetry.exporter.internal; +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ -import org.junit.jupiter.api.Test; +package io.opentelemetry.exporter.internal; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.Test; + class InstrumentationUtilTest { @Test void verifySuppressInstrumentation() { @@ -12,7 +17,8 @@ void verifySuppressInstrumentation() { assertFalse(InstrumentationUtil.shouldSuppressInstrumentation()); // Should be true inside the Runnable passed to InstrumentationUtil.suppressInstrumentation. - InstrumentationUtil.suppressInstrumentation(() -> assertTrue(InstrumentationUtil.shouldSuppressInstrumentation())); + InstrumentationUtil.suppressInstrumentation( + () -> assertTrue(InstrumentationUtil.shouldSuppressInstrumentation())); // Should be false after the runnable finishes. assertFalse(InstrumentationUtil.shouldSuppressInstrumentation()); diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java index e9b3a42d5b3..9ed05b003b5 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java @@ -113,51 +113,53 @@ public void send(T request, Runnable onSuccess, BiConsumer client - .newCall(requestBuilder.build()) - .enqueue( - new Callback() { - @Override - public void onFailure(Call call, IOException e) { - String description = e.getMessage(); - if (description == null) { - description = ""; - } - onError.accept(GrpcResponse.create(2 /* UNKNOWN */, description), e); - } - - @Override - public void onResponse(Call call, Response response) { - // Response body is empty but must be consumed to access trailers. - try { - response.body().bytes(); - } catch (IOException e) { - onError.accept( - GrpcResponse.create( - GrpcExporterUtil.GRPC_STATUS_UNKNOWN, - "Could not consume server response."), - e); - return; - } - - String status = grpcStatus(response); - if ("0".equals(status)) { - onSuccess.run(); - return; - } - - String errorMessage = grpcMessage(response); - int statusCode; - try { - statusCode = Integer.parseInt(status); - } catch (NumberFormatException ex) { - statusCode = GrpcExporterUtil.GRPC_STATUS_UNKNOWN; - } - onError.accept( - GrpcResponse.create(statusCode, errorMessage), - new IllegalStateException(errorMessage)); - } - })); + InstrumentationUtil.suppressInstrumentation( + () -> + client + .newCall(requestBuilder.build()) + .enqueue( + new Callback() { + @Override + public void onFailure(Call call, IOException e) { + String description = e.getMessage(); + if (description == null) { + description = ""; + } + onError.accept(GrpcResponse.create(2 /* UNKNOWN */, description), e); + } + + @Override + public void onResponse(Call call, Response response) { + // Response body is empty but must be consumed to access trailers. + try { + response.body().bytes(); + } catch (IOException e) { + onError.accept( + GrpcResponse.create( + GrpcExporterUtil.GRPC_STATUS_UNKNOWN, + "Could not consume server response."), + e); + return; + } + + String status = grpcStatus(response); + if ("0".equals(status)) { + onSuccess.run(); + return; + } + + String errorMessage = grpcMessage(response); + int statusCode; + try { + statusCode = Integer.parseInt(status); + } catch (NumberFormatException ex) { + statusCode = GrpcExporterUtil.GRPC_STATUS_UNKNOWN; + } + onError.accept( + GrpcResponse.create(statusCode, errorMessage), + new IllegalStateException(errorMessage)); + } + })); } @Nullable diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java index 8cf567f590e..2355a94ba60 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java @@ -102,38 +102,40 @@ public void send( requestBuilder.post(body); } - InstrumentationUtil.suppressInstrumentation(()-> client - .newCall(requestBuilder.build()) - .enqueue( - new Callback() { - @Override - public void onFailure(Call call, IOException e) { - onError.accept(e); - } - - @Override - public void onResponse(Call call, okhttp3.Response response) { - try (ResponseBody body = response.body()) { - onResponse.accept( - new Response() { - @Override - public int statusCode() { - return response.code(); + InstrumentationUtil.suppressInstrumentation( + () -> + client + .newCall(requestBuilder.build()) + .enqueue( + new Callback() { + @Override + public void onFailure(Call call, IOException e) { + onError.accept(e); + } + + @Override + public void onResponse(Call call, okhttp3.Response response) { + try (ResponseBody body = response.body()) { + onResponse.accept( + new Response() { + @Override + public int statusCode() { + return response.code(); + } + + @Override + public String statusMessage() { + return response.message(); + } + + @Override + public byte[] responseBody() throws IOException { + return body.bytes(); + } + }); } - - @Override - public String statusMessage() { - return response.message(); - } - - @Override - public byte[] responseBody() throws IOException { - return body.bytes(); - } - }); - } - } - })); + } + })); } @Override From f7c5ef80be75d11a592646160428462b9c026cc1 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 25 Oct 2023 13:47:05 +0200 Subject: [PATCH 05/24] Verifying suppress instrumentation key is present in okhttp calls --- .../sender/okhttp/internal/OkHttpUtil.java | 4 +- .../okhttp/internal/OkHttpGrpcSenderTest.java | 31 +++++++++++ .../okhttp/internal/OkHttpHttpSenderTest.java | 35 +++++++++++++ .../okhttp/internal/OkHttpSenderTest.java | 51 +++++++++++++++++++ .../sdk/internal/DaemonThreadFactory.java | 9 +++- 5 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java create mode 100644 exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java create mode 100644 exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java index 8aef6b7c21a..754c2a50b9a 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java @@ -18,6 +18,8 @@ * at any time. */ public final class OkHttpUtil { + // For testing purposes + static boolean propagateContextInDispatcher = false; /** Returns a {@link Dispatcher} using daemon threads, otherwise matching the OkHttp default. */ public static Dispatcher newDispatcher() { @@ -28,7 +30,7 @@ public static Dispatcher newDispatcher() { 60, TimeUnit.SECONDS, new SynchronousQueue<>(), - new DaemonThreadFactory("okhttp-dispatch"))); + new DaemonThreadFactory("okhttp-dispatch", propagateContextInDispatcher))); } private OkHttpUtil() {} diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java new file mode 100644 index 00000000000..7b9293a626c --- /dev/null +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java @@ -0,0 +1,31 @@ +package io.opentelemetry.exporter.sender.okhttp.internal; + +import io.opentelemetry.exporter.internal.grpc.GrpcSender; +import io.opentelemetry.exporter.internal.marshal.MarshalerWithSize; +import io.opentelemetry.exporter.internal.marshal.Serializer; +import java.util.Collections; + +class OkHttpGrpcSenderTest + extends OkHttpSenderTest> { + + @Override + void send(GrpcSender sender, Runnable onSuccess, Runnable onFailure) { + sender.send(new DummyMarshaler(), onSuccess, (grpcResponse, throwable) -> onFailure.run()); + } + + @Override + GrpcSender createSender(String endpoint) { + return new OkHttpGrpcSender<>( + "https://localhost", false, 10L, Collections.emptyMap(), null, null, null); + } + + protected static class DummyMarshaler extends MarshalerWithSize { + + protected DummyMarshaler() { + super(0); + } + + @Override + protected void writeTo(Serializer output) {} + } +} diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java new file mode 100644 index 00000000000..441ce49a40b --- /dev/null +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java @@ -0,0 +1,35 @@ +package io.opentelemetry.exporter.sender.okhttp.internal; + +import io.opentelemetry.exporter.internal.http.HttpSender; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.function.Consumer; + +class OkHttpHttpSenderTest extends OkHttpSenderTest { + + @Override + void send(HttpSender sender, Runnable onSuccess, Runnable onFailure) { + byte[] content = "A".getBytes(StandardCharsets.UTF_8); + Consumer outputStreamConsumer = + outputStream -> { + try { + outputStream.write(content); + } catch (IOException e) { + throw new RuntimeException(e); + } + }; + sender.send( + outputStreamConsumer, + content.length, + (response) -> onSuccess.run(), + (error) -> onFailure.run()); + } + + @Override + HttpSender createSender(String endpoint) { + return new OkHttpHttpSender( + endpoint, false, "text/plain", 10L, Collections::emptyMap, null, null, null, null); + } +} diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java new file mode 100644 index 00000000000..cfac2feb0a7 --- /dev/null +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java @@ -0,0 +1,51 @@ +package io.opentelemetry.exporter.sender.okhttp.internal; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.opentelemetry.exporter.internal.InstrumentationUtil; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +abstract class OkHttpSenderTest { + + @BeforeEach + void setUp() { + OkHttpUtil.propagateContextInDispatcher = true; + } + + @AfterEach + void tearDown() { + OkHttpUtil.propagateContextInDispatcher = false; + } + + @Test + void testSuppressInstrumentation() throws InterruptedException { + CountDownLatch lock = new CountDownLatch(1); + AtomicBoolean suppressInstrumentation = new AtomicBoolean(false); + + Runnable onSuccess = Assertions::fail; + Runnable onFailure = + () -> { + suppressInstrumentation.set(InstrumentationUtil.shouldSuppressInstrumentation()); + lock.countDown(); + }; + + send(getSender(), onSuccess, onFailure); + + lock.await(); + + assertTrue(suppressInstrumentation.get()); + } + + abstract void send(T sender, Runnable onSuccess, Runnable onFailure); + + private T getSender() { + return createSender("https://none"); + } + + abstract T createSender(String endpoint); +} diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java index 9e319d8f735..6688e3408b6 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java @@ -5,6 +5,7 @@ package io.opentelemetry.sdk.internal; +import io.opentelemetry.context.Context; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; @@ -20,14 +21,20 @@ public final class DaemonThreadFactory implements ThreadFactory { private final String namePrefix; private final AtomicInteger counter = new AtomicInteger(); private final ThreadFactory delegate = Executors.defaultThreadFactory(); + private final boolean propagateContext; public DaemonThreadFactory(String namePrefix) { + this(namePrefix, /* propagateContext= */ false); + } + + public DaemonThreadFactory(String namePrefix, boolean propagateContext) { this.namePrefix = namePrefix; + this.propagateContext = propagateContext; } @Override public Thread newThread(Runnable runnable) { - Thread t = delegate.newThread(runnable); + Thread t = delegate.newThread(propagateContext ? Context.current().wrap(runnable) : runnable); try { t.setDaemon(true); t.setName(namePrefix + "-" + counter.incrementAndGet()); From 2bb40e56fc7a9326794416641e36994ab24abac8 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 25 Oct 2023 13:47:34 +0200 Subject: [PATCH 06/24] Spotless --- .../sender/okhttp/internal/OkHttpGrpcSenderTest.java | 5 +++++ .../sender/okhttp/internal/OkHttpHttpSenderTest.java | 5 +++++ .../exporter/sender/okhttp/internal/OkHttpSenderTest.java | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java index 7b9293a626c..19ab60c5582 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.exporter.sender.okhttp.internal; import io.opentelemetry.exporter.internal.grpc.GrpcSender; diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java index 441ce49a40b..ff8c0d1f0c1 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.exporter.sender.okhttp.internal; import io.opentelemetry.exporter.internal.http.HttpSender; diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java index cfac2feb0a7..fc9bc3c7c21 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.exporter.sender.okhttp.internal; import static org.junit.jupiter.api.Assertions.assertTrue; From 52b83a01ab6f5a79a6927fef6e4ab18758872f21 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 25 Oct 2023 14:06:09 +0200 Subject: [PATCH 07/24] Suppressing NonFinalStaticField --- .../exporter/sender/okhttp/internal/OkHttpUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java index 754c2a50b9a..36d0361e65d 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java @@ -19,6 +19,7 @@ */ public final class OkHttpUtil { // For testing purposes + @SuppressWarnings("NonFinalStaticField") static boolean propagateContextInDispatcher = false; /** Returns a {@link Dispatcher} using daemon threads, otherwise matching the OkHttp default. */ From 2e2bed7433e17b358f474576fda87dcbad7239a6 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:16:57 +0200 Subject: [PATCH 08/24] Added javadoc --- .../exporter/internal/InstrumentationUtil.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index 9b5e3b10308..f18f14a3938 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -38,6 +38,12 @@ public static boolean shouldSuppressInstrumentation(Context context) { return Objects.equals(context.get(SUPPRESS_INSTRUMENTATION_KEY), true); } + /** + * Checks if an automatic instrumentation should be suppressed within the current Context. + * + * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the + * instrumentation. + */ public static boolean shouldSuppressInstrumentation() { return shouldSuppressInstrumentation(Context.current()); } From 456f71b343216eefe34ba78ae035758f5d1f730e Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:17:33 +0200 Subject: [PATCH 09/24] Renaming lock by latch --- .../exporter/sender/okhttp/internal/OkHttpSenderTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java index fc9bc3c7c21..36ca8fe9e7e 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java @@ -29,19 +29,19 @@ void tearDown() { @Test void testSuppressInstrumentation() throws InterruptedException { - CountDownLatch lock = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(1); AtomicBoolean suppressInstrumentation = new AtomicBoolean(false); Runnable onSuccess = Assertions::fail; Runnable onFailure = () -> { suppressInstrumentation.set(InstrumentationUtil.shouldSuppressInstrumentation()); - lock.countDown(); + latch.countDown(); }; send(getSender(), onSuccess, onFailure); - lock.await(); + latch.await(); assertTrue(suppressInstrumentation.get()); } From faa5f6568201c2374324b6dab285993eb643b58b Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:23:18 +0200 Subject: [PATCH 10/24] Reordering functions --- .../exporter/internal/InstrumentationUtil.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index f18f14a3938..b45cb7cabd7 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -29,22 +29,22 @@ public static void suppressInstrumentation(Runnable runnable) { } /** - * Checks if an automatic instrumentation should be suppressed with the provided Context. + * Checks if an automatic instrumentation should be suppressed within the current Context. * * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the * instrumentation. */ - public static boolean shouldSuppressInstrumentation(Context context) { - return Objects.equals(context.get(SUPPRESS_INSTRUMENTATION_KEY), true); + public static boolean shouldSuppressInstrumentation() { + return shouldSuppressInstrumentation(Context.current()); } /** - * Checks if an automatic instrumentation should be suppressed within the current Context. + * Checks if an automatic instrumentation should be suppressed with the provided Context. * * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the * instrumentation. */ - public static boolean shouldSuppressInstrumentation() { - return shouldSuppressInstrumentation(Context.current()); + public static boolean shouldSuppressInstrumentation(Context context) { + return Objects.equals(context.get(SUPPRESS_INSTRUMENTATION_KEY), true); } } From adff2950d5285ed83592d4a8f738861294816bbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar?= <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:26:54 +0100 Subject: [PATCH 11/24] Update exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com> --- .../io/opentelemetry/exporter/internal/InstrumentationUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index b45cb7cabd7..8a5a60f2906 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -13,7 +13,7 @@ * This class is internal and is hence not for public use. Its APIs are unstable and can change at * any time. */ -public class InstrumentationUtil { +public final class InstrumentationUtil { private static final ContextKey SUPPRESS_INSTRUMENTATION_KEY = ContextKey.named("suppress_instrumentation"); From 246ed421f35b731d01b501165bcf506d313a3208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar?= <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:27:21 +0100 Subject: [PATCH 12/24] Update exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com> --- .../exporter/internal/InstrumentationUtil.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index 8a5a60f2906..2c41addef03 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -28,15 +28,6 @@ public static void suppressInstrumentation(Runnable runnable) { Context.current().with(SUPPRESS_INSTRUMENTATION_KEY, true).wrap(runnable).run(); } - /** - * Checks if an automatic instrumentation should be suppressed within the current Context. - * - * @return TRUE to suppress the automatic instrumentation, FALSE to continue with the - * instrumentation. - */ - public static boolean shouldSuppressInstrumentation() { - return shouldSuppressInstrumentation(Context.current()); - } /** * Checks if an automatic instrumentation should be suppressed with the provided Context. From 0b7601bf2e82a37d3a618470a7e0d0275e9b2fe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar?= <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:29:30 +0100 Subject: [PATCH 13/24] Update exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com> --- .../exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java index ff8c0d1f0c1..c4d404bf758 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java @@ -12,7 +12,7 @@ import java.util.Collections; import java.util.function.Consumer; -class OkHttpHttpSenderTest extends OkHttpSenderTest { +class OkHttpHttpSenderTest extends OkHttpSenderTest { @Override void send(HttpSender sender, Runnable onSuccess, Runnable onFailure) { From 33b68cf8c7aa671aff564307546832f73c423da9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar?= <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:29:37 +0100 Subject: [PATCH 14/24] Update exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com> --- .../exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java index 19ab60c5582..4352635eec1 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java @@ -11,7 +11,7 @@ import java.util.Collections; class OkHttpGrpcSenderTest - extends OkHttpSenderTest> { + extends OkHttpSenderTest> { @Override void send(GrpcSender sender, Runnable onSuccess, Runnable onFailure) { From 79e616b287354b6219d665d2e25af02a3ccf3422 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:31:21 +0100 Subject: [PATCH 15/24] Spotless --- .../io/opentelemetry/exporter/internal/InstrumentationUtil.java | 1 - 1 file changed, 1 deletion(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index 2c41addef03..d80aedfdf32 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -28,7 +28,6 @@ public static void suppressInstrumentation(Runnable runnable) { Context.current().with(SUPPRESS_INSTRUMENTATION_KEY, true).wrap(runnable).run(); } - /** * Checks if an automatic instrumentation should be suppressed with the provided Context. * From 9a47e53743e5a76fc3a56469c5c9e3339841df96 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:31:45 +0100 Subject: [PATCH 16/24] Renamed ContextKey to suppress_internal_exporter_instrumentation --- .../io/opentelemetry/exporter/internal/InstrumentationUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java index d80aedfdf32..4ce9622fe4c 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/InstrumentationUtil.java @@ -15,7 +15,7 @@ */ public final class InstrumentationUtil { private static final ContextKey SUPPRESS_INSTRUMENTATION_KEY = - ContextKey.named("suppress_instrumentation"); + ContextKey.named("suppress_internal_exporter_instrumentation"); private InstrumentationUtil() {} From b5f90ac859ffd8e863fcba897c37ff74ef051c5d Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:35:58 +0100 Subject: [PATCH 17/24] Renaming test classes --- ...derTest.java => AbstractOkHttpSuppressionTest.java} | 6 ++++-- ...cSenderTest.java => OkHttpGrpcSuppressionTest.java} | 10 +++++----- ...pSenderTest.java => OkHttpHttpSuppressionTest.java} | 7 +++---- 3 files changed, 12 insertions(+), 11 deletions(-) rename exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/{OkHttpSenderTest.java => AbstractOkHttpSuppressionTest.java} (86%) rename exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/{OkHttpGrpcSenderTest.java => OkHttpGrpcSuppressionTest.java} (69%) rename exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/{OkHttpHttpSenderTest.java => OkHttpHttpSuppressionTest.java} (78%) diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java similarity index 86% rename from exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java rename to exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java index 36ca8fe9e7e..60addd0323a 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; +import io.opentelemetry.context.Context; import io.opentelemetry.exporter.internal.InstrumentationUtil; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; @@ -15,7 +16,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -abstract class OkHttpSenderTest { +abstract class AbstractOkHttpSuppressionTest { @BeforeEach void setUp() { @@ -35,7 +36,8 @@ void testSuppressInstrumentation() throws InterruptedException { Runnable onSuccess = Assertions::fail; Runnable onFailure = () -> { - suppressInstrumentation.set(InstrumentationUtil.shouldSuppressInstrumentation()); + suppressInstrumentation.set( + InstrumentationUtil.shouldSuppressInstrumentation(Context.current())); latch.countDown(); }; diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSuppressionTest.java similarity index 69% rename from exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java rename to exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSuppressionTest.java index 4352635eec1..df52ff0f891 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSuppressionTest.java @@ -5,21 +5,21 @@ package io.opentelemetry.exporter.sender.okhttp.internal; -import io.opentelemetry.exporter.internal.grpc.GrpcSender; import io.opentelemetry.exporter.internal.marshal.MarshalerWithSize; import io.opentelemetry.exporter.internal.marshal.Serializer; import java.util.Collections; -class OkHttpGrpcSenderTest - extends OkHttpSenderTest> { +class OkHttpGrpcSuppressionTest + extends AbstractOkHttpSuppressionTest< + OkHttpGrpcSender> { @Override - void send(GrpcSender sender, Runnable onSuccess, Runnable onFailure) { + void send(OkHttpGrpcSender sender, Runnable onSuccess, Runnable onFailure) { sender.send(new DummyMarshaler(), onSuccess, (grpcResponse, throwable) -> onFailure.run()); } @Override - GrpcSender createSender(String endpoint) { + OkHttpGrpcSender createSender(String endpoint) { return new OkHttpGrpcSender<>( "https://localhost", false, 10L, Collections.emptyMap(), null, null, null); } diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java similarity index 78% rename from exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java rename to exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java index c4d404bf758..22c2c4a0dd0 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSenderTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSuppressionTest.java @@ -5,17 +5,16 @@ package io.opentelemetry.exporter.sender.okhttp.internal; -import io.opentelemetry.exporter.internal.http.HttpSender; import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.function.Consumer; -class OkHttpHttpSenderTest extends OkHttpSenderTest { +class OkHttpHttpSuppressionTest extends AbstractOkHttpSuppressionTest { @Override - void send(HttpSender sender, Runnable onSuccess, Runnable onFailure) { + void send(OkHttpHttpSender sender, Runnable onSuccess, Runnable onFailure) { byte[] content = "A".getBytes(StandardCharsets.UTF_8); Consumer outputStreamConsumer = outputStream -> { @@ -33,7 +32,7 @@ void send(HttpSender sender, Runnable onSuccess, Runnable onFailure) { } @Override - HttpSender createSender(String endpoint) { + OkHttpHttpSender createSender(String endpoint) { return new OkHttpHttpSender( endpoint, false, "text/plain", 10L, Collections::emptyMap, null, null, null, null); } From 5fe0c06cbfd8cf2c98e0c36fa91798e95763e4a4 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 6 Nov 2023 14:09:35 +0100 Subject: [PATCH 18/24] Updating InstrumentationUtilTest --- .../exporter/internal/InstrumentationUtilTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java index 328fabb2326..2c7e9f540c7 100644 --- a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/InstrumentationUtilTest.java @@ -8,19 +8,20 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import io.opentelemetry.context.Context; import org.junit.jupiter.api.Test; class InstrumentationUtilTest { @Test void verifySuppressInstrumentation() { // Should be false by default. - assertFalse(InstrumentationUtil.shouldSuppressInstrumentation()); + assertFalse(InstrumentationUtil.shouldSuppressInstrumentation(Context.current())); // Should be true inside the Runnable passed to InstrumentationUtil.suppressInstrumentation. InstrumentationUtil.suppressInstrumentation( - () -> assertTrue(InstrumentationUtil.shouldSuppressInstrumentation())); + () -> assertTrue(InstrumentationUtil.shouldSuppressInstrumentation(Context.current()))); // Should be false after the runnable finishes. - assertFalse(InstrumentationUtil.shouldSuppressInstrumentation()); + assertFalse(InstrumentationUtil.shouldSuppressInstrumentation(Context.current())); } } From 5011af86437900a72aee31c2b31d38b1ceb69616 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:24:00 +0100 Subject: [PATCH 19/24] Adding javadoc to DaemonThreadFactory's constructor --- .../sdk/internal/DaemonThreadFactory.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java index 6688e3408b6..0e1ece19ddf 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java @@ -21,20 +21,28 @@ public final class DaemonThreadFactory implements ThreadFactory { private final String namePrefix; private final AtomicInteger counter = new AtomicInteger(); private final ThreadFactory delegate = Executors.defaultThreadFactory(); - private final boolean propagateContext; + private final boolean propagateContextForTesting; public DaemonThreadFactory(String namePrefix) { this(namePrefix, /* propagateContext= */ false); } - public DaemonThreadFactory(String namePrefix, boolean propagateContext) { + /** + * @param namePrefix Used when setting the new thread's name. + * @param propagateContextForTesting For tests only. When enabled, the current thread's {@link + * Context} will be passed over to the new threads, this is useful for testing scenarios where + * context propagation is available through bytecode instrumentation. + */ + public DaemonThreadFactory(String namePrefix, boolean propagateContextForTesting) { this.namePrefix = namePrefix; - this.propagateContext = propagateContext; + this.propagateContextForTesting = propagateContextForTesting; } @Override public Thread newThread(Runnable runnable) { - Thread t = delegate.newThread(propagateContext ? Context.current().wrap(runnable) : runnable); + Thread t = + delegate.newThread( + propagateContextForTesting ? Context.current().wrap(runnable) : runnable); try { t.setDaemon(true); t.setName(namePrefix + "-" + counter.incrementAndGet()); From 2dcdb09a1bce9cc86280d5d3469fdf1cf85e1653 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:31:10 +0100 Subject: [PATCH 20/24] Adding setter for OkHttpUtil.propagateContextForTestingInDispatcher --- .../exporter/sender/okhttp/internal/OkHttpUtil.java | 11 +++++++---- .../internal/AbstractOkHttpSuppressionTest.java | 4 ++-- .../sdk/internal/DaemonThreadFactory.java | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java index 36d0361e65d..738160a133f 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java @@ -18,9 +18,12 @@ * at any time. */ public final class OkHttpUtil { - // For testing purposes - @SuppressWarnings("NonFinalStaticField") - static boolean propagateContextInDispatcher = false; + private static boolean propagateContextForTestingInDispatcher = false; + + public static void setPropagateContextForTestingInDispatcher( + boolean propagateContextForTestingInDispatcher) { + OkHttpUtil.propagateContextForTestingInDispatcher = propagateContextForTestingInDispatcher; + } /** Returns a {@link Dispatcher} using daemon threads, otherwise matching the OkHttp default. */ public static Dispatcher newDispatcher() { @@ -31,7 +34,7 @@ public static Dispatcher newDispatcher() { 60, TimeUnit.SECONDS, new SynchronousQueue<>(), - new DaemonThreadFactory("okhttp-dispatch", propagateContextInDispatcher))); + new DaemonThreadFactory("okhttp-dispatch", propagateContextForTestingInDispatcher))); } private OkHttpUtil() {} diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java index 60addd0323a..d1a84a7491d 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/AbstractOkHttpSuppressionTest.java @@ -20,12 +20,12 @@ abstract class AbstractOkHttpSuppressionTest { @BeforeEach void setUp() { - OkHttpUtil.propagateContextInDispatcher = true; + OkHttpUtil.setPropagateContextForTestingInDispatcher(true); } @AfterEach void tearDown() { - OkHttpUtil.propagateContextInDispatcher = false; + OkHttpUtil.setPropagateContextForTestingInDispatcher(false); } @Test diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java index 0e1ece19ddf..de1073d777a 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java @@ -24,13 +24,13 @@ public final class DaemonThreadFactory implements ThreadFactory { private final boolean propagateContextForTesting; public DaemonThreadFactory(String namePrefix) { - this(namePrefix, /* propagateContext= */ false); + this(namePrefix, /* propagateContextForTesting= */ false); } /** * @param namePrefix Used when setting the new thread's name. * @param propagateContextForTesting For tests only. When enabled, the current thread's {@link - * Context} will be passed over to the new threads, this is useful for testing scenarios where + * Context} will be passed over to the new threads, this is useful for validating scenarios where * context propagation is available through bytecode instrumentation. */ public DaemonThreadFactory(String namePrefix, boolean propagateContextForTesting) { From 7ca697e7145139f773390d0fd9597f6c04b35b7b Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:31:33 +0100 Subject: [PATCH 21/24] Spotless --- .../io/opentelemetry/sdk/internal/DaemonThreadFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java index de1073d777a..76143f5cdfc 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java @@ -30,8 +30,8 @@ public DaemonThreadFactory(String namePrefix) { /** * @param namePrefix Used when setting the new thread's name. * @param propagateContextForTesting For tests only. When enabled, the current thread's {@link - * Context} will be passed over to the new threads, this is useful for validating scenarios where - * context propagation is available through bytecode instrumentation. + * Context} will be passed over to the new threads, this is useful for validating scenarios + * where context propagation is available through bytecode instrumentation. */ public DaemonThreadFactory(String namePrefix, boolean propagateContextForTesting) { this.namePrefix = namePrefix; From 7c2c16581683761709fa8b8cd6ed7c041032212f Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:34:24 +0100 Subject: [PATCH 22/24] Suppressing NonFinalStaticField warning --- .../exporter/sender/okhttp/internal/OkHttpUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java index 738160a133f..b641d1bad05 100644 --- a/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java +++ b/exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpUtil.java @@ -18,6 +18,7 @@ * at any time. */ public final class OkHttpUtil { + @SuppressWarnings("NonFinalStaticField") private static boolean propagateContextForTestingInDispatcher = false; public static void setPropagateContextForTestingInDispatcher( From 180a5237442d720b81c3e047f7fe693b99c3723a Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:41:51 +0100 Subject: [PATCH 23/24] Adding DaemonThreadFactory constructor's summary javadoc to avoid "SummaryJavadoc" style warning --- .../java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java index 76143f5cdfc..71152bd9567 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java @@ -28,6 +28,8 @@ public DaemonThreadFactory(String namePrefix) { } /** + * {@link DaemonThreadFactory's constructor}. + * * @param namePrefix Used when setting the new thread's name. * @param propagateContextForTesting For tests only. When enabled, the current thread's {@link * Context} will be passed over to the new threads, this is useful for validating scenarios From db23665f8326fb019f06b65e1634948bff7f4ae7 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:46:53 +0100 Subject: [PATCH 24/24] Fixing DaemonThreadFactory's constructor javadoc --- .../java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java index 71152bd9567..e8f75abe40f 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DaemonThreadFactory.java @@ -28,7 +28,7 @@ public DaemonThreadFactory(String namePrefix) { } /** - * {@link DaemonThreadFactory's constructor}. + * {@link DaemonThreadFactory}'s constructor. * * @param namePrefix Used when setting the new thread's name. * @param propagateContextForTesting For tests only. When enabled, the current thread's {@link