JavaScript Snippet Injection #7650

siyuniu-ms · 2023-01-25T00:25:04Z

Open Questions

1. Character encoding

what if one of the <head> tag characters is split across two bytes in some character encoding?
what if some character encoding translates a character to <h and we think it looks like the beginning of a head tag?

2. What if ServletOutputStream delegates to another ServletOutputStream?

Currently the instrumentation may inject the snippet twice.

Thoughts: Could use ThreadLocal to suppress duplicate injection?

Future Work

1. Support Servlet 5

2. Content Type set after calling getOutputStream or getWriter

Currently, injection is not performed in this case.

Cases: static html and JSP Possible solution: as soon as the content type is being set, create injection state and connect the state to servlet out put stream and print writer

3. Support head attributes, for example,

4. Support Netty

5. Handle spaces in tag ex:< head >

6. Add tests for multibyte character encodings, e.g. Chinese

7. Send back trace Id for the server request to the browser, so that you can associate the page to the original request

Doc

Snippet could only be set once, it could not be overwrite once it is set.

linux-foundation-easycla · 2023-01-25T00:25:13Z

The committers listed above are authorized under a signed CLA.

✅ login: siyuniums (add7e0c, 74a8e20, f9f6e1b, 02f6ae3, fd95e12, 8908ffe, 31c7804, 337e570, e6caf7f, 8c5e987, af95cbe, c29644b, 6f94478, 1822546, 0f03948, 473fa95, ac029ae, a1b42a8)
✅ login: trask / name: Trask Stalnaker (b667bc9, 94a699c, c5f3a84, 75156ed, 1637125, b59e83e, a77fd98)
✅ login: siyuniu-ms (f997d8e, 428c2ce, 3c6c68d, 01905a6, 927453b, 2e42e6f, 1ecd6e3, 6d06ac5, 7b4ffe1, 93b1689)

siyuniu-ms · 2023-01-25T00:28:22Z

/easycla

...test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionTest.java

instrumentation/servlet/servlet-3.0/javaagent-unit-tests/build.gradle.kts

...test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionTest.java

...etry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapperTest.java

...entelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3OutputStreamWriteBytesAdvice.java

...elemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java

instrumentation/servlet/servlet-3.0/javaagent/src/test/java/RequestDispatcherServlet.java

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy

...common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

trask

🥳

.../src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/TestUtil.java

...metry/javaagent/instrumentation/servlet/v3_0/snippet/OutputStreamSnippetInjectionHelper.java

...pentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingPrintWriter.java

...elemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java

...common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/ServerEndpoint.java

...nt/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java

...rc/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Singletons.java

...elemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java

...metry/javaagent/instrumentation/servlet/v3_0/snippet/OutputStreamSnippetInjectionHelper.java

.../src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/TestUtil.java

...ain/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionState.java

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy

...ap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ExperimentalSnippetHolder.java

linux-foundation-easycla · 2023-04-19T07:46:09Z

The committers listed above are authorized under a signed CLA.

✅ login: siyuniu-ms (7fdfcdb, 8f9813d, d2af9ca, 95a6714, 6a5329a, 85c43e1, 6d422bf, f597144, 9fc8899, 1919eb0, 26b55c5, 72563d2, bd024bc, 3bd615e, 79b2ed5, c1b5da5, f0715a1, 7554774, 2097d0b, e64c3c5, bf171da, 1008aef, 0aa85b0, 175c2f4, df43675, 4a35251, 6aaf18b, eff7aba, 5f33dca)
✅ login: trask / name: Trask Stalnaker (25c2af1, 95a6714, 6a5329a, 85c43e1, 6d422bf, 26b55c5, 3bd615e, 79b2ed5, c1b5da5, f0715a1, ca7eb61, b46cbff, db6dcaa, 0aa85b0)
✅ login: web-flow / name: GitHub Web Flow (95a6714, 6a5329a, 85c43e1, 6d422bf, db6dcaa, 0aa85b0)

linux-foundation-easycla · 2023-04-20T21:26:55Z

The committers listed above are authorized under a signed CLA.

✅ login: siyuniu-ms (7fdfcdb, 8f9813d, d2af9ca, 95a6714, 6a5329a, 85c43e1, 6d422bf, f597144, 9fc8899, 1919eb0, 26b55c5, 72563d2, bd024bc, 3bd615e, 79b2ed5, c1b5da5, f0715a1, 7554774, 2097d0b, e64c3c5, bf171da, 1008aef, 0aa85b0, 175c2f4, df43675, 4a35251, 6aaf18b, eff7aba, 5f33dca, 4d269b8, cdc0cd4, 390bf48)
✅ login: trask / name: Trask Stalnaker (25c2af1, 95a6714, 6a5329a, 85c43e1, 6d422bf, 26b55c5, 3bd615e, 79b2ed5, c1b5da5, f0715a1, ca7eb61, b46cbff, db6dcaa, 0aa85b0)

...ap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ExperimentalSnippetHolder.java

trask · 2023-04-26T23:09:28Z

thx @siyuniu-ms 💯

laurit · 2023-04-27T09:00:13Z

@siyuniu-ms this pr introduces flaky test https://ge.opentelemetry.io/scans/tests?search.buildOutcome=success&search.tags=CI&search.timeZoneId=Europe/Tallinn&tests.sortField=FLAKY&tests.unstableOnly=true please fix

trask mentioned this pull request Feb 14, 2023

JavaScript Snippet injection #6381

Closed

JavaScript snippet injection

7fdfcdb

siyuniu-ms force-pushed the pr/6381 branch from 93b1689 to 7fdfcdb Compare February 14, 2023 23:12

trask reviewed Feb 14, 2023

View reviewed changes

...test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionTest.java Outdated Show resolved Hide resolved

