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.

Sign up for GitHub

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

Record exception for async servlet invocations #4677

Merged
merged 3 commits into from
Nov 22, 2021
Merged

Record exception for async servlet invocations #4677

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8644b4e
Record exception for asyn servlet invocations
laurit Nov 19, 2021
4da3587
add back accidentally commented out line
laurit Nov 19, 2021
f7d0b65
rearrange test so that it passes on both jetty and tomcat
laurit Nov 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,6 @@ class JettyServlet3TestFakeAsync extends JettyServlet3Test {
Class<Servlet> servlet() {
TestServlet3.FakeAsync
}

@Override
boolean testException() {
// we expect this request to fail with http 500 but is succeeds with http 200
// when using -PtestLatestDeps=true
false
}
}

class JettyServlet3TestForward extends JettyDispatchTest {
Expand Down
7 changes: 5 additions & 2 deletions instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import groovy.servlet.AbstractHttpServlet
import io.opentelemetry.instrumentation.test.base.HttpServerTest

import javax.servlet.RequestDispatcher
import javax.servlet.ServletException
import javax.servlet.annotation.WebServlet
Expand Down Expand Up @@ -72,6 +71,9 @@ class TestServlet3 {
HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath)
def latch = new CountDownLatch(1)
def context = req.startAsync()
if (endpoint == EXCEPTION) {
context.setTimeout(5000)
}
context.start {
try {
HttpServerTest.controller(endpoint) {
Expand Down Expand Up @@ -111,7 +113,6 @@ class TestServlet3 {
case EXCEPTION:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
context.complete()
throw new ServletException(endpoint.body)
}
}
Expand Down Expand Up @@ -157,6 +158,8 @@ class TestServlet3 {
resp.sendError(endpoint.status, endpoint.body)
break
case EXCEPTION:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
throw new ServletException(endpoint.body)
}
}
Expand Down
6 changes: 0 additions & 6 deletions instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/JettyServlet5Test.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ class JettyServlet5TestFakeAsync extends JettyServlet5Test {
Class<Servlet> servlet() {
TestServlet5.FakeAsync
}

@Override
boolean testException() {
// we expect this request to fail with http 500 but is succeeds with http 200
false
}
}

@IgnoreIf({ !jvm.java11Compatible })
Expand Down
6 changes: 5 additions & 1 deletion instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class TestServlet5 {
HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath)
def latch = new CountDownLatch(1)
def context = req.startAsync()
if (endpoint == EXCEPTION) {
context.setTimeout(5000)
}
context.start {
try {
HttpServerTest.controller(endpoint) {
Expand Down Expand Up @@ -111,7 +114,6 @@ class TestServlet5 {
case EXCEPTION:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
context.complete()
throw new ServletException(endpoint.body)
}
}
Expand Down Expand Up @@ -157,6 +159,8 @@ class TestServlet5 {
resp.sendError(endpoint.status, endpoint.body)
break
case EXCEPTION:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
throw new ServletException(endpoint.body)
}
}
Expand Down
16 changes: 7 additions & 9 deletions ...c/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/AsyncServlet.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class AsyncServlet extends HttpServlet {
HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath)
def latch = new CountDownLatch(1)
def context = req.startAsync()
if (endpoint == EXCEPTION) {
context.setTimeout(5000)
}
context.start {
try {
HttpServerTest.controller(endpoint) {
Expand All @@ -37,41 +40,36 @@ class AsyncServlet extends HttpServlet {
case SUCCESS:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
context.complete()
break
case INDEXED_CHILD:
endpoint.collectSpanAttributes { req.getParameter(it) }
resp.status = endpoint.status
context.complete()
break
case QUERY_PARAM:
resp.status = endpoint.status
resp.writer.print(req.queryString)
context.complete()
break
case REDIRECT:
resp.sendRedirect(endpoint.body)
context.complete()
break
case CAPTURE_HEADERS:
resp.setHeader("X-Test-Response", req.getHeader("X-Test-Request"))
resp.status = endpoint.status
resp.writer.print(endpoint.body)
context.complete()
break
case ERROR:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
context.complete()
break
case EXCEPTION:
resp.status = endpoint.status
resp.writer.print(endpoint.body)
context.complete()
throw new ServletException(endpoint.body)
}
}
} finally {
// complete at the end so the server span will end after the controller span
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

if (endpoint != EXCEPTION) {
context.complete()
}
latch.countDown()
}
}
Expand Down
7 changes: 6 additions & 1 deletion ...rc/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/AsyncServlet.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class AsyncServlet extends AbstractHttpServlet {
HttpServerTest.ServerEndpoint endpoint = HttpServerTest.ServerEndpoint.forPath(req.servletPath)
def latch = new CountDownLatch(1)
def context = req.startAsync()
if (endpoint == EXCEPTION) {
context.setTimeout(5000)
}
context.start {
try {
HttpServerTest.controller(endpoint) {
Expand Down Expand Up @@ -65,7 +68,9 @@ class AsyncServlet extends AbstractHttpServlet {
}
} finally {
// complete at the end so the server span will end after the controller span
context.complete()
if (endpoint != EXCEPTION) {
context.complete()
}
latch.countDown()
}
}
Expand Down