From 17b0dd0b66ad66e2aaf369fa2a21df1cb073b428 Mon Sep 17 00:00:00 2001 From: Michal Dvorak Date: Fri, 8 Mar 2019 09:15:26 +0100 Subject: [PATCH 1/3] Fixes #334 - Add ScopeListener extension point to ThreadLocalScopeManager --- .../opentracing/util/NoopScopeListener.java | 31 +++++++++++++++++++ .../io/opentracing/util/ScopeListener.java | 23 ++++++++++++++ .../io/opentracing/util/ThreadLocalScope.java | 9 +++++- .../util/ThreadLocalScopeManager.java | 10 ++++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java create mode 100644 opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java diff --git a/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java b/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java new file mode 100644 index 00000000..700d6131 --- /dev/null +++ b/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java @@ -0,0 +1,31 @@ +/* + * Copyright 2016-2019 The OpenTracing Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package io.opentracing.util; + +import io.opentracing.Span; + +public interface NoopScopeListener extends ScopeListener { + NoopScopeListener INSTANCE = new NoopScopeListenerImpl(); +} + +class NoopScopeListenerImpl implements NoopScopeListener { + + @Override + public void onActivate(Span span) { + } + + @Override + public void onClose() { + } +} diff --git a/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java b/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java new file mode 100644 index 00000000..9da7aac9 --- /dev/null +++ b/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java @@ -0,0 +1,23 @@ +/* + * Copyright 2016-2019 The OpenTracing Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package io.opentracing.util; + +import io.opentracing.Span; + +public interface ScopeListener { + + void onActivate(Span span); + + void onClose(); +} diff --git a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java index e3a2f2dd..2de3093d 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java @@ -35,6 +35,7 @@ public class ThreadLocalScope implements Scope { this.finishOnClose = finishOnClose; this.toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); + scopeManager.listener.onActivate(wrapped); } @Override @@ -48,7 +49,13 @@ public void close() { wrapped.finish(); } - scopeManager.tlsScope.set(toRestore); + if (toRestore != null) { + scopeManager.tlsScope.set(toRestore); + scopeManager.listener.onActivate(toRestore.wrapped); + } else { + scopeManager.tlsScope.remove(); + scopeManager.listener.onClose(); + } } @Override diff --git a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java index 7ed13f59..14b678a3 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java @@ -23,7 +23,17 @@ * @see ThreadLocalScope */ public class ThreadLocalScopeManager implements ScopeManager { + final ThreadLocal tlsScope = new ThreadLocal(); + final ScopeListener listener; + + public ThreadLocalScopeManager() { + this(null); + } + + public ThreadLocalScopeManager(ScopeListener listener) { + this.listener = listener != null ? listener : NoopScopeListener.INSTANCE; + } @Override public Scope activate(Span span, boolean finishOnClose) { From 5ae192fdfc0648ed0e9a8555f0e6f4dac66bf74b Mon Sep 17 00:00:00 2001 From: Michal Dvorak Date: Fri, 15 Mar 2019 09:44:07 +0100 Subject: [PATCH 2/3] Closes #334 - Tests and docs for ScopeListener --- .../opentracing/util/NoopScopeListener.java | 3 +++ .../io/opentracing/util/ScopeListener.java | 21 +++++++++++++++++++ .../util/ThreadLocalScopeManager.java | 12 +++++++++++ .../util/ThreadLocalScopeTest.java | 16 ++++++++++++-- 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java b/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java index 700d6131..1a6a61d1 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java +++ b/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java @@ -19,6 +19,9 @@ public interface NoopScopeListener extends ScopeListener { NoopScopeListener INSTANCE = new NoopScopeListenerImpl(); } +/** + * A noop (i.e., cheap-as-possible) implementation of a ScopeListener. + */ class NoopScopeListenerImpl implements NoopScopeListener { @Override diff --git a/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java b/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java index 9da7aac9..3a5b1edd 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java @@ -13,11 +13,32 @@ */ package io.opentracing.util; +import io.opentracing.Scope; +import io.opentracing.ScopeManager; import io.opentracing.Span; +/** + * Listener that can react on changes of currently active {@link Span}. + *

