Skip to content

Commit 0ace891

Browse files
pepeshorepepeshore
committed
refactor: improve JFinal instrumentation implementation
- Remove unused code in ActionHandlerInstrumentation and ActionMappingInstrumentation - Refactor JFinalSingletons to improve code organization - Update JFinalInstrumentationModule configuration - Enhance test coverage in JFinalTest - Update TestController implementation - Add JFinal support to documentation
1 parent f61ac70 commit 0ace891

File tree

8 files changed

+5100
-31
lines changed

8 files changed

+5100
-31
lines changed

‎docs/apidiffs/current_vs_latest/opentelemetry-javaagent.txt‎

Lines changed: 5055 additions & 0 deletions
Large diffs are not rendered by default.

‎docs/supported-libraries.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ These are the supported libraries and frameworks:
9696
| [JBoss Log Manager](https://github.com/jboss-logging/jboss-logmanager) | 1.1+ | N/A | none |
9797
| [JDBC](https://docs.oracle.com/javase/8/docs/api/java/sql/package-summary.html) | Java 8+ | [opentelemetry-jdbc](../instrumentation/jdbc/library) | [Database Client Spans], [Database Client Metrics] [6] |
9898
| [Jedis](https://github.com/xetorthio/jedis) | 1.4+ | N/A | [Database Client Spans], [Database Client Metrics] [6] |
99+
| [JFinal](https://github.com/jfinal/jfinal) | 3.6+ | N/A | Provides `http.route` [2], Controller Spans [3] |
99100
| [JMS](https://javaee.github.io/javaee-spec/javadocs/javax/jms/package-summary.html) | 1.1+ | N/A | [Messaging Spans] |
100101
| [Jodd Http](https://http.jodd.org/) | 4.2+ | N/A | [HTTP Client Spans], [HTTP Client Metrics] |
101102
| [JSP](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/jsp/package-summary.html) | 2.3.x only | N/A | Controller Spans [3] |

‎instrumentation/jfinal-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jfinal/v3_6/ActionHandlerInstrumentation.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package io.opentelemetry.javaagent.instrumentation.jfinal.v3_6;
77

88
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
9-
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
109
import static io.opentelemetry.javaagent.instrumentation.jfinal.v3_6.JFinalSingletons.instrumenter;
1110
import static net.bytebuddy.matcher.ElementMatchers.named;
1211
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
@@ -22,10 +21,6 @@
2221
import net.bytebuddy.matcher.ElementMatcher;
2322

2423
public class ActionHandlerInstrumentation implements TypeInstrumentation {
25-
@Override
26-
public ElementMatcher<ClassLoader> classLoaderOptimization() {
27-
return hasClassesNamed("com.jfinal.core.ActionHandler");
28-
}
2924

3025
@Override
3126
public ElementMatcher<TypeDescription> typeMatcher() {

‎instrumentation/jfinal-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jfinal/v3_6/ActionMappingInstrumentation.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
package io.opentelemetry.javaagent.instrumentation.jfinal.v3_6;
77

8-
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
98
import static net.bytebuddy.matcher.ElementMatchers.named;
109

1110
import com.jfinal.core.Action;
@@ -16,10 +15,6 @@
1615
import net.bytebuddy.matcher.ElementMatcher;
1716

1817
public class ActionMappingInstrumentation implements TypeInstrumentation {
19-
@Override
20-
public ElementMatcher<ClassLoader> classLoaderOptimization() {
21-
return hasClassesNamed("com.jfinal.core.ActionMapping");
22-
}
2318

2419
@Override
2520
public ElementMatcher<TypeDescription> typeMatcher() {

‎instrumentation/jfinal-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jfinal/v3_6/JFinalInstrumentationModule.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
66
package io.opentelemetry.javaagent.instrumentation.jfinal.v3_6;
77

88
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
9+
import static java.util.Arrays.asList;
910

1011
import com.google.auto.service.AutoService;
1112
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1213
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
13-
import java.util.Arrays;
1414
import java.util.List;
1515
import net.bytebuddy.matcher.ElementMatcher;
1616

@@ -27,6 +27,6 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
2727

2828
@Override
2929
public List<TypeInstrumentation> typeInstrumentations() {
30-
return Arrays.asList(new ActionMappingInstrumentation(), new ActionHandlerInstrumentation());
30+
return asList(new ActionMappingInstrumentation(), new ActionHandlerInstrumentation());
3131
}
3232
}

‎instrumentation/jfinal-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jfinal/v3_6/JFinalSingletons.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,17 @@
66
package io.opentelemetry.javaagent.instrumentation.jfinal.v3_6;
77

88
import com.jfinal.core.Action;
9+
import com.jfinal.core.Controller;
910
import com.jfinal.render.JsonRender;
1011
import io.opentelemetry.api.GlobalOpenTelemetry;
12+
import io.opentelemetry.api.trace.Span;
1113
import io.opentelemetry.context.Context;
1214
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1315
import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute;
1416
import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRouteSource;
1517
import io.opentelemetry.javaagent.bootstrap.internal.ExperimentalConfig;
18+
19+
import java.lang.reflect.Method;
1620
import java.util.logging.Level;
1721
import java.util.logging.Logger;
1822

@@ -28,7 +32,7 @@ public final class JFinalSingletons {
2832
.buildInstrumenter();
2933

3034
static {
31-
// see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/11465
35+
// see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/11465#issuecomment-2137294837
3236
excludeOtAttrs();
3337
}
3438

@@ -41,17 +45,20 @@ public static void updateSpan(Action action) {
4145
return;
4246
}
4347
String route = action.getActionKey();
44-
if (route == null) {
45-
return;
46-
}
4748
Context context = Context.current();
48-
HttpServerRoute.update(context, HttpServerRouteSource.CONTROLLER, route);
49+
if (route != null) {
50+
HttpServerRoute.update(context, HttpServerRouteSource.CONTROLLER, route);
51+
}
52+
Class<? extends Controller> clazz = action.getControllerClass();
53+
Method method = action.getMethod();
54+
if (clazz != null && method != null) {
55+
Span.fromContext(context).updateName(clazz.getSimpleName() + '.' + method.getName());
56+
}
4957
}
5058

5159
private static void excludeOtAttrs() {
5260
try {
5361
JsonRender.addExcludedAttrs(
54-
"io.opentelemetry.javaagent.instrumentation.servlet.ServletHelper.AsyncListenerResponse",
5562
"io.opentelemetry.javaagent.instrumentation.servlet.ServletHelper.Context",
5663
"trace_id",
5764
"span_id");

‎instrumentation/jfinal-3.6/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jfinal/JFinalTest.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@
66
package io.opentelemetry.javaagent.instrumentation.jfinal;
77

88
import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
9+
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.PATH_PARAM;
910
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.NOT_FOUND;
1011
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.REDIRECT;
12+
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR;
13+
14+
1115
import static org.assertj.core.api.Assertions.assertThat;
1216

1317
import com.jfinal.core.JFinalFilter;
@@ -50,7 +54,7 @@ protected void configure(HttpServerTestOptions options) {
5054
options.setHasHandlerSpan(unused -> true);
5155
options.setExpectedHttpRoute(
5256
(endpoint, method) -> {
53-
if (endpoint == ServerEndpoint.PATH_PARAM) {
57+
if (endpoint == PATH_PARAM) {
5458
return "/path/123/param";
5559
}
5660
if (HttpConstants._OTHER.equals(method)) {
@@ -101,7 +105,18 @@ protected SpanDataAssert assertResponseSpan(
101105
@Override
102106
public SpanDataAssert assertHandlerSpan(
103107
SpanDataAssert span, String method, ServerEndpoint endpoint) {
104-
span.hasName("jfinal.handle").hasKind(INTERNAL);
108+
span.hasName(getHandlerSpanName(endpoint)).hasKind(INTERNAL);
105109
return span;
106110
}
111+
112+
private static String getHandlerSpanName(ServerEndpoint endpoint) {
113+
if (PATH_PARAM.equals(endpoint)) {
114+
return "TestController.pathParam";
115+
} else if (NOT_FOUND.equals(endpoint)) {
116+
return "jfinal.handle";
117+
} else if (ERROR.equals(endpoint)) {
118+
return "TestController.error";
119+
}
120+
return "TestController." + endpoint.getPath().replace("/", "");
121+
}
107122
}

‎instrumentation/jfinal-3.6/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jfinal/TestController.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.javaagent.instrumentation.jfinal;
77

8+
import static io.opentelemetry.instrumentation.testing.GlobalTraceUtil.runWithSpan;
89
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.EXCEPTION;
910

1011
import com.jfinal.core.ActionKey;
@@ -17,22 +18,22 @@
1718
public class TestController extends Controller {
1819

1920
public void success() {
20-
GlobalTraceUtil.runWithSpan("controller", () -> renderText(ServerEndpoint.SUCCESS.getBody()));
21+
runWithSpan("controller", () -> renderText(ServerEndpoint.SUCCESS.getBody()));
2122
}
2223

2324
public void redirect() {
24-
GlobalTraceUtil.runWithSpan("controller", () -> redirect(ServerEndpoint.REDIRECT.getBody()));
25+
runWithSpan("controller", () -> redirect(ServerEndpoint.REDIRECT.getBody()));
2526
}
2627

2728
@ActionKey("error-status")
2829
public void error() throws Exception {
29-
GlobalTraceUtil.runWithSpan(
30+
runWithSpan(
3031
"controller", () -> renderError(500, new TextRender(ServerEndpoint.ERROR.getBody())));
3132
}
3233

3334
public void exception() throws Throwable {
3435
try {
35-
GlobalTraceUtil.runWithSpan(
36+
runWithSpan(
3637
"controller",
3738
() -> {
3839
throw new IllegalStateException(EXCEPTION.getBody());
@@ -44,7 +45,7 @@ public void exception() throws Throwable {
4445
}
4546

4647
public void captureHeaders() {
47-
GlobalTraceUtil.runWithSpan(
48+
runWithSpan(
4849
"controller",
4950
() -> {
5051
String header = getHeader("X-Test-Request");
@@ -54,7 +55,7 @@ public void captureHeaders() {
5455
}
5556

5657
public void captureParameters() {
57-
GlobalTraceUtil.runWithSpan(
58+
runWithSpan(
5859
"controller",
5960
() -> {
6061
renderText(ServerEndpoint.CAPTURE_PARAMETERS.getBody());
@@ -63,7 +64,7 @@ public void captureParameters() {
6364

6465
public void query() {
6566

66-
GlobalTraceUtil.runWithSpan(
67+
runWithSpan(
6768
"controller",
6869
() -> {
6970
renderText(ServerEndpoint.QUERY_PARAM.getBody());
@@ -72,23 +73,23 @@ public void query() {
7273

7374
@ActionKey("path/123/param")
7475
public void pathVar() {
75-
GlobalTraceUtil.runWithSpan(
76+
runWithSpan(
7677
"controller",
7778
() -> {
7879
renderText(ServerEndpoint.PATH_PARAM.getBody());
7980
});
8081
}
8182

8283
public void authRequired() {
83-
GlobalTraceUtil.runWithSpan(
84+
runWithSpan(
8485
"controller",
8586
() -> {
8687
renderText(ServerEndpoint.AUTH_REQUIRED.getBody());
8788
});
8889
}
8990

9091
public void login() {
91-
GlobalTraceUtil.runWithSpan(
92+
runWithSpan(
9293
"controller",
9394
() -> {
9495
redirect(ServerEndpoint.LOGIN.getBody());
@@ -101,7 +102,7 @@ public void basicsecured_endpoint() {
101102
}
102103

103104
public void child() {
104-
GlobalTraceUtil.runWithSpan(
105+
runWithSpan(
105106
"controller",
106107
() -> {
107108
ServerEndpoint.INDEXED_CHILD.collectSpanAttributes(

0 commit comments

Comments
 (0)