Jsf: simplify asserting exception (#11736)

open-telemetry · Jul 4, 2024 · e930b09 · e930b09
1 parent 4439dec
commit e930b09
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 39 deletions.
diff --git a/...ing/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/jakarta/BaseJsfTest.java b/...ing/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/jakarta/BaseJsfTest.java
@@ -17,7 +17,6 @@
 import io.opentelemetry.sdk.testing.assertj.TraceAssert;
 import io.opentelemetry.sdk.trace.data.StatusData;
 import io.opentelemetry.semconv.ClientAttributes;
-import io.opentelemetry.semconv.ExceptionAttributes;
 import io.opentelemetry.semconv.HttpAttributes;
 import io.opentelemetry.semconv.NetworkAttributes;
 import io.opentelemetry.semconv.ServerAttributes;
@@ -30,7 +29,6 @@
 import io.opentelemetry.testing.internal.armeria.common.MediaType;
 import io.opentelemetry.testing.internal.armeria.common.QueryParams;
 import io.opentelemetry.testing.internal.armeria.common.RequestHeaders;
-import jakarta.servlet.ServletException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -257,7 +255,7 @@ void testException() {
     AggregatedHttpResponse response2 = client.execute(request2).aggregate().join();
     assertThat(response2.status().code()).isEqualTo(500);
 
-    ServletException ex = new ServletException("submit exception");
+    IllegalStateException expectedException = new IllegalStateException("submit exception");
 
     testing.waitAndAssertTraces(
         trace ->
@@ -267,21 +265,8 @@ void testException() {
                         .hasKind(SpanKind.SERVER)
                         .hasNoParent()
                         .hasStatus(StatusData.error())
-                        .hasEventsSatisfyingExactly(
-                            event ->
-                                event
-                                    .hasName("exception")
-                                    .hasAttributesSatisfyingExactly(
-                                        equalTo(
-                                            ExceptionAttributes.EXCEPTION_TYPE,
-                                            ex.getClass().getName()),
-                                        satisfies(
-                                            ExceptionAttributes.EXCEPTION_STACKTRACE,
-                                            stacktrace -> stacktrace.contains("submit exception")),
-                                        satisfies(
-                                            ExceptionAttributes.EXCEPTION_MESSAGE,
-                                            message -> message.endsWith(ex.getMessage())))),
-                span -> handlerSpan(trace, 0, "#{greetingForm.submit()}", ex)));
+                        .hasException(expectedException),
+                span -> handlerSpan(trace, 0, "#{greetingForm.submit()}", expectedException)));
   }
 
   List<Consumer<SpanDataAssert>> handlerSpan(
@@ -295,22 +280,7 @@ List<Consumer<SpanDataAssert>> handlerSpan(
                         .hasParent(trace.getSpan(parentIndex))));
 
     if (expectedException != null) {
-      assertions.add(
-          span ->
-              span.hasStatus(StatusData.error())
-                  .hasEventsSatisfyingExactly(
-                      event ->
-                          event
-                              .hasName("exception")
-                              .hasAttributesSatisfyingExactly(
-                                  equalTo(
-                                      ExceptionAttributes.EXCEPTION_TYPE,
-                                      expectedException.getClass().getName()),
-                                  satisfies(
-                                      ExceptionAttributes.EXCEPTION_MESSAGE,
-                                      message ->
-                                          message.startsWithIgnoringCase(
-                                              expectedException.getMessage())))));
+      assertions.add(span -> span.hasStatus(StatusData.error()).hasException(expectedException));
     }
     return assertions;
   }

diff --git a/...src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/jakarta/ExceptionFilter.java b/...src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/jakarta/ExceptionFilter.java
@@ -11,18 +11,27 @@
 import jakarta.servlet.ServletException;
 import jakarta.servlet.ServletRequest;
 import jakarta.servlet.ServletResponse;
+import java.io.IOException;
 
 public class ExceptionFilter implements Filter {
   @Override
   public void init(FilterConfig filterConfig) {}
 
   @Override
   public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
-      throws ServletException {
+      throws ServletException, IOException {
     try {
       chain.doFilter(request, response);
-    } catch (Exception exception) {
-      throw new ServletException(exception);
+    } catch (ServletException exception) {
+      // to ease testing unwrap our exception to root cause
+      Throwable tmp = exception;
+      while (tmp.getCause() != null) {
+        tmp = tmp.getCause();
+      }
+      if (tmp.getMessage() != null && tmp.getMessage().contains("submit exception")) {
+        throw (IllegalStateException) tmp;
+      }
+      throw exception;
     }
   }
 

diff --git a/...ng/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/jakarta/GreetingForm.java b/...ng/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/jakarta/GreetingForm.java
@@ -22,10 +22,10 @@ public String getMessage() {
     return message;
   }
 
-  public void submit() throws Exception {
+  public void submit() {
     message = "Hello " + name;
     if (name.equals("exception")) {
-      throw new Exception("submit exception");
+      throw new IllegalStateException("submit exception");
     }
   }
 }