+ * The {@link #onActivate(Span)} method will be called, whenever scope changes - that can be both + * as result of a {@link ScopeManager#activate(Span, boolean)} call or when {@link Scope#close()} + * is closed on a nested scope. + *

+ * {@link #onClose()} is called when closing outermost scope - meaning no scope is currently active. + * + * @see ThreadLocalScopeManager + */ public interface ScopeListener { + /** + * Called whenever a scope was activated (changed). + * + * @param span Activated span. Never null. + */ void onActivate(Span span); + /** + * Called when outermost scope was deactivated. + */ void onClose(); } diff --git a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java index 14b678a3..76e1f44a 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScopeManager.java @@ -19,6 +19,10 @@ /** * A simple {@link ScopeManager} implementation built on top of Java's thread-local storage primitive. + *

+ * Optionally supports {@link ScopeListener}, to perform additional actions when scope is changed for given thread. + * Listener methods are always called synchronously on the same thread, right after activation (meaning {@link #active()} + * already returns new a scope). * * @see ThreadLocalScope */ @@ -27,10 +31,18 @@ public class ThreadLocalScopeManager implements ScopeManager { final ThreadLocal tlsScope = new ThreadLocal(); final ScopeListener listener; + /** + * Default constructor for {@link ThreadLocalScopeManager}, without any listener. + */ public ThreadLocalScopeManager() { this(null); } + /** + * Constructs {@link ThreadLocalScopeManager} with custom {@link ScopeListener}. + * + * @param listener Listener to register. When null, noop listener will be used. + */ public ThreadLocalScopeManager(ScopeListener listener) { this.listener = listener != null ? listener : NoopScopeListener.INSTANCE; } diff --git a/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java b/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java index 874d9a9a..25be9d87 100644 --- a/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java +++ b/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java @@ -27,10 +27,12 @@ public class ThreadLocalScopeTest { private ThreadLocalScopeManager scopeManager; + private ScopeListener scopeListener; @Before public void before() throws Exception { - scopeManager = new ThreadLocalScopeManager(); + scopeListener = mock(ScopeListener.class); + scopeManager = new ThreadLocalScopeManager(scopeListener); } @Test @@ -63,6 +65,11 @@ public void implicitSpanStack() throws Exception { verify(backgroundSpan, times(1)).finish(); verify(foregroundSpan, times(1)).finish(); + // Verify listener calls + verify(scopeListener, times(2)).onActivate(backgroundSpan); + verify(scopeListener, times(1)).onActivate(foregroundSpan); + verify(scopeListener, times(1)).onClose(); + // And now nothing is active. Scope missingSpan = scopeManager.active(); assertNull(missingSpan); @@ -71,11 +78,16 @@ public void implicitSpanStack() throws Exception { @Test public void testDeactivateWhenDifferentSpanIsActive() { Span span = mock(Span.class); + Span nestedSpan = mock(Span.class); Scope active = scopeManager.activate(span, false); - scopeManager.activate(mock(Span.class), false); + scopeManager.activate(nestedSpan, false); active.close(); verify(span, times(0)).finish(); + + verify(scopeListener, times(1)).onActivate(span); + verify(scopeListener, times(1)).onActivate(nestedSpan); + verify(scopeListener, times(0)).onClose(); } } From 4fa85ad3c81d1c6a8fb40ef2afe58b3a5245439b Mon Sep 17 00:00:00 2001 From: Michal Dvorak Date: Fri, 15 Mar 2019 17:12:30 +0100 Subject: [PATCH 3/3] Renamed ScopeListener methods to past tense --- .../java/io/opentracing/util/NoopScopeListener.java | 4 ++-- .../main/java/io/opentracing/util/ScopeListener.java | 8 ++++---- .../java/io/opentracing/util/ThreadLocalScope.java | 6 +++--- .../io/opentracing/util/ThreadLocalScopeTest.java | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java b/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java index 1a6a61d1..c8f044ec 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java +++ b/opentracing-util/src/main/java/io/opentracing/util/NoopScopeListener.java @@ -25,10 +25,10 @@ public interface NoopScopeListener extends ScopeListener { class NoopScopeListenerImpl implements NoopScopeListener { @Override - public void onActivate(Span span) { + public void onActivated(Span span) { } @Override - public void onClose() { + public void onClosed() { } } diff --git a/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java b/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java index 3a5b1edd..4ce34a81 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ScopeListener.java @@ -20,11 +20,11 @@ /** * Listener that can react on changes of currently active {@link Span}. *

- * The {@link #onActivate(Span)} method will be called, whenever scope changes - that can be both + * The {@link #onActivated} method will be called, whenever scope changes - that can be both * as result of a {@link ScopeManager#activate(Span, boolean)} call or when {@link Scope#close()} * is closed on a nested scope. *

- * {@link #onClose()} is called when closing outermost scope - meaning no scope is currently active. + * {@link #onClosed} is called when closing outermost scope - meaning no scope is currently active. * * @see ThreadLocalScopeManager */ @@ -35,10 +35,10 @@ public interface ScopeListener { * * @param span Activated span. Never null. */ - void onActivate(Span span); + void onActivated(Span span); /** * Called when outermost scope was deactivated. */ - void onClose(); + void onClosed(); } diff --git a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java index 2de3093d..d98b425c 100644 --- a/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java +++ b/opentracing-util/src/main/java/io/opentracing/util/ThreadLocalScope.java @@ -35,7 +35,7 @@ public class ThreadLocalScope implements Scope { this.finishOnClose = finishOnClose; this.toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); - scopeManager.listener.onActivate(wrapped); + scopeManager.listener.onActivated(wrapped); } @Override @@ -51,10 +51,10 @@ public void close() { if (toRestore != null) { scopeManager.tlsScope.set(toRestore); - scopeManager.listener.onActivate(toRestore.wrapped); + scopeManager.listener.onActivated(toRestore.wrapped); } else { scopeManager.tlsScope.remove(); - scopeManager.listener.onClose(); + scopeManager.listener.onClosed(); } } diff --git a/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java b/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java index 25be9d87..a94fd620 100644 --- a/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java +++ b/opentracing-util/src/test/java/io/opentracing/util/ThreadLocalScopeTest.java @@ -66,9 +66,9 @@ public void implicitSpanStack() throws Exception { verify(foregroundSpan, times(1)).finish(); // Verify listener calls - verify(scopeListener, times(2)).onActivate(backgroundSpan); - verify(scopeListener, times(1)).onActivate(foregroundSpan); - verify(scopeListener, times(1)).onClose(); + verify(scopeListener, times(2)).onActivated(backgroundSpan); + verify(scopeListener, times(1)).onActivated(foregroundSpan); + verify(scopeListener, times(1)).onClosed(); // And now nothing is active. Scope missingSpan = scopeManager.active(); @@ -86,8 +86,8 @@ public void testDeactivateWhenDifferentSpanIsActive() { verify(span, times(0)).finish(); - verify(scopeListener, times(1)).onActivate(span); - verify(scopeListener, times(1)).onActivate(nestedSpan); - verify(scopeListener, times(0)).onClose(); + verify(scopeListener, times(1)).onActivated(span); + verify(scopeListener, times(1)).onActivated(nestedSpan); + verify(scopeListener, times(0)).onClosed(); } }