Skip to content

Commit 73fc942

Browse files
jaydelucajaydeluca
authored
Disable JSP compile spans by default (#15261)
1 parent 6cfcb75 commit 73fc942

File tree

8 files changed

+121
-37
lines changed

8 files changed

+121
-37
lines changed

‎docs/instrumentation-list.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6859,12 +6859,45 @@ libraries:
68596859
attributes: []
68606860
jsp:
68616861
- name: jsp-2.3
6862+
display_name: JSP (JavaServer Pages)
6863+
description: |
6864+
This instrumentation enables view spans for JSP page rendering and compilation (view spans are disabled by default).
6865+
library_link: https://jakarta.ee/specifications/pages/
6866+
features:
6867+
- VIEW_SPANS
68626868
source_path: instrumentation/jsp-2.3
68636869
scope:
68646870
name: io.opentelemetry.jsp-2.3
68656871
target_versions:
68666872
javaagent:
68676873
- org.apache.tomcat:tomcat-jasper:[7.0.19,10)
6874+
configurations:
6875+
- name: otel.instrumentation.common.experimental.view-telemetry.enabled
6876+
description: Enables the creation of experimental view spans.
6877+
type: boolean
6878+
default: false
6879+
- name: otel.instrumentation.jsp.experimental-span-attributes
6880+
description: |
6881+
Enables experimental span attributes `jsp.forwardOrigin`, `jsp.requestURL`, `jsp.compiler`, and `jsp.classFQCN`.
6882+
type: boolean
6883+
default: false
6884+
telemetry:
6885+
- when: otel.instrumentation.common.experimental.view-telemetry.enabled=true
6886+
spans:
6887+
- span_kind: INTERNAL
6888+
attributes: []
6889+
- when: otel.instrumentation.common.experimental.view-telemetry.enabled=true,otel.instrumentation.jsp.experimental-span-attributes=true
6890+
spans:
6891+
- span_kind: INTERNAL
6892+
attributes:
6893+
- name: jsp.classFQCN
6894+
type: STRING
6895+
- name: jsp.compiler
6896+
type: STRING
6897+
- name: jsp.forwardOrigin
6898+
type: STRING
6899+
- name: jsp.requestURL
6900+
type: STRING
68686901
kafka:
68696902
- name: kafka-clients-0.11
68706903
display_name: Apache Kafka Client

‎instrumentation-docs/instrumentations.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ readonly INSTRUMENTATIONS=(
146146
"jsf:jsf-mojarra-3.0:javaagent:test"
147147
"jsf:jsf-myfaces-1.2:javaagent:myfaces2Test"
148148
"jsf:jsf-myfaces-3.0:javaagent:test"
149+
"jsp-2.3:javaagent:test"
150+
"jsp-2.3:javaagent:testExperimental"
149151
"kafka:kafka-clients:kafka-clients-2.6:library:test"
150152
"kafka:kafka-connect-2.6:testing:test"
151153
"nats:nats-2.17:javaagent:test"

‎instrumentation/jsp-2.3/javaagent/build.gradle.kts‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,33 @@ dependencies {
4545
latestDepTestLibrary("org.apache.tomcat.embed:tomcat-embed-logging-juli:9.+") // documented limitation
4646
}
4747

48-
tasks.withType<Test>().configureEach {
49-
// skip jar scanning using environment variables:
50-
// http://tomcat.apache.org/tomcat-7.0-doc/config/systemprops.html#JAR_Scanning
51-
// having this set allows us to test with old versions of the tomcat api since
52-
// JarScanFilter did not exist in the tomcat 7 api
53-
jvmArgs("-Dorg.apache.catalina.startup.ContextConfig.jarsToSkip=*")
54-
jvmArgs("-Dorg.apache.catalina.startup.TldConfig.jarsToSkip=*")
48+
tasks {
49+
withType<Test>().configureEach {
50+
// skip jar scanning using environment variables:
51+
// http://tomcat.apache.org/tomcat-7.0-doc/config/systemprops.html#JAR_Scanning
52+
// having this set allows us to test with old versions of the tomcat api since
53+
// JarScanFilter did not exist in the tomcat 7 api
54+
jvmArgs("-Dorg.apache.catalina.startup.ContextConfig.jarsToSkip=*")
55+
jvmArgs("-Dorg.apache.catalina.startup.TldConfig.jarsToSkip=*")
5556

56-
// required on jdk17
57-
jvmArgs("--add-opens=java.rmi/sun.rmi.transport=ALL-UNNAMED")
58-
jvmArgs("--add-opens=java.base/java.util=ALL-UNNAMED")
59-
jvmArgs("-XX:+IgnoreUnrecognizedVMOptions")
57+
// required on jdk17
58+
jvmArgs("--add-opens=java.rmi/sun.rmi.transport=ALL-UNNAMED")
59+
jvmArgs("--add-opens=java.base/java.util=ALL-UNNAMED")
60+
jvmArgs("-XX:+IgnoreUnrecognizedVMOptions")
61+
jvmArgs("-Dotel.instrumentation.common.experimental.view-telemetry.enabled=true")
6062

61-
// TODO run tests both with and without experimental span attributes
62-
jvmArgs("-Dotel.instrumentation.jsp.experimental-span-attributes=true")
63+
systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
64+
}
65+
66+
test {
67+
systemProperty("metadataConfig", "otel.instrumentation.common.experimental.view-telemetry.enabled=true")
68+
}
6369

64-
jvmArgs("-Dotel.instrumentation.common.experimental.view-telemetry.enabled=true")
70+
val testExperimental by registering(Test::class) {
71+
testClassesDirs = sourceSets.test.get().output.classesDirs
72+
classpath = sourceSets.test.get().runtimeClasspath
73+
74+
jvmArgs("-Dotel.instrumentation.jsp.experimental-span-attributes=true")
75+
systemProperty("metadataConfig", "otel.instrumentation.common.experimental.view-telemetry.enabled=true,otel.instrumentation.jsp.experimental-span-attributes=true")
76+
}
6577
}

‎instrumentation/jsp-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsp/JspCompilationContextInstrumentationSingletons.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1313
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
1414
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
15+
import io.opentelemetry.javaagent.bootstrap.internal.ExperimentalConfig;
1516
import javax.annotation.Nullable;
1617
import org.apache.jasper.JspCompilationContext;
1718
import org.apache.jasper.compiler.Compiler;
@@ -30,6 +31,7 @@ public class JspCompilationContextInstrumentationSingletons {
3031
"io.opentelemetry.jsp-2.3",
3132
JspCompilationContextInstrumentationSingletons::spanNameOnCompile)
3233
.addAttributesExtractor(new CompilationAttributesExtractor())
34+
.setEnabled(ExperimentalConfig.get().viewTelemetryEnabled())
3335
.buildInstrumenter(SpanKindExtractor.alwaysInternal());
3436
}
3537

‎instrumentation/jsp-2.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jsp/JspInstrumentationBasicTests.java‎

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

88
import static io.opentelemetry.api.common.AttributeKey.stringKey;
9+
import static io.opentelemetry.javaagent.instrumentation.jsp.JspSpanAssertions.experimental;
910
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
1011
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
1112
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
@@ -44,8 +45,7 @@
4445
class JspInstrumentationBasicTests extends AbstractHttpServerUsingTest<Tomcat> {
4546

4647
@RegisterExtension
47-
public static final InstrumentationExtension testing =
48-
HttpServerInstrumentationExtension.forAgent();
48+
static final InstrumentationExtension testing = HttpServerInstrumentationExtension.forAgent();
4949

5050
private static JspSpanAssertions spanAsserts;
5151

@@ -428,10 +428,11 @@ void testCompileErrorShouldNotProduceRenderTracesAndSpans(
428428
.hasAttributesSatisfyingExactly(
429429
equalTo(
430430
stringKey("jsp.classFQCN"),
431-
"org.apache.jsp." + jspClassNamePrefix + jspClassName),
431+
experimental(
432+
"org.apache.jsp." + jspClassNamePrefix + jspClassName)),
432433
equalTo(
433434
stringKey("jsp.compiler"),
434-
"org.apache.jasper.compiler.JDTCompiler"))));
435+
experimental("org.apache.jasper.compiler.JDTCompiler")))));
435436
}
436437

