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

Fix tomcat 9.0.52 #3789

Merged
merged 1 commit into from
Aug 7, 2021
Merged

Fix tomcat 9.0.52 #3789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions ...entelemetry/javaagent/instrumentation/javaconcurrent/AbstractExecutorInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ protected AbstractExecutorInstrumentation() {
"java.util.concurrent.ForkJoinPool",
"java.util.concurrent.ScheduledThreadPoolExecutor",
"java.util.concurrent.ThreadPoolExecutor",
"org.apache.tomcat.util.threads.ThreadPoolExecutor",
"org.eclipse.jetty.util.thread.QueuedThreadPool", // dispatch() is covered in the jetty
// module
"org.eclipse.jetty.util.thread.ReservedThreadExecutor",
Expand Down
8 changes: 4 additions & 4 deletions instrumentation/tomcat/tomcat-7.0/javaagent/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ muzzle {
}

dependencies {
library("org.apache.tomcat.embed:tomcat-embed-core:7.0.4")
compileOnly("org.apache.tomcat.embed:tomcat-embed-core:7.0.4")
implementation(project(":instrumentation:tomcat:tomcat-common:javaagent"))
implementation(project(":instrumentation:servlet:servlet-3.0:javaagent"))
testInstrumentation(project(":instrumentation:servlet:servlet-javax-common:javaagent"))
// Make sure nothing breaks due to both 7.0 and 10.0 modules being present together
testInstrumentation(project(":instrumentation:tomcat:tomcat-10.0:javaagent"))

// Tests need at least version 9 to have necessary classes to configure the embedded tomcat...
// ... but not newer that version 10, because its servlet 5.
testLibrary("org.apache.tomcat.embed:tomcat-embed-core:[9.+, 10)")
testLibrary("org.apache.tomcat.embed:tomcat-embed-core:8.0.41")
testLibrary("org.apache.tomcat.embed:tomcat-embed-jasper:8.0.41")
Comment on lines +23 to +24
Copy link
Member

@trask trask Aug 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

is this part of the comment still relevant?

Suggested change
testLibrary("org.apache.tomcat.embed:tomcat-embed-core:8.0.41")
testLibrary("org.apache.tomcat.embed:tomcat-embed-jasper:8.0.41")
// Tests need at least version 8.0.41 to have necessary classes to configure the embedded tomcat...
// ... but not newer that version 10, because its servlet 5.
testLibrary("org.apache.tomcat.embed:tomcat-embed-core:8.0.41")
testLibrary("org.apache.tomcat.embed:tomcat-embed-jasper:8.0.41")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion this comment isn't relevant. It was previously using 9+ for both regular build and latest deps which is bad because it is really confusing when regular build starts failing without any code changes. 8.0.41 is the version used in servlet-3.0 tests. I didn't try to figure out whether this is really the earliest version that works, only checked that 8.0.1 didn't work. The part of version 10 is kind of obvious, servlet-3.0 uses the same construct and doesn't have a comment. Jasper dependency was added to get rid of a non fatal stack trace in test output.

latestDepTestLibrary("org.apache.tomcat.embed:tomcat-embed-core:[9.+, 10)")
latestDepTestLibrary("org.apache.tomcat.embed:tomcat-embed-jasper:[9.+, 10)")
}