-
Notifications
You must be signed in to change notification settings - Fork 319
Implement Rum injection for servlet 3 #9102
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
Changes from all commits
257f167
5e734e3
dde7164
bb22e1a
1bf99a1
8814d86
a1f0a77
f941b4a
fe2006c
2ac167a
c389b77
01f6aae
486ee57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package datadog.trace.bootstrap.instrumentation.buffer; | ||
|
|
||
| import static java.util.concurrent.TimeUnit.MICROSECONDS; | ||
| import static java.util.concurrent.TimeUnit.SECONDS; | ||
|
|
||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.PrintWriter; | ||
| import java.net.URL; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.List; | ||
| import org.apache.commons.io.IOUtils; | ||
| import org.openjdk.jmh.annotations.Benchmark; | ||
| import org.openjdk.jmh.annotations.BenchmarkMode; | ||
| import org.openjdk.jmh.annotations.Fork; | ||
| import org.openjdk.jmh.annotations.Measurement; | ||
| import org.openjdk.jmh.annotations.Mode; | ||
| import org.openjdk.jmh.annotations.OutputTimeUnit; | ||
| import org.openjdk.jmh.annotations.Scope; | ||
| import org.openjdk.jmh.annotations.State; | ||
| import org.openjdk.jmh.annotations.Warmup; | ||
|
|
||
| @State(Scope.Benchmark) | ||
| @Warmup(iterations = 1, time = 30, timeUnit = SECONDS) | ||
| @Measurement(iterations = 2, time = 30, timeUnit = SECONDS) | ||
| @BenchmarkMode(Mode.AverageTime) | ||
| @OutputTimeUnit(MICROSECONDS) | ||
| @Fork(value = 1) | ||
| public class InjectingPipeOutputStreamBenchmark { | ||
| private static final List<String> htmlContent; | ||
| private static final byte[] marker; | ||
| private static final byte[] content; | ||
|
|
||
| static { | ||
| try (InputStream is = new URL("https://www.google.com").openStream()) { | ||
| htmlContent = IOUtils.readLines(is, StandardCharsets.UTF_8); | ||
| } catch (IOException ioe) { | ||
| throw new RuntimeException(ioe); | ||
| } | ||
| marker = "</head>".getBytes(StandardCharsets.UTF_8); | ||
| content = "<script/>".getBytes(StandardCharsets.UTF_8); | ||
| } | ||
|
|
||
| @Benchmark | ||
| public void withPipe() throws Exception { | ||
| try (final PrintWriter out = | ||
| new PrintWriter( | ||
| new InjectingPipeOutputStream(new ByteArrayOutputStream(), marker, content, null))) { | ||
| htmlContent.forEach(out::println); | ||
| } | ||
| } | ||
|
|
||
| @Benchmark | ||
| public void withoutPipe() throws Exception { | ||
| try (final PrintWriter out = new PrintWriter(new ByteArrayOutputStream())) { | ||
| htmlContent.forEach(out::println); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | ||||||
| package datadog.trace.bootstrap.instrumentation.buffer; | ||||||
|
|
||||||
| import java.io.FilterOutputStream; | ||||||
| import java.io.IOException; | ||||||
| import java.io.OutputStream; | ||||||
| import java.util.function.Consumer; | ||||||
|
|
||||||
| /** | ||||||
| * A circular buffer that holds n+1 bytes and with a lookbehind buffer of n bytes. The first time | ||||||
| * that the latest n bytes matches the marker, a content is injected before. | ||||||
| */ | ||||||
| public class InjectingPipeOutputStream extends FilterOutputStream { | ||||||
| private final byte[] lookbehind; | ||||||
| private int pos; | ||||||
| private boolean bufferFilled; | ||||||
| private final byte[] marker; | ||||||
| private final byte[] contentToInject; | ||||||
| private boolean found = false; | ||||||
| private int matchingPos = 0; | ||||||
| private final Consumer<Void> onContentInjected; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! I will change it (need to change the test as well) |
||||||
|
|
||||||
| /** | ||||||
| * @param downstream the delegate output stream | ||||||
| * @param marker the marker to find in the stream | ||||||
| * @param contentToInject the content to inject once before the marker if found. | ||||||
| * @param onContentInjected callback called when and if the content is injected. | ||||||
| */ | ||||||
| public InjectingPipeOutputStream( | ||||||
| final OutputStream downstream, | ||||||
| final byte[] marker, | ||||||
| final byte[] contentToInject, | ||||||
| final Consumer<Void> onContentInjected) { | ||||||
| super(downstream); | ||||||
| this.marker = marker; | ||||||
| this.lookbehind = new byte[marker.length + 1]; | ||||||
| this.pos = 0; | ||||||
| this.contentToInject = contentToInject; | ||||||
| this.onContentInjected = onContentInjected; | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public void write(int b) throws IOException { | ||||||
| if (found) { | ||||||
| out.write(b); | ||||||
| return; | ||||||
| } | ||||||
| lookbehind[pos] = (byte) b; | ||||||
| pos = (pos + 1) % lookbehind.length; | ||||||
|
|
||||||
| if (marker[matchingPos++] == b) { | ||||||
| if (matchingPos == marker.length) { | ||||||
| found = true; | ||||||
| out.write(contentToInject); | ||||||
| if (onContentInjected != null) { | ||||||
| onContentInjected.accept(null); | ||||||
| } | ||||||
| drain((pos + 1) % lookbehind.length, marker.length); | ||||||
| return; | ||||||
| } | ||||||
| } else { | ||||||
| matchingPos = 0; | ||||||
| } | ||||||
|
|
||||||
| if (!bufferFilled) { | ||||||
| bufferFilled = pos == lookbehind.length - 1; | ||||||
| } | ||||||
|
|
||||||
| if (bufferFilled) { | ||||||
| super.write(lookbehind[(pos + 1) % lookbehind.length]); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private void drain(int from, int size) throws IOException { | ||||||
| while (size-- > 0) { | ||||||
| super.write(Character.valueOf((char) lookbehind[from])); | ||||||
| from = (from + 1) % lookbehind.length; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public void close() throws IOException { | ||||||
| if (!found) { | ||||||
| if (bufferFilled) { | ||||||
| drain((pos + 2) % lookbehind.length, marker.length - 1); | ||||||
| } else { | ||||||
| drain(0, pos); | ||||||
| } | ||||||
| } | ||||||
| super.close(); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package datadog.trace.bootstrap.instrumentation.buffer | ||
|
|
||
| import spock.lang.Specification | ||
|
|
||
| class InjectingPipeOutputStreamTest extends Specification { | ||
| def 'should filter a buffer and inject if found #found'() { | ||
| setup: | ||
| def downstream = new ByteArrayOutputStream() | ||
| def piped = new OutputStreamWriter(new InjectingPipeOutputStream(downstream, marker.getBytes("UTF-8"), contentToInject.getBytes("UTF-8"), null), | ||
| "UTF-8") | ||
| when: | ||
| try (def closeme = piped) { | ||
| piped.write(body) | ||
| } | ||
| then: | ||
| assert downstream.toByteArray() == expected.getBytes("UTF-8") | ||
| where: | ||
| body | marker | contentToInject | found | expected | ||
| "<html><head><foo/></head><body/></html>" | "</head>" | "<script>true</script>" | true | "<html><head><foo/><script>true</script></head><body/></html>" | ||
| "<html><body/></html>" | "</head>" | "<something/>" | false | "<html><body/></html>" | ||
| "<foo/>" | "<longerThanFoo>" | "<nothing>" | false | "<foo/>" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| package datadog.trace.instrumentation.servlet3; | ||
|
|
||
| import datadog.trace.api.rum.RumInjector; | ||
| import java.io.IOException; | ||
| import java.io.PrintWriter; | ||
| import javax.servlet.ServletOutputStream; | ||
| import javax.servlet.http.HttpServletResponse; | ||
| import javax.servlet.http.HttpServletResponseWrapper; | ||
|
|
||
| public class RumHttpServletResponseWrapper extends HttpServletResponseWrapper { | ||
| private ServletOutputStream outputStream; | ||
| private PrintWriter printWriter; | ||
| private boolean shouldInject; | ||
|
|
||
| public RumHttpServletResponseWrapper(HttpServletResponse response) { | ||
| super(response); | ||
| } | ||
|
|
||
| @Override | ||
| public ServletOutputStream getOutputStream() throws IOException { | ||
| if (!shouldInject) { | ||
| return super.getOutputStream(); | ||
| } | ||
| if (outputStream == null) { | ||
| String encoding = getCharacterEncoding(); | ||
| if (encoding == null) { | ||
| encoding = "UTF-8"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason to use UTF-8 as default?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right. perhaps using the platform default is better |
||
| } | ||
| outputStream = | ||
| new WrappedServletOutputStream( | ||
| super.getOutputStream(), | ||
| RumInjector.getMarker(encoding), | ||
| RumInjector.getSnippet(encoding), | ||
| this::onInjected); | ||
| } | ||
| return outputStream; | ||
| } | ||
|
|
||
| @Override | ||
| public PrintWriter getWriter() throws IOException { | ||
| if (!shouldInject) { | ||
| return super.getWriter(); | ||
| } | ||
| if (printWriter == null) { | ||
| printWriter = new PrintWriter(getOutputStream()); | ||
| } | ||
| return printWriter; | ||
| } | ||
|
|
||
| @Override | ||
| public void setContentLength(int len) { | ||
| // don't set it since we don't know if we will inject | ||
| } | ||
|
|
||
| @Override | ||
| public void reset() { | ||
| this.outputStream = null; | ||
| this.printWriter = null; | ||
| this.shouldInject = false; | ||
| super.reset(); | ||
| } | ||
|
|
||
| @Override | ||
| public void resetBuffer() { | ||
| this.outputStream = null; | ||
| this.printWriter = null; | ||
| this.shouldInject = false; | ||
| super.resetBuffer(); | ||
| } | ||
|
|
||
| public void onInjected(Void ignored) { | ||
| try { | ||
| setHeader("x-datadog-rum-injected", "1"); | ||
| } catch (Throwable ignored2) { | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void setContentType(String type) { | ||
| if (type != null && type.contains("html")) { | ||
| shouldInject = true; | ||
| } | ||
|
Comment on lines
+80
to
+82
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you like to re-introduce my
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if that simple logic is just enough? Otherwise we can reintroduce it. I've no strong opinions |
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.