437438
private static Stream<Arguments> compileErrorsArgs() {

‎instrumentation/jsp-2.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jsp/JspInstrumentationForwardTests.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@
3838
class JspInstrumentationForwardTests extends AbstractHttpServerUsingTest<Tomcat> {
3939

4040
@RegisterExtension
41-
public static final InstrumentationExtension testing =
42-
HttpServerInstrumentationExtension.forAgent();
41+
static final InstrumentationExtension testing = HttpServerInstrumentationExtension.forAgent();
4342

4443
private static JspSpanAssertions spanAsserts;
4544

‎instrumentation/jsp-2.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jsp/JspSpanAssertions.java‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,12 @@
2121
import io.opentelemetry.semconv.ServerAttributes;
2222
import io.opentelemetry.semconv.UrlAttributes;
2323
import io.opentelemetry.semconv.UserAgentAttributes;
24+
import javax.annotation.Nullable;
2425

2526
class JspSpanAssertions {
27+
static final boolean isExperimentalEnabled =
28+
Boolean.getBoolean("otel.instrumentation.jsp.experimental-span-attributes");
29+
2630
private final String baseUrl;
2731
private final int port;
2832

@@ -31,7 +35,15 @@ class JspSpanAssertions {
3135
this.port = port;
3236
}
3337

34-
SpanDataAssert assertServerSpan(SpanDataAssert span, JspSpan spanData) {
38+
@Nullable
39+
public static String experimental(String value) {
40+
if (isExperimentalEnabled) {
41+
return value;
42+
}
43+
return null;
44+
}
45+
46+
void assertServerSpan(SpanDataAssert span, JspSpan spanData) {
3547
if (spanData.getExceptionClass() != null) {
3648
span.hasStatus(StatusData.error())
3749
.hasEventsSatisfyingExactly(
@@ -58,7 +70,7 @@ SpanDataAssert assertServerSpan(SpanDataAssert span, JspSpan spanData) {
5870
val -> val.isInstanceOf(String.class))));
5971
}
6072

61-
return span.hasName(spanData.getMethod() + " " + spanData.getRoute())
73+
span.hasName(spanData.getMethod() + " " + spanData.getRoute())
6274
.hasNoParent()
6375
.hasKind(SpanKind.SERVER)
6476
.hasAttributesSatisfyingExactly(
@@ -83,7 +95,7 @@ SpanDataAssert assertServerSpan(SpanDataAssert span, JspSpan spanData) {
8395
v -> assertThat(v).isEqualTo("500"))));
8496
}
8597

86-
SpanDataAssert assertCompileSpan(SpanDataAssert span, JspSpan spanData) {
98+
void assertCompileSpan(SpanDataAssert span, JspSpan spanData) {
8799
if (spanData.getExceptionClass() != null) {
88100
span.hasStatus(StatusData.error())
89101
.hasEventsSatisfyingExactly(
@@ -102,14 +114,17 @@ SpanDataAssert assertCompileSpan(SpanDataAssert span, JspSpan spanData) {
102114
val -> val.isInstanceOf(String.class))));
103115
}
104116

105-
return span.hasName("Compile " + spanData.getRoute())
117+
span.hasName("Compile " + spanData.getRoute())
106118
.hasParent(spanData.getParent())
107119
.hasAttributesSatisfyingExactly(
108-
equalTo(stringKey("jsp.classFQCN"), "org.apache.jsp." + spanData.getClassName()),
109-
equalTo(stringKey("jsp.compiler"), "org.apache.jasper.compiler.JDTCompiler"));
120+
equalTo(
121+
stringKey("jsp.classFQCN"),
122+
experimental("org.apache.jsp." + spanData.getClassName())),
123+
equalTo(
124+
stringKey("jsp.compiler"), experimental("org.apache.jasper.compiler.JDTCompiler")));
110125
}
111126

112-
SpanDataAssert assertRenderSpan(SpanDataAssert span, JspSpan spanData) {
127+
void assertRenderSpan(SpanDataAssert span, JspSpan spanData) {
113128
String requestUrl = spanData.getRoute();
114129
if (spanData.getRequestUrlOverride() != null) {
115130
requestUrl = spanData.getRequestUrlOverride();
@@ -141,15 +156,17 @@ SpanDataAssert assertRenderSpan(SpanDataAssert span, JspSpan spanData) {
141156
val -> val.isInstanceOf(String.class))));
142157
}
143158

144-
return span.hasName("Render " + spanData.getRoute())
145-
.hasParent(spanData.getParent())
146-
.hasAttributesSatisfyingExactly(
147-
equalTo(stringKey("jsp.requestURL"), baseUrl + requestUrl),
148-
satisfies(
149-
stringKey("jsp.forwardOrigin"),
150-
val ->
151-
val.satisfiesAnyOf(
152-
v -> assertThat(spanData.getForwardOrigin()).isNull(),
153-
v -> assertThat(v).isEqualTo(spanData.getForwardOrigin()))));
159+
span.hasName("Render " + spanData.getRoute()).hasParent(spanData.getParent());
160+
161+
if (isExperimentalEnabled) {
162+
span.hasAttributesSatisfyingExactly(
163+
equalTo(stringKey("jsp.requestURL"), baseUrl + requestUrl),
164+
satisfies(
165+
stringKey("jsp.forwardOrigin"),
166+
val ->
167+
val.satisfiesAnyOf(
168+
v -> assertThat(spanData.getForwardOrigin()).isNull(),
169+
v -> assertThat(v).isEqualTo(spanData.getForwardOrigin()))));
170+
}
154171
}
155172
}

‎instrumentation/jsp-2.3/metadata.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
display_name: JSP (JavaServer Pages)
2+
description: >
3+
This instrumentation enables view spans for JSP page rendering and compilation (view spans are
4+
disabled by default).
5+
features:
6+
- VIEW_SPANS
7+
library_link: https://jakarta.ee/specifications/pages/
8+
configurations:
9+
- name: otel.instrumentation.common.experimental.view-telemetry.enabled
10+
description: Enables the creation of experimental view spans.
11+
type: boolean
12+
default: false
13+
- name: otel.instrumentation.jsp.experimental-span-attributes
14+
description: >
15+
Enables experimental span attributes `jsp.forwardOrigin`, `jsp.requestURL`, `jsp.compiler`,
16+
and `jsp.classFQCN`.
17+
type: boolean
18+
default: false

0 commit comments

Comments
 (0)