Skip to content

Commit 39a68b3

Browse files
amarzialiamarziali
authored
Avoid finishing twice a servlet 3 async dispatch span (#7395)
* Remove phaser barriers from servlet3 async tests * Don't finish spans twice in async dispatch
1 parent 567c4e9 commit 39a68b3

2 files changed

Lines changed: 60 additions & 63 deletions

File tree

‎dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/FinishAsyncDispatchListener.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,15 @@ public void onComplete(final AsyncEvent event) throws IOException {
5555
}
5656
}
5757
DECORATE.beforeFinish(span);
58-
span.finish();
58+
maybeFinishSpan();
5959
}
6060
}
6161

6262
@Override
6363
public void run() {
6464
if (activated.compareAndSet(false, true)) {
6565
DECORATE.beforeFinish(span);
66-
span.finish();
66+
maybeFinishSpan();
6767
}
6868
}
6969

@@ -75,7 +75,7 @@ public void onTimeout(final AsyncEvent event) throws IOException {
7575
}
7676
span.setTag(TIMEOUT, event.getAsyncContext().getTimeout());
7777
DECORATE.beforeFinish(span);
78-
span.finish();
78+
maybeFinishSpan();
7979
}
8080
}
8181

@@ -84,7 +84,7 @@ public void onError(final AsyncEvent event) throws IOException {
8484
if (event.getThrowable() != null && activated.compareAndSet(false, true)) {
8585
DECORATE.onError(span, event.getThrowable());
8686
DECORATE.beforeFinish(span);
87-
span.finish();
87+
maybeFinishSpan();
8888
}
8989
}
9090

@@ -93,4 +93,10 @@ public void onError(final AsyncEvent event) throws IOException {
9393
public void onStartAsync(final AsyncEvent event) throws IOException {
9494
event.getAsyncContext().addListener(this);
9595
}
96+
97+
private void maybeFinishSpan() {
98+
if (span.phasedFinish()) {
99+
span.publish();
100+
}
101+
}
96102
}

‎dd-java-agent/instrumentation/servlet/request-3/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/TestServlet3.groovy‎

Lines changed: 50 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import javax.servlet.http.HttpServletRequest
1212
import javax.servlet.http.HttpServletResponse
1313
import java.lang.reflect.Field
1414
import java.lang.reflect.Modifier
15-
import java.util.concurrent.Phaser
1615

1716
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED
1817
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
@@ -128,7 +127,6 @@ class TestServlet3 {
128127
@Override
129128
protected void service(HttpServletRequest req, HttpServletResponse resp) {
130129
HttpServerTest.ServerEndpoint endpoint = getEndpoint(req)
131-
def phaser = new Phaser(2)
132130
def context = req.startAsync()
133131
context.setTimeout(SERVLET_TIMEOUT)
134132
if (resp.class.name.startsWith("org.eclipse.jetty")) {
@@ -149,66 +147,59 @@ class TestServlet3 {
149147
})
150148
}
151149
context.start {
152-
try {
153-
phaser.arrive()
154-
HttpServerTest.controller(endpoint) {
155-
resp.contentType = "text/plain"
156-
resp.addHeader(HttpServerTest.IG_RESPONSE_HEADER, HttpServerTest.IG_RESPONSE_HEADER_VALUE)
157-
switch (endpoint) {
158-
case SUCCESS:
159-
resp.status = endpoint.status
160-
resp.writer.print(endpoint.body)
161-
context.complete()
162-
break
163-
case CREATED:
164-
resp.status = endpoint.status
165-
resp.writer.print("${endpoint.body}: ${req.reader.text}")
166-
break
167-
case CREATED_IS:
168-
resp.status = endpoint.status
169-
resp.writer.print("${endpoint.body}: ${req.inputStream.getText('UTF-8')}")
170-
break
171-
case FORWARDED:
172-
resp.status = endpoint.status
173-
resp.writer.print(req.getHeader("x-forwarded-for"))
174-
context.complete()
175-
break
176-
case QUERY_ENCODED_BOTH:
177-
case QUERY_ENCODED_QUERY:
178-
case QUERY_PARAM:
179-
resp.status = endpoint.status
180-
resp.writer.print(endpoint.bodyForQuery(req.queryString))
181-
context.complete()
182-
break
183-
case REDIRECT:
184-
resp.sendRedirect(endpoint.body)
185-
context.complete()
186-
break
187-
case ERROR:
188-
resp.sendError(endpoint.status, endpoint.body)
189-
context.complete()
190-
break
191-
case EXCEPTION:
192-
throw new Exception(endpoint.body)
193-
case CUSTOM_EXCEPTION:
194-
throw new InputMismatchException(endpoint.body)
195-
case TIMEOUT:
196-
case TIMEOUT_ERROR:
197-
sleep context.getTimeout() + 10
198-
break
199-
case USER_BLOCK:
200-
Blocking.forUser('user-to-block').blockIfMatch()
201-
resp.writer.print('should not be reached')
202-
context.complete()
203-
break
204-
}
150+
HttpServerTest.controller(endpoint) {
151+
resp.contentType = "text/plain"
152+
resp.addHeader(HttpServerTest.IG_RESPONSE_HEADER, HttpServerTest.IG_RESPONSE_HEADER_VALUE)
153+
switch (endpoint) {
154+
case SUCCESS:
155+
resp.status = endpoint.status
156+
resp.writer.print(endpoint.body)
157+
context.complete()
158+
break
159+
case CREATED:
160+
resp.status = endpoint.status
161+
resp.writer.print("${endpoint.body}: ${req.reader.text}")
162+
break
163+
case CREATED_IS:
164+
resp.status = endpoint.status
165+
resp.writer.print("${endpoint.body}: ${req.inputStream.getText('UTF-8')}")
166+
break
167+
case FORWARDED:
168+
resp.status = endpoint.status
169+
resp.writer.print(req.getHeader("x-forwarded-for"))
170+
context.complete()
171+
break
172+
case QUERY_ENCODED_BOTH:
173+
case QUERY_ENCODED_QUERY:
174+
case QUERY_PARAM:
175+
resp.status = endpoint.status
176+
resp.writer.print(endpoint.bodyForQuery(req.queryString))
177+
context.complete()
178+
break
179+
case REDIRECT:
180+
resp.sendRedirect(endpoint.body)
181+
context.complete()
182+
break
183+
case ERROR:
184+
resp.sendError(endpoint.status, endpoint.body)
185+
context.complete()
186+
break
187+
case EXCEPTION:
188+
throw new Exception(endpoint.body)
189+
case CUSTOM_EXCEPTION:
190+
throw new InputMismatchException(endpoint.body)
191+
case TIMEOUT:
192+
case TIMEOUT_ERROR:
193+
sleep context.getTimeout() + 10
194+
break
195+
case USER_BLOCK:
196+
Blocking.forUser('user-to-block').blockIfMatch()
197+
resp.writer.print('should not be reached')
198+
context.complete()
199+
break
205200
}
206-
} finally {
207-
phaser.arriveAndDeregister()
208201
}
209202
}
210-
phaser.arriveAndAwaitAdvance()
211-
phaser.arriveAndAwaitAdvance()
212203
}
213204
}
214205

0 commit comments

Comments
 (0)