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.

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 context propagation in tomcat thread pool #4521

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

laurit
Copy link
Contributor

@laurit laurit commented Oct 27, 2021

Resolves #4497
Tomcat ThreadPoolExecutor extends java.util.concurrent.ThreadPoolExecutor and overrides the execute method. It also uses a custom task queue that reports queue full when pool size is smaller than max. This makes j.u.c.ThreadPoolExecutor create new threads. If maximum number of threads is reached then j.u.c.ThreadPoolExecutor will reject the task because it could not be queued (queue behaved as it is full) and thread could not be created. Tomcat ThreadPoolExecutor catches RejectedExecutionException and places task in queue. The problem is that we clear propagated context when j.u.c.ThreadPoolExecutor.execute fails with RejectedExecutionException because we interpret an exception in execute as a failure to queue task and assume that it could never execute.
This pr alters context propagation handling so that when task already has expected context we just skip it and return null, which disables the cleanup on exception logic.
Hopefully this pr also gets rid of debug logs Failed to propagate context because previous propagated context is where new and old context are the same and have different debug locations.

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

💯

@trask trask merged commit e31439e into open-telemetry:main Oct 27, 2021
RashmiRam pushed a commit to RashmiRam/opentelemetry-auto-instr-java that referenced this pull request May 23, 2022
@laurit laurit deleted the tomcat-thread-pool branch July 6, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sporadic CI failure: TomcatServlet*TestDispatchAsync
3 participants