Skip to content

Support late servlet content types for snippet injection - #20921

Open
trask with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-servlet-output-stream-injection
Open

trask with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-servlet-output-stream-injection

Conversation

Copilot AI commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Allow servlet streams and writers to be acquired before the response becomes HTML, provided the content type is set before writing the head tag.

Fixes #8869.

ServletOutputStream out = response.getOutputStream();
response.setContentType("text/html");
out.print("<head>");

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-10-24 05:32 UTC

Review the latest changes.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

…cquisition

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix servlet output stream injection for content type HTML Support late servlet content types for snippet injection Oct 23, 2026
Copilot AI requested a review from trask October 23, 2026 22:49
@trask
trask requested a balanced review from Copilot October 23, 2026 22:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Bulk writer calls now repeat content-type lookups for every character until a head tag is found, adding avoidable work to the response-writing path.

1 open issue
What changed in this PR

This PR lets servlet streams and writers be acquired before an HTML content type is set, extending snippet injection support across both servlet namespaces.

Changes:

  • Check injection eligibility at write time and discard partial matches across non-HTML writes.
  • Preserve once-only injection and track content length before the content type is known.
  • Add regression coverage for late content types, split writes, reacquisition, and committed responses.
File Description
instrumentation/​servlet/​servlet-common/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​common/​snippet/​SnippetInjectingPrintWriter.java Checks writer eligibility at write time.
instrumentation/​servlet/​servlet-common/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​common/​snippet/​OutputStreamSnippetInjectionHelper.java Checks stream eligibility at write time.
instrumentation/​servlet/​servlet-common/​bootstrap/​src/​main/​java/​io/​opentelemetry/​javaagent/​bootstrap/​servlet/​InjectionState.java Centralizes eligibility and partial-match reset.
instrumentation/​servlet/​servlet-5.0/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v5_0/​snippet/​ServletOutputStreamInjectionState.java Retains Jakarta stream state before HTML eligibility.
instrumentation/​servlet/​servlet-5.0/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v5_0/​snippet/​Servlet5SnippetInjectingResponseWrapper.java Supports early writer acquisition and content-length tracking.
instrumentation/​servlet/​servlet-5.0/​javaagent-unit-tests/​src/​test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v5_0/​snippet/​SnippetServletOutputStreamTest.java Supplies HTML content type in existing fixtures.
instrumentation/​servlet/​servlet-5.0/​javaagent-unit-tests/​src/​test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v5_0/​snippet/​LateContentTypeTest.java Adds Jakarta late-content-type regression tests.
instrumentation/​servlet/​servlet-3.0/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v3_0/​snippet/​ServletOutputStreamInjectionState.java Retains javax stream state before HTML eligibility.
instrumentation/​servlet/​servlet-3.0/​javaagent/​src/​main/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v3_0/​snippet/​Servlet3SnippetInjectingResponseWrapper.java Supports early writer acquisition and content-length tracking.
instrumentation/​servlet/​servlet-3.0/​javaagent-unit-tests/​src/​test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v3_0/​snippet/​SnippetServletOutputStreamTest.java Supplies HTML content type in existing fixtures.
instrumentation/​servlet/​servlet-3.0/​javaagent-unit-tests/​src/​test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v3_0/​snippet/​LateContentTypeTest.java Adds javax late-content-type regression tests.

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 23, 2026 23:10
@trask
trask requested a balanced review from Copilot October 23, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Late-content-type behavior needs installed-agent integration coverage for transformed writes and real-container Content-Length handling.

1 open issue
1 resolved since last review

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 23, 2026 23:46
@trask
trask requested a balanced review from Copilot October 23, 2026 23:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Only a minor test-organization convention issue remains, with no identified blocking correctness defects.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Low severity Move standalone servlet tests to the javaagent unit-test suite

instrumentation/​servlet/​servlet-3.0/​javaagent-unit-tests/​src/​test/​java/​io/​opentelemetry/​javaagent/​instrumentation/​servlet/​v3_0/​snippet/​LateContentTypeTest.java:32

[Testing] This new class calls the wrapper and injection helper directly without requiring an installed agent, so its location does not follow .github/instructions/java-tests.instructions.md:22–25. Move it to the servlet-3.0 javaagent module's src/unitTests/java and register a unitTests JvmTestSuite; apply the same change to the new servlet-5.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/snippet/LateContentTypeTest.java:33. Existing standalone tests need not be migrated in this PR.

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 00:23
@trask
trask requested a balanced review from Copilot October 24, 2026 00:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The write-time eligibility changes are consistent across both servlet namespaces, with focused regression coverage and no unresolved blocking findings.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 04:06
@trask
trask requested a balanced review from Copilot October 24, 2026 04:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved blocking issues remain, and regression coverage addresses late content types and Content-Length adjustment in both servlet namespaces.

No open issues 🎉

Review effort: Balanced

Co-authored-by: trask <218610+trask@users.noreply.github.com>
Copilot AI requested a review from trask October 24, 2026 04:24
@trask
trask requested a balanced review from Copilot October 24, 2026 04:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved defects were identified, and unit and installed-agent regression coverage addresses both servlet namespaces and output paths.

No open issues 🎉

Review effort: Balanced

@trask
trask marked this pull request as ready for review October 24, 2026 04:34
@trask
trask requested a review from a team as a code owner October 24, 2026 04:34

This branch has not been deployed

No deployments
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.

Servlet snippet inject not work after call getOutputStream

3 participants