trask and others added 2 commits February 14, 2023 15:59

Avoid dependency on globals for unit tests

25c2af1

Update Servlet3OutputStreamWriteBytesAndOffsetAdvice.java

8f9813d

trask reviewed Feb 16, 2023

View reviewed changes

...test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionTest.java Outdated Show resolved Hide resolved

trask reviewed Feb 16, 2023

View reviewed changes

instrumentation/servlet/servlet-3.0/javaagent-unit-tests/build.gradle.kts Outdated Show resolved Hide resolved

trask reviewed Feb 16, 2023

View reviewed changes

...test/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/InjectionTest.java Outdated Show resolved Hide resolved

trask reviewed Feb 16, 2023

View reviewed changes

siyuniu-ms and others added 5 commits February 17, 2023 18:36

update comments

d2af9ca

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

95a6714

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

6a5329a

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

85c43e1

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

6d422bf

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

siyuniu-ms commented Feb 18, 2023

View reviewed changes

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy Outdated Show resolved Hide resolved

siyuniu-ms added 2 commits February 21, 2023 11:11

Update SnippetInjectingResponseWrapper.java

f597144

Update Servlet3Singletons.java

9fc8899

siyuniu-ms force-pushed the pr/6381 branch from 25e474f to 9fc8899 Compare February 22, 2023 00:17

siyuniu-ms and others added 9 commits February 21, 2023 17:48

JavaScript snippet injection

1919eb0

Avoid dependency on globals for unit tests

26b55c5

Update Servlet3OutputStreamWriteBytesAndOffsetAdvice.java

72563d2

update comments

bd024bc

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

3bd615e

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

79b2ed5

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

c1b5da5

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io…

f0715a1

…/opentelemetry/javaagent/instrumentation/servlet/v3_0/snippet/SnippetInjectingResponseWrapper.java Co-authored-by: Trask Stalnaker <[email protected]>

Update SnippetInjectingResponseWrapper.java

7554774

change naming and util test

175c2f4

trask approved these changes Feb 28, 2023

View reviewed changes

optimization on PrintWriter

df43675

mateuszrzeszutek reviewed Mar 29, 2023

View reviewed changes

change based on comments

4a35251

mateuszrzeszutek reviewed Apr 5, 2023

View reviewed changes

...ap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ExperimentalSnippetHolder.java Outdated Show resolved Hide resolved

siyuniu-ms mentioned this pull request Apr 10, 2023

Snippet Injection special case study for Jetty #8267

Closed

update for atomicity

6aaf18b

trask reviewed Apr 18, 2023

View reviewed changes

...ap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ExperimentalSnippetHolder.java Outdated Show resolved Hide resolved

Update ExperimentalSnippetHolder.java

eff7aba

siyuniu-ms added 2 commits April 19, 2023 12:49

initial value change to empty string

5f33dca

remove unnecessary print

4d269b8

siyuniu-ms mentioned this pull request Apr 20, 2023

change endpoint from enum to regular class #8325

Merged

Merge branch 'main' into pr/6381

cdc0cd4

Update ExperimentalSnippetHolder.java

390bf48

mateuszrzeszutek approved these changes Apr 21, 2023

View reviewed changes

...ap/src/main/java/io/opentelemetry/javaagent/bootstrap/servlet/ExperimentalSnippetHolder.java Show resolved Hide resolved

siyuniu-ms added 6 commits April 22, 2023 00:13

Update ExperimentalSnippetHolder.java

be5d10a

Merge remote-tracking branch 'upstream/main' into pr/6381

e775627

change based on new serverEndpoint

06dcd94

solve the import error

b47fbd1

Update ServerEndpoint.java

c184c86

Update AbstractServlet3Test.groovy

672e8a7

trask merged commit c7bd3e4 into open-telemetry:main Apr 26, 2023

siyuniu-ms mentioned this pull request May 25, 2023

JavaScript Snippet Injection Support Servlet 5 #8569

Merged

This was referenced Jun 6, 2023

Snippet Injection Smoke Test #8655

Merged

Snippet Injection rollback and update test #8669

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JavaScript Snippet Injection #7650

JavaScript Snippet Injection #7650

siyuniu-ms commented Jan 25, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jan 25, 2023 •

edited

Loading

siyuniu-ms commented Jan 25, 2023

trask left a comment

linux-foundation-easycla bot commented Apr 19, 2023 •

edited

Loading

linux-foundation-easycla bot commented Apr 20, 2023 •

edited

Loading

trask commented Apr 26, 2023

laurit commented Apr 27, 2023

JavaScript Snippet Injection #7650

JavaScript Snippet Injection #7650

Conversation

siyuniu-ms commented Jan 25, 2023 • edited Loading

Open Questions

1. Character encoding

2. What if ServletOutputStream delegates to another ServletOutputStream?

Future Work

1. Support Servlet 5

2. Content Type set after calling getOutputStream or getWriter

3. Support head attributes, for example,

4. Support Netty

5. Handle spaces in tag ex:< head >

6. Add tests for multibyte character encodings, e.g. Chinese

7. Send back trace Id for the server request to the browser, so that you can associate the page to the original request

Doc

linux-foundation-easycla bot commented Jan 25, 2023 • edited Loading

siyuniu-ms commented Jan 25, 2023

trask left a comment

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Apr 19, 2023 • edited Loading

linux-foundation-easycla bot commented Apr 20, 2023 • edited Loading

trask commented Apr 26, 2023

laurit commented Apr 27, 2023

siyuniu-ms commented Jan 25, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jan 25, 2023 •

edited

Loading

linux-foundation-easycla bot commented Apr 19, 2023 •

edited

Loading

linux-foundation-easycla bot commented Apr 20, 2023 •

edited

Loading