Skip to content

Commit d1aecc9

Browse files
karle0wnekarle0wne
authored
guarantee MDC cleanup plus span termination when traces are absent (#73)
1 parent 7c47e1a commit d1aecc9

12 files changed

Lines changed: 164 additions & 52 deletions

File tree

‎.github/workflows/basic-linters.yml‎

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎.github/workflows/build.yml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
name: Maven Build Artifact
22

33
on:
4-
pull_request:
4+
push:
55
branches:
66
- '*'
77

88
jobs:
99
build:
10-
uses: valitydev/java-workflow/.github/workflows/maven-library-build.yml@v2
10+
uses: valitydev/java-workflow/.github/workflows/maven-library-build.yml@v3
11+
with:
12+
java-version: "11"

‎.github/workflows/deploy.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ on:
55
branches:
66
- 'master'
77
- 'main'
8+
- 'epic/**'
89

910
jobs:
1011
deploy:
11-
uses: valitydev/java-workflow/.github/workflows/maven-library-deploy.yml@v2
12+
uses: valitydev/java-workflow/.github/workflows/maven-library-deploy.yml@v3
13+
with:
14+
java-version: "11"
1215
secrets:
1316
server-username: ${{ secrets.OSSRH_USERNAME }}
1417
server-password: ${{ secrets.OSSRH_TOKEN }}

‎libthrift/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<parent>
66
<artifactId>woody</artifactId>
77
<groupId>dev.vality.woody</groupId>
8-
<version>2.0.9</version>
8+
<version>${revision}</version>
99
</parent>
1010
<modelVersion>4.0.0</modelVersion>
1111

‎pom.xml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
<parent>
66
<groupId>dev.vality</groupId>
77
<artifactId>library-parent-pom</artifactId>
8-
<version>2.0.1</version>
8+
<version>3.1.0</version>
99
</parent>
1010

1111
<modelVersion>4.0.0</modelVersion>
1212
<packaging>pom</packaging>
1313
<groupId>dev.vality.woody</groupId>
1414
<artifactId>woody</artifactId>
15-
<version>2.0.9</version>
15+
<version>${revision}</version>
1616

1717
<name>Woody Java</name>
1818
<description>Java implementation for Woody spec</description>
@@ -40,6 +40,7 @@
4040
</scm>
4141

4242
<properties>
43+
<revision>2.0.10</revision>
4344
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
4445
<maven.compiler.source>11</maven.compiler.source>
4546
<maven.compiler.target>11</maven.compiler.target>

‎woody-api/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<parent>
66
<artifactId>woody</artifactId>
77
<groupId>dev.vality.woody</groupId>
8-
<version>2.0.9</version>
8+
<version>${revision}</version>
99
</parent>
1010
<modelVersion>4.0.0</modelVersion>
1111

‎woody-api/src/main/java/dev/vality/woody/api/interceptor/ContextInterceptor.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package dev.vality.woody.api.interceptor;
22

3+
import dev.vality.woody.api.MDCUtils;
34
import dev.vality.woody.api.trace.ContextUtils;
45
import dev.vality.woody.api.trace.TraceData;
56
import dev.vality.woody.api.trace.context.TraceContext;
@@ -14,6 +15,7 @@ public class ContextInterceptor implements CommonInterceptor {
1415

1516
private final TraceContext traceContext;
1617
private final CommonInterceptor interceptor;
18+
private final ThreadLocal<Boolean> contextInitialized = ThreadLocal.withInitial(() -> Boolean.FALSE);
1719

1820
public ContextInterceptor(TraceContext traceContext, CommonInterceptor interceptor) {
1921
this.traceContext = Objects.requireNonNull(traceContext, "TraceContext can't be null");
@@ -23,10 +25,14 @@ public ContextInterceptor(TraceContext traceContext, CommonInterceptor intercept
2325
@Override
2426
public boolean interceptRequest(TraceData traceData, Object providerContext, Object... contextParams) {
2527
LOG.trace("Intercept request context");
26-
if (!TraceContext.getCurrentTraceData().getServiceSpan().isFilled()) {
27-
throw new IllegalStateException("TraceContext service span must be filled");
28+
boolean spanFilled = TraceContext.getCurrentTraceData() != null
29+
&& TraceContext.getCurrentTraceData().getServiceSpan().isFilled();
30+
if (spanFilled) {
31+
traceContext.init();
32+
} else {
33+
LOG.trace("Skipping trace context init due to empty service span");
2834
}
29-
traceContext.init();
35+
contextInitialized.set(spanFilled);
3036
return interceptor.interceptRequest(traceData, providerContext, contextParams);
3137
}
3238

@@ -36,7 +42,19 @@ public boolean interceptResponse(TraceData traceData, Object providerContext, Ob
3642
try {
3743
return interceptor.interceptResponse(traceData, providerContext, contextParams);
3844
} finally {
39-
traceContext.destroy(ContextUtils.hasCallErrors(traceData.getActiveSpan()));
45+
Boolean initialized = contextInitialized.get();
46+
try {
47+
if (Boolean.TRUE.equals(initialized)
48+
&& traceData != null
49+
&& traceData.getServiceSpan().isFilled()) {
50+
traceContext.destroy(ContextUtils.hasCallErrors(traceData.getActiveSpan()));
51+
} else {
52+
TraceContext.reset();
53+
MDCUtils.removeSpanData();
54+
}
55+
} finally {
56+
contextInitialized.remove();
57+
}
4058
}
4159
}
4260
}

‎woody-api/src/main/java/dev/vality/woody/api/trace/context/TraceContext.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ public void destroy() {
166166

167167
public void destroy(boolean onError) {
168168
TraceData traceData = getCurrentTraceData();
169+
if (traceData == null) {
170+
MDCUtils.removeSpanData();
171+
return;
172+
}
169173
boolean isClient = isClientDestroy(traceData);
170174
setDuration(traceData, isClient);
171175
try {
@@ -175,15 +179,22 @@ public void destroy(boolean onError) {
175179
preDestroy.run();
176180
}
177181
} finally {
182+
TraceData restored;
178183
if (isClient) {
179-
traceData = destroyClientContext(traceData);
184+
restored = destroyClientContext(traceData);
185+
if (restored == null) {
186+
setCurrentTraceData(null);
187+
MDCUtils.removeSpanData();
188+
traceData.getOtelSpan().end();
189+
return;
190+
}
180191
} else {
181-
traceData = destroyServiceContext(traceData);
192+
restored = destroyServiceContext(traceData);
182193
}
183-
setCurrentTraceData(traceData);
194+
setCurrentTraceData(restored);
184195

185-
if (traceData.getServiceSpan().isFilled()) {
186-
MDCUtils.putSpanData(traceData.getServiceSpan().getSpan(), traceData.getOtelSpan());
196+
if (restored.getServiceSpan().isFilled()) {
197+
MDCUtils.putSpanData(restored.getServiceSpan().getSpan(), restored.getOtelSpan());
187198
} else {
188199
MDCUtils.removeSpanData();
189200
}

‎woody-api/src/test/java/dev/vality/woody/api/interceptor/ContextInterceptorTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package dev.vality.woody.api.interceptor;
2+
3+
import dev.vality.woody.api.MDCUtils;
4+
import dev.vality.woody.api.trace.TraceData;
5+
import dev.vality.woody.api.trace.context.TraceContext;
6+
import org.junit.After;
7+
import org.junit.Before;
8+
import org.junit.Test;
9+
import org.slf4j.MDC;
10+
11+
import static org.junit.Assert.*;
12+
13+
public class ContextInterceptorTest {
14+
15+
private TraceData originalTraceData;
16+
private TraceData testTraceData;
17+
18+
@Before
19+
public void setUp() {
20+
originalTraceData = TraceContext.getCurrentTraceData();
21+
testTraceData = new TraceData();
22+
TraceContext.setCurrentTraceData(testTraceData);
23+
MDC.clear();
24+
}
25+
26+
@After
27+
public void tearDown() {
28+
MDC.clear();
29+
if (testTraceData != null) {
30+
testTraceData.getOtelSpan().end();
31+
}
32+
TraceContext.setCurrentTraceData(originalTraceData);
33+
}
34+
35+
@Test
36+
public void skipInitWhenServiceSpanEmpty() {
37+
RecordingInterceptor delegate = new RecordingInterceptor();
38+
ContextInterceptor interceptor = new ContextInterceptor(TraceContext.forService(), delegate);
39+
40+
TraceData current = TraceContext.getCurrentTraceData();
41+
MDC.put(MDCUtils.SPAN_ID, "existing");
42+
43+
assertTrue(interceptor.interceptRequest(current, null));
44+
assertTrue(delegate.requestInvoked);
45+
46+
interceptor.interceptResponse(current, null);
47+
48+
assertTrue(delegate.responseInvoked);
49+
assertNull(MDC.get(MDCUtils.SPAN_ID));
50+
assertFalse(TraceContext.getCurrentTraceData().getServiceSpan().isFilled());
51+
}
52+
53+
@Test
54+
public void destroyWithoutTraceDataClearsMdc() {
55+
TraceContext.setCurrentTraceData(null);
56+
MDC.put(MDCUtils.SPAN_ID, "value");
57+
58+
TraceContext.forService().destroy();
59+
60+
assertNull(MDC.get(MDCUtils.SPAN_ID));
61+
TraceContext.setCurrentTraceData(testTraceData);
62+
}
63+
64+
private static class RecordingInterceptor extends EmptyCommonInterceptor {
65+
private boolean requestInvoked;
66+
private boolean responseInvoked;
67+
68+
@Override
69+
public boolean interceptRequest(TraceData traceData, Object providerContext, Object... contextParams) {
70+
requestInvoked = true;
71+
return super.interceptRequest(traceData, providerContext, contextParams);
72+
}
73+
74+
@Override
75+
public boolean interceptResponse(TraceData traceData, Object providerContext, Object... contextParams) {
76+
responseInvoked = true;
77+
return super.interceptResponse(traceData, providerContext, contextParams);
78+
}
79+
}
80+
}

‎woody-thrift/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<parent>
66
<artifactId>woody</artifactId>
77
<groupId>dev.vality.woody</groupId>
8-
<version>2.0.9</version>
8+
<version>${revision}</version>
99
</parent>
1010
<modelVersion>4.0.0</modelVersion>
1111

0 commit comments

Comments
 (0)