Skip to content

Commit 3b74567

Browse files
bradAtTraceablebradAtTraceable
authored
ENG-17140 Ensure requests can be filtered because of method body (#364)
* ENG-17140 Ensure requests can be filtered because of method body * Fix smoke tests
1 parent d0c2e8c commit 3b74567

File tree

5 files changed

+119
-66
lines changed

5 files changed

+119
-66
lines changed

‎instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/Servlet30AndFilterInstrumentation.java

Lines changed: 53 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -154,61 +154,63 @@ public static void exit(
154154
HttpServletRequest httpRequest = (HttpServletRequest) request;
155155
InstrumentationConfig instrumentationConfig = InstrumentationConfig.ConfigProvider.get();
156156

157-
if (throwable instanceof HypertraceEvaluationException) {
158-
httpResponse.setStatus(403);
159-
// bytebuddy treats the reassignment of this variable to null as an instruction to suppress
160-
// this exception, which is what we want
161-
throwable = null;
162-
return;
163-
}
157+
try {
158+
// response context to capture body and clear the context
159+
VirtualField<HttpServletResponse, SpanAndObjectPair> responseContextStore =
160+
VirtualField.find(HttpServletResponse.class, SpanAndObjectPair.class);
161+
VirtualField<ServletOutputStream, BoundedByteArrayOutputStream> outputStreamContextStore =
162+
VirtualField.find(ServletOutputStream.class, BoundedByteArrayOutputStream.class);
163+
VirtualField<PrintWriter, BoundedCharArrayWriter> writerContextStore =
164+
VirtualField.find(PrintWriter.class, BoundedCharArrayWriter.class);
165+
166+
// request context to clear body buffer
167+
VirtualField<HttpServletRequest, SpanAndObjectPair> requestContextStore =
168+
VirtualField.find(HttpServletRequest.class, SpanAndObjectPair.class);
169+
VirtualField<ServletInputStream, ByteBufferSpanPair> inputStreamContextStore =
170+
VirtualField.find(ServletInputStream.class, ByteBufferSpanPair.class);
171+
VirtualField<BufferedReader, CharBufferSpanPair> readerContextStore =
172+
VirtualField.find(BufferedReader.class, CharBufferSpanPair.class);
173+
VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore =
174+
VirtualField.find(HttpServletRequest.class, StringMapSpanPair.class);
175+
176+
if (!request.isAsyncStarted()) {
177+
if (instrumentationConfig.httpHeaders().response()) {
178+
for (String headerName : httpResponse.getHeaderNames()) {
179+
String headerValue = httpResponse.getHeader(headerName);
180+
currentSpan.setAttribute(
181+
HypertraceSemanticAttributes.httpResponseHeader(headerName), headerValue);
182+
}
183+
}
164184

165-
// response context to capture body and clear the context
166-
VirtualField<HttpServletResponse, SpanAndObjectPair> responseContextStore =
167-
VirtualField.find(HttpServletResponse.class, SpanAndObjectPair.class);
168-
VirtualField<ServletOutputStream, BoundedByteArrayOutputStream> outputStreamContextStore =
169-
VirtualField.find(ServletOutputStream.class, BoundedByteArrayOutputStream.class);
170-
VirtualField<PrintWriter, BoundedCharArrayWriter> writerContextStore =
171-
VirtualField.find(PrintWriter.class, BoundedCharArrayWriter.class);
172-
173-
// request context to clear body buffer
174-
VirtualField<HttpServletRequest, SpanAndObjectPair> requestContextStore =
175-
VirtualField.find(HttpServletRequest.class, SpanAndObjectPair.class);
176-
VirtualField<ServletInputStream, ByteBufferSpanPair> inputStreamContextStore =
177-
VirtualField.find(ServletInputStream.class, ByteBufferSpanPair.class);
178-
VirtualField<BufferedReader, CharBufferSpanPair> readerContextStore =
179-
VirtualField.find(BufferedReader.class, CharBufferSpanPair.class);
180-
VirtualField<HttpServletRequest, StringMapSpanPair> urlEncodedMapContextStore =
181-
VirtualField.find(HttpServletRequest.class, StringMapSpanPair.class);
182-
183-
if (!request.isAsyncStarted()) {
184-
if (instrumentationConfig.httpHeaders().response()) {
185-
for (String headerName : httpResponse.getHeaderNames()) {
186-
String headerValue = httpResponse.getHeader(headerName);
187-
currentSpan.setAttribute(
188-
HypertraceSemanticAttributes.httpResponseHeader(headerName), headerValue);
185+
// capture response body
186+
if (instrumentationConfig.httpBody().response()
187+
&& ContentTypeUtils.shouldCapture(httpResponse.getContentType())) {
188+
Utils.captureResponseBody(
189+
currentSpan,
190+
httpResponse,
191+
responseContextStore,
192+
outputStreamContextStore,
193+
writerContextStore);
189194
}
190-
}
191195

192-
// capture response body
193-
if (instrumentationConfig.httpBody().response()
194-
&& ContentTypeUtils.shouldCapture(httpResponse.getContentType())) {
195-
Utils.captureResponseBody(
196-
currentSpan,
197-
httpResponse,
198-
responseContextStore,
199-
outputStreamContextStore,
200-
writerContextStore);
196+
// remove request body buffers from context stores, otherwise they might get reused
197+
if (instrumentationConfig.httpBody().request()
198+
&& ContentTypeUtils.shouldCapture(httpRequest.getContentType())) {
199+
Utils.resetRequestBodyBuffers(
200+
httpRequest,
201+
requestContextStore,
202+
inputStreamContextStore,
203+
readerContextStore,
204+
urlEncodedMapContextStore);
205+
}
201206
}
202-
203-
// remove request body buffers from context stores, otherwise they might get reused
204-
if (instrumentationConfig.httpBody().request()
205-
&& ContentTypeUtils.shouldCapture(httpRequest.getContentType())) {
206-
Utils.resetRequestBodyBuffers(
207-
httpRequest,
208-
requestContextStore,
209-
inputStreamContextStore,
210-
readerContextStore,
211-
urlEncodedMapContextStore);
207+
} finally {
208+
if (throwable instanceof HypertraceEvaluationException) {
209+
httpResponse.setStatus(403);
210+
// bytebuddy treats the reassignment of this variable to null as an instruction to
211+
// suppress
212+
// this exception, which is what we want
213+
throwable = null;
212214
}
213215
}
214216
}

‎instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentation.java

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public static void exit(
115115
if (read == -1) {
116116
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
117117
} else {
118-
bufferSpanPair.buffer.write((byte) read);
118+
bufferSpanPair.writeToBuffer((byte) read);
119119
}
120120
} catch (Throwable t) {
121121
if (t instanceof HypertraceEvaluationException) {
@@ -143,9 +143,11 @@ public static ByteBufferSpanPair enter(@Advice.This ServletInputStream thizz) {
143143

144144
@Advice.OnMethodExit(onThrowable = Throwable.class)
145145
public static void exit(
146+
@Advice.This ServletInputStream thizz,
146147
@Advice.Return int read,
147148
@Advice.Argument(0) byte b[],
148-
@Advice.Enter ByteBufferSpanPair bufferSpanPair) {
149+
@Advice.Enter ByteBufferSpanPair bufferSpanPair)
150+
throws Throwable {
149151
try {
150152
if (bufferSpanPair == null) {
151153
return;
@@ -159,7 +161,10 @@ public static void exit(
159161
if (read == -1) {
160162
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
161163
} else {
162-
bufferSpanPair.buffer.write(b, 0, read);
164+
bufferSpanPair.writeToBuffer(b, 0, read);
165+
if (thizz.available() == 0) {
166+
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
167+
}
163168
}
164169
} catch (Throwable t) {
165170
if (t instanceof HypertraceEvaluationException) {
@@ -187,11 +192,13 @@ public static ByteBufferSpanPair enter(@Advice.This ServletInputStream thizz) {
187192

188193
@Advice.OnMethodExit(onThrowable = Throwable.class)
189194
public static void exit(
195+
@Advice.This ServletInputStream thizz,
190196
@Advice.Return int read,
191197
@Advice.Argument(0) byte b[],
192198
@Advice.Argument(1) int off,
193199
@Advice.Argument(2) int len,
194-
@Advice.Enter ByteBufferSpanPair bufferSpanPair) {
200+
@Advice.Enter ByteBufferSpanPair bufferSpanPair)
201+
throws Throwable {
195202
try {
196203

197204
if (bufferSpanPair == null) {
@@ -206,7 +213,10 @@ public static void exit(
206213
if (read == -1) {
207214
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
208215
} else {
209-
bufferSpanPair.buffer.write(b, off, read);
216+
bufferSpanPair.writeToBuffer(b, off, read);
217+
if (thizz.available() == 0) {
218+
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
219+
}
210220
}
211221
} catch (Throwable t) {
212222
if (t instanceof HypertraceEvaluationException) {
@@ -245,8 +255,7 @@ public static void exit(
245255
if (callDepth > 0) {
246256
return;
247257
}
248-
249-
bufferSpanPair.buffer.write(b);
258+
bufferSpanPair.writeToBuffer(b);
250259
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
251260
} catch (Throwable t) {
252261
if (t instanceof HypertraceEvaluationException) {
@@ -274,11 +283,13 @@ public static ByteBufferSpanPair enter(@Advice.This ServletInputStream thizz) {
274283

275284
@Advice.OnMethodExit(onThrowable = Throwable.class)
276285
public static void exit(
286+
@Advice.This ServletInputStream thizz,
277287
@Advice.Return int read,
278288
@Advice.Argument(0) byte[] b,
279289
@Advice.Argument(1) int off,
280290
@Advice.Argument(2) int len,
281-
@Advice.Enter ByteBufferSpanPair bufferSpanPair) {
291+
@Advice.Enter ByteBufferSpanPair bufferSpanPair)
292+
throws Throwable {
282293
try {
283294
if (bufferSpanPair == null) {
284295
return;
@@ -292,7 +303,10 @@ public static void exit(
292303
if (read == -1) {
293304
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
294305
} else {
295-
bufferSpanPair.buffer.write(b, off, read);
306+
bufferSpanPair.writeToBuffer(b, off, read);
307+
if (thizz.available() == 0) {
308+
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
309+
}
296310
}
297311
} catch (Throwable t) {
298312
if (t instanceof HypertraceEvaluationException) {

‎instrumentation/servlet/servlet-rw/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/rw/reader/BufferedReaderInstrumentation.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public static void exit(
9595
if (read == -1) {
9696
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
9797
} else {
98-
bufferSpanPair.buffer.write(read);
98+
bufferSpanPair.writeToBuffer((byte) read);
9999
}
100100
}
101101
}
@@ -129,7 +129,7 @@ public static void exit(
129129
if (read == -1) {
130130
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
131131
} else {
132-
bufferSpanPair.buffer.write(c, 0, read);
132+
bufferSpanPair.writeToBuffer(c, 0, read);
133133
}
134134
}
135135
}
@@ -165,7 +165,7 @@ public static void exit(
165165
if (read == -1) {
166166
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
167167
} else {
168-
bufferSpanPair.buffer.write(c, off, read);
168+
bufferSpanPair.writeToBuffer(c, off, read);
169169
}
170170
}
171171
}
@@ -198,7 +198,7 @@ public static void exit(
198198
if (line == null) {
199199
bufferSpanPair.captureBody(HypertraceSemanticAttributes.HTTP_REQUEST_BODY);
200200
} else {
201-
bufferSpanPair.buffer.write(line);
201+
bufferSpanPair.writeLine(line);
202202
}
203203
}
204204
}

‎javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/ByteBufferSpanPair.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import io.opentelemetry.api.common.AttributeKey;
2020
import io.opentelemetry.api.trace.Span;
21+
import java.io.IOException;
2122
import java.io.UnsupportedEncodingException;
2223
import java.util.Map;
2324
import java.util.Objects;
@@ -27,7 +28,7 @@
2728
public class ByteBufferSpanPair {
2829

2930
public final Span span;
30-
public final BoundedByteArrayOutputStream buffer;
31+
private final BoundedByteArrayOutputStream buffer;
3132
private final Map<String, String> headers;
3233
private boolean bufferCaptured;
3334
private final TriFunction<Span, String, Map<String, String>, Boolean> filter;
@@ -62,4 +63,19 @@ public void captureBody(AttributeKey<String> attributeKey) {
6263
throw new HypertraceEvaluationException();
6364
}
6465
}
66+
67+
public void writeToBuffer(byte singleByte) {
68+
bufferCaptured = false;
69+
buffer.write(singleByte);
70+
}
71+
72+
public void writeToBuffer(byte[] b, int offset, int len) {
73+
bufferCaptured = false;
74+
buffer.write(b, offset, len);
75+
}
76+
77+
public void writeToBuffer(byte[] b) throws IOException {
78+
bufferCaptured = false;
79+
buffer.write(b);
80+
}
6581
}

‎javaagent-core/src/main/java/org/hypertrace/agent/core/instrumentation/buffer/CharBufferSpanPair.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import io.opentelemetry.api.common.AttributeKey;
2020
import io.opentelemetry.api.trace.Span;
21+
import java.io.IOException;
2122
import java.util.Map;
2223
import org.hypertrace.agent.core.TriFunction;
2324
import org.hypertrace.agent.core.instrumentation.HypertraceEvaluationException;
@@ -26,7 +27,7 @@ public class CharBufferSpanPair {
2627

2728
public final Span span;
2829
public final Map<String, String> headers;
29-
public final BoundedCharArrayWriter buffer;
30+
private final BoundedCharArrayWriter buffer;
3031
private final TriFunction<Span, String, Map<String, String>, Boolean> filter;
3132

3233
/**
@@ -59,4 +60,24 @@ public void captureBody(AttributeKey<String> attributeKey) {
5960
throw new HypertraceEvaluationException();
6061
}
6162
}
63+
64+
public void writeToBuffer(byte singleByte) {
65+
bufferCaptured = false;
66+
buffer.write(singleByte);
67+
}
68+
69+
public void writeToBuffer(char[] c, int offset, int len) {
70+
bufferCaptured = false;
71+
buffer.write(c, offset, len);
72+
}
73+
74+
public void writeToBuffer(char[] c) throws IOException {
75+
bufferCaptured = false;
76+
buffer.write(c);
77+
}
78+
79+
public void writeLine(String line) throws IOException {
80+
bufferCaptured = false;
81+
buffer.write(line);
82+
}
6283
}

0 commit comments

Comments
 (0)