Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add helper class to capture context using ScheduledExecutorService #6712

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Prev Previous commit
Avoid wrapping a previously wrapped (Scheduled)ExecutorService
  • Loading branch information
ammachado committed Sep 19, 2024
commit 0629d7968f297c4ef7438063d1851070528b183c
6 changes: 6 additions & 0 deletions context/src/main/java/io/opentelemetry/context/Context.java
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ static Executor taskWrapping(Executor executor) {
* @since 1.1.0
*/
static ExecutorService taskWrapping(ExecutorService executorService) {
if (executorService instanceof CurrentContextExecutorService) {
return executorService;
}
return new CurrentContextExecutorService(executorService);
}

Expand All @@ -153,6 +156,9 @@ static ExecutorService taskWrapping(ExecutorService executorService) {
* @since 1.43.0
*/
static ScheduledExecutorService taskWrapping(ScheduledExecutorService executorService) {
ammachado marked this conversation as resolved.
Show resolved Hide resolved
if (executorService instanceof CurrentContextScheduledExecutorService) {
return executorService;
}
return new CurrentContextScheduledExecutorService(executorService);
}

Expand Down
28 changes: 21 additions & 7 deletions context/src/test/java/io/opentelemetry/context/ContextTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -121,7 +122,7 @@ void newThreadStartsWithRoot() throws Exception {
}

@Test
public void closingScopeWhenNotActiveIsNoopAndLogged() {
void closingScopeWhenNotActiveIsNoopAndLogged() {
Context initial = Context.current();
Context context = initial.with(ANIMAL, "cat");
try (Scope scope = context.makeCurrent()) {
Expand All @@ -140,7 +141,7 @@ public void closingScopeWhenNotActiveIsNoopAndLogged() {

@SuppressWarnings("MustBeClosedChecker")
@Test
public void closeScopeIsIdempotent() {
void closeScopeIsIdempotent() {
Context initial = Context.current();
Context context1 = Context.root().with(ANIMAL, "cat");
Scope scope1 = context1.makeCurrent();
Expand Down Expand Up @@ -191,8 +192,7 @@ void withValues() {
assertThat(context5).isSameAs(context4);

String dog = new String("dog");
assertThat(dog).isEqualTo("dog");
assertThat(dog).isNotSameAs("dog");
assertThat(dog).isEqualTo("dog").isNotSameAs("dog");
Context context6 = context5.with(ANIMAL, dog);
assertThat(context6.get(ANIMAL)).isEqualTo("dog");
// We reuse the context object when values match by reference, not value.
Expand Down Expand Up @@ -237,7 +237,7 @@ void wrapCallable() throws Exception {
void wrapFunction() {
AtomicReference<String> value = new AtomicReference<>();
Function<String, String> callback =
(a) -> {
a -> {
value.set(Context.current().get(ANIMAL));
return "foo";
};
Expand Down Expand Up @@ -276,7 +276,7 @@ void wrapConsumer() {
AtomicReference<String> value = new AtomicReference<>();
AtomicBoolean consumed = new AtomicBoolean();
Consumer<String> callback =
(a) -> {
a -> {
value.set(Context.current().get(ANIMAL));
consumed.set(true);
};
Expand Down Expand Up @@ -867,7 +867,7 @@ void delegatesCleanupMethods() throws Exception {

@Test
void emptyContext() {
assertThat(Context.root().get(new HashCollidingKey())).isEqualTo(null);
assertThat(Context.root().get(new HashCollidingKey())).isNull();
}

@Test
Expand All @@ -890,6 +890,20 @@ void hashcodeCollidingKeys() {
assertThat(twoKeys.get(cheese)).isEqualTo("whiz");
}

@Test
void doNotWrapExecutorService() {
ExecutorService executor = mock(CurrentContextExecutorService.class);
ExecutorService wrapped = Context.taskWrapping(executor);
assertThat(wrapped).isSameAs(executor);
}

@Test
void doNotWrapScheduledExecutorService() {
ScheduledExecutorService executor = mock(CurrentContextScheduledExecutorService.class);
ScheduledExecutorService wrapped = Context.taskWrapping(executor);
assertThat(wrapped).isSameAs(executor);
}

@SuppressWarnings("HashCodeToString")
private static class HashCollidingKey implements ContextKey<String> {
@Override
Expand Down
Loading