Skip to content

Commit 817ce57

Browse files
author
Anuraag Agrawal
authored
Make StrictContextStorage closeable. (open-telemetry#2776)
* Make StrictContextStorage closeable. * More javadoc
1 parent 74fed3e commit 817ce57

File tree

2 files changed

+10
-36
lines changed

2 files changed

+10
-36
lines changed

api/context/src/main/java/io/opentelemetry/context/StrictContextStorage.java

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -33,39 +33,13 @@
3333

3434
/**
3535
* A {@link ContextStorage} which keeps track of opened and closed {@link Scope}s, reporting caller
36-
* information if a {@link Scope} is closed incorrectly or not at all. This is useful in
37-
* instrumentation tests to check whether any scopes leaked.
36+
* information if a {@link Scope} is closed incorrectly or not at all.
3837
*
39-
* <pre>{@code
40-
* > class MyInstrumentationTest {
41-
* > private static ContextStorage previousStorage;
42-
* > private static StrictContextStorage strictStorage;
43-
* >
44-
* > @BeforeAll
45-
* > static void setUp() {
46-
* > previousStorage = ContextStorage.get()
47-
* > strictStorage = StrictContextStorage.create(previousStorage);
48-
* > ContextStorage.set(strictStorage);
49-
* > }
50-
* >
51-
* > @AfterEach
52-
* > void checkScopes() {
53-
* > strictStorage.ensureAllClosed();
54-
* > }
55-
* >
56-
* > @AfterAll
57-
* > static void tearDown() {
58-
* > ContextStorage.set(previousStorage);
59-
* > }
60-
* >
61-
* > @Test
62-
* > void badTest() {
63-
* > Context.root().makeCurrent();
64-
* > }
65-
* > }
66-
* }</pre>
38+
* <p>Calling {@link StrictContextStorage#close()} will check at the moment it's called whether
39+
* there are any scopes that have been opened but not closed yet. This could be called at the end of
40+
* a unit test to ensure the tested code cleaned up scopes correctly.
6741
*/
68-
final class StrictContextStorage implements ContextStorage {
42+
final class StrictContextStorage implements ContextStorage, AutoCloseable {
6943

7044
/**
7145
* Returns a new {@link StrictContextStorage} which delegates to the provided {@link
@@ -153,8 +127,8 @@ public Context current() {
153127
* @throws AssertionError if any scopes were left unclosed.
154128
*/
155129
// AssertionError to ensure test runners render the stack trace
156-
// Visible for testing
157-
void ensureAllClosed() {
130+
@Override
131+
public void close() {
158132
pendingScopes.expungeStaleEntries();
159133
List<CallerStackTrace> leaked = pendingScopes.drainPendingCallers();
160134
if (!leaked.isEmpty()) {

api/context/src/strictContextEnabledTest/java/io/opentelemetry/context/StrictContextStorageTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void decorator_close_afterCorrectUsage() {
6868
try (Scope ws2 = Context.current().with(ANIMAL, "dog").makeCurrent()) {}
6969
}
7070

71-
((StrictContextStorage) ContextStorage.get()).ensureAllClosed(); // doesn't error
71+
((StrictContextStorage) ContextStorage.get()).close(); // doesn't error
7272
}
7373

7474
static final class BusinessClass {
@@ -146,7 +146,7 @@ void decorator_close_withLeakedScope(Supplier<Scope> method, String methodName)
146146
thread.start();
147147
thread.join();
148148

149-
assertThatThrownBy(() -> ((StrictContextStorage) ContextStorage.get()).ensureAllClosed())
149+
assertThatThrownBy(() -> ((StrictContextStorage) ContextStorage.get()).close())
150150
.isInstanceOf(AssertionError.class)
151151
.satisfies(
152152
t -> assertThat(t.getMessage()).matches("Thread \\[t1\\] opened a scope of .* here:"))
@@ -162,7 +162,7 @@ static void assertStackTraceStartsWithMethod(Throwable throwable, String methodN
162162
void multipleLeaks() {
163163
Scope scope1 = Context.current().with(ANIMAL, "cat").makeCurrent();
164164
Scope scope2 = Context.current().with(ANIMAL, "dog").makeCurrent();
165-
assertThatThrownBy(() -> ((StrictContextStorage) ContextStorage.get()).ensureAllClosed())
165+
assertThatThrownBy(() -> ((StrictContextStorage) ContextStorage.get()).close())
166166
.isInstanceOf(AssertionError.class);
167167
}
168168

0 commit comments

Comments
 (0)