Skip to content

Commit bcc9c1d

Browse files
Build Span Attributes which may be missing from OTEL-provided Span
1 parent 0a08c9c commit bcc9c1d

File tree

17 files changed

+152
-30
lines changed

17 files changed

+152
-30
lines changed

filter-api/src/main/java/org/hypertrace/agent/filter/MultiFilter.java

+41
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,25 @@ public boolean evaluateRequestHeaders(Span span, Map<String, String> headers) {
5151
return shouldBlock;
5252
}
5353

54+
@Override
55+
public boolean evaluateRequestHeaders(
56+
Span span, Map<String, String> headers, Map<String, String> possiblyMissingSpanAttrs) {
57+
boolean shouldBlock = false;
58+
for (Filter filter : filters) {
59+
try {
60+
if (filter.evaluateRequestHeaders(span, headers, possiblyMissingSpanAttrs)) {
61+
shouldBlock = true;
62+
}
63+
} catch (Throwable t) {
64+
logger.warn(
65+
"Throwable thrown while evaluating Request headers for filter {}",
66+
filter.getClass().getName(),
67+
t);
68+
}
69+
}
70+
return shouldBlock;
71+
}
72+
5473
@Override
5574
public boolean evaluateRequestBody(Span span, String body, Map<String, String> headers) {
5675
boolean shouldBlock = false;
@@ -68,4 +87,26 @@ public boolean evaluateRequestBody(Span span, String body, Map<String, String> h
6887
}
6988
return shouldBlock;
7089
}
90+
91+
@Override
92+
public boolean evaluateRequestBody(
93+
Span span,
94+
String body,
95+
Map<String, String> headers,
96+
Map<String, String> possiblyMissingAttrs) {
97+
boolean shouldBlock = false;
98+
for (Filter filter : filters) {
99+
try {
100+
if (filter.evaluateRequestBody(span, body, headers, possiblyMissingAttrs)) {
101+
shouldBlock = true;
102+
}
103+
} catch (Throwable t) {
104+
logger.warn(
105+
"Throwable thrown while evaluating Request body for filter {}",
106+
filter.getClass().getName(),
107+
t);
108+
}
109+
}
110+
return shouldBlock;
111+
}
71112
}

filter-api/src/main/java/org/hypertrace/agent/filter/api/Filter.java

+21
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,19 @@ public interface Filter {
3434
*/
3535
boolean evaluateRequestHeaders(Span span, Map<String, String> headers);
3636

37+
/**
38+
* Evaluate the execution (includes extra attributes which might be missing from the span).
39+
*
40+
* @param span
41+
* @param headers
42+
* @param possiblyMissingSpanAttrs
43+
* @return
44+
*/
45+
default boolean evaluateRequestHeaders(
46+
Span span, Map<String, String> headers, Map<String, String> possiblyMissingSpanAttrs) {
47+
return evaluateRequestHeaders(span, headers);
48+
}
49+
3750
/**
3851
* Evaluate the execution.
3952
*
@@ -43,4 +56,12 @@ public interface Filter {
4356
* @return filter result
4457
*/
4558
boolean evaluateRequestBody(Span span, String body, Map<String, String> headers);
59+
60+
default boolean evaluateRequestBody(
61+
Span span,
62+
String body,
63+
Map<String, String> headers,
64+
Map<String, String> possiblyMissingSpanAttrs) {
65+
return evaluateRequestBody(span, body, headers);
66+
}
4667
}

instrumentation/apache-httpasyncclient-4.1/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ dependencies {
3535
testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:${versions["opentelemetry_semconv"]}")
3636
library("org.apache.httpcomponents:httpasyncclient:4.1")
3737
testImplementation(testFixtures(project(":testing-common")))
38+
implementation(project(":instrumentation:utils"))
3839
}
3940

instrumentation/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ dependencies{
1919
implementation(project(":instrumentation:undertow:undertow-1.4"))
2020
implementation(project(":instrumentation:undertow:undertow-servlet-1.4"))
2121
implementation(project(":instrumentation:vertx:vertx-web-3.0"))
22+
implementation(project(":instrumentation:utils"))
2223
implementation(project(":otel-extensions"))
2324
}
2425

instrumentation/netty/netty-4.0/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,6 @@ dependencies {
8888

8989
testImplementation(testFixtures(project(":testing-common")))
9090
testImplementation("org.asynchttpclient:async-http-client:2.0.9")
91+
implementation(project(":instrumentation:utils"))
9192
}
9293

instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_0/server/HttpServerBlockingRequestHandler.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.opentelemetry.api.trace.Span;
2929
import io.opentelemetry.context.Context;
3030
import io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_0.AttributeKeys;
31+
import io.opentelemetry.javaagent.instrumentation.hypertrace.utils.SpanUtils;
3132
import java.util.Map;
3233
import org.hypertrace.agent.filter.FilterRegistry;
3334

@@ -50,7 +51,11 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
5051
if (msg instanceof HttpRequest) {
5152
Attribute<Map<String, String>> headersAttr = channel.attr(AttributeKeys.REQUEST_HEADERS);
5253
Map<String, String> headers = headersAttr.getAndRemove();
53-
if (headers != null && FilterRegistry.getFilter().evaluateRequestHeaders(span, headers)) {
54+
Map<String, String> possiblyMissingAttrs =
55+
SpanUtils.getPossiblyMissingSpanAttributesFromURI(((HttpRequest) msg).getUri());
56+
if (headers != null
57+
&& FilterRegistry.getFilter()
58+
.evaluateRequestHeaders(span, headers, possiblyMissingAttrs)) {
5459
forbidden(ctx, (HttpRequest) msg);
5560
return;
5661
}

instrumentation/netty/netty-4.1/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,6 @@ dependencies {
5353
testImplementation(testFixtures(project(":testing-common")))
5454
testImplementation("io.netty:netty-handler:4.1.0.Final")
5555
testImplementation("org.asynchttpclient:async-http-client:2.1.0")
56+
implementation(project(":instrumentation:utils"))
5657
}
5758

instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/javaagent/instrumentation/hypertrace/netty/v4_1/server/HttpServerBlockingRequestHandler.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import io.opentelemetry.api.trace.Span;
2929
import io.opentelemetry.context.Context;
3030
import io.opentelemetry.javaagent.instrumentation.hypertrace.netty.v4_1.AttributeKeys;
31+
import io.opentelemetry.javaagent.instrumentation.hypertrace.utils.SpanUtils;
3132
import java.util.Map;
3233
import org.hypertrace.agent.filter.FilterRegistry;
3334

@@ -50,7 +51,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
5051
if (msg instanceof HttpRequest) {
5152
Attribute<Map<String, String>> headersAttr = channel.attr(AttributeKeys.REQUEST_HEADERS);
5253
Map<String, String> headers = headersAttr.getAndRemove();
53-
if (headers != null && FilterRegistry.getFilter().evaluateRequestHeaders(span, headers)) {
54+
Map<String, String> possiblyMissingAttrs =
55+
SpanUtils.getPossiblyMissingSpanAttributesFromURI(((HttpRequest) msg).uri());
56+
57+
if (headers != null
58+
&& FilterRegistry.getFilter()
59+
.evaluateRequestHeaders(span, headers, possiblyMissingAttrs)) {
5460
forbidden(ctx, (HttpRequest) msg);
5561
return;
5662
}

instrumentation/servlet/servlet-3.0/build.gradle.kts

+1
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,5 @@ dependencies {
4646
testImplementation("org.eclipse.jetty:jetty-server:8.1.22.v20160922")
4747
testImplementation("org.eclipse.jetty:jetty-servlet:8.1.22.v20160922")
4848
testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:${versions["opentelemetry_semconv"]}")
49+
implementation(project(":instrumentation:utils"))
4950
}

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,11 @@ public static boolean start(
115115
headers.put(attributeKey.getKey(), headerValue);
116116
}
117117

118-
if (FilterRegistry.getFilter().evaluateRequestHeaders(currentSpan, headers)) {
118+
Map<String, String> possiblyMissingAttrs =
119+
Utils.getPossiblyMissingSpanAttributes(httpRequest);
120+
121+
if (FilterRegistry.getFilter()
122+
.evaluateRequestHeaders(currentSpan, headers, possiblyMissingAttrs)) {
119123
httpResponse.setStatus(403);
120124
// skip execution of the user code
121125
return true;

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

+11
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import io.opentelemetry.api.trace.Span;
2020
import io.opentelemetry.instrumentation.api.field.VirtualField;
21+
import io.opentelemetry.javaagent.instrumentation.hypertrace.utils.SpanUtils;
2122
import java.io.BufferedReader;
2223
import java.io.PrintWriter;
2324
import java.io.UnsupportedEncodingException;
@@ -121,4 +122,14 @@ public static void resetRequestBodyBuffers(
121122
}
122123
}
123124
}
125+
126+
/**
127+
* Creates a Map of those attributes that may be missing from the Span.
128+
*
129+
* @param request
130+
* @return
131+
*/
132+
public static Map<String, String> getPossiblyMissingSpanAttributes(HttpServletRequest request) {
133+
return SpanUtils.getPossiblyMissingSpanAttributes(request);
134+
}
124135
}

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

+11-2
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,16 @@ public static ByteBufferSpanPair createRequestByteBufferSpanPair(
4343
if (contentLength < 0) {
4444
contentLength = ContentLengthUtils.DEFAULT;
4545
}
46+
Map<String, String> possiblyMissingAttrs =
47+
io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.Utils
48+
.getPossiblyMissingSpanAttributes(httpServletRequest);
49+
4650
return new ByteBufferSpanPair(
4751
span,
4852
BoundedBuffersFactory.createStream(contentLength, charset),
4953
filter::evaluateRequestBody,
50-
headers);
54+
headers,
55+
possiblyMissingAttrs);
5156
}
5257

5358
public static CharBufferSpanPair createRequestCharBufferSpanPair(
@@ -56,11 +61,15 @@ public static CharBufferSpanPair createRequestCharBufferSpanPair(
5661
if (contentLength < 0) {
5762
contentLength = ContentLengthUtils.DEFAULT;
5863
}
64+
Map<String, String> possiblyMissingAttrs =
65+
io.opentelemetry.javaagent.instrumentation.hypertrace.servlet.v3_0.nowrapping.Utils
66+
.getPossiblyMissingSpanAttributes(httpServletRequest);
5967
return new CharBufferSpanPair(
6068
span,
6169
BoundedBuffersFactory.createWriter(contentLength),
6270
filter::evaluateRequestBody,
63-
headers);
71+
headers,
72+
possiblyMissingAttrs);
6473
}
6574

6675
/**

instrumentation/servlet/servlet-3.0/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/v3_0/nowrapping/request/ServletInputStreamInstrumentationTest.java

+9-6
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import javax.servlet.ServletInputStream;
2626
import org.ServletStreamContextAccess;
2727
import org.TestServletInputStream;
28-
import org.hypertrace.agent.core.TriFunction;
28+
import org.hypertrace.agent.core.QuadFunction;
2929
import org.hypertrace.agent.core.instrumentation.buffer.BoundedBuffersFactory;
3030
import org.hypertrace.agent.core.instrumentation.buffer.BoundedByteArrayOutputStream;
3131
import org.hypertrace.agent.core.instrumentation.buffer.ByteBufferSpanPair;
@@ -48,7 +48,8 @@ public void read() throws IOException {
4848
BoundedByteArrayOutputStream buffer =
4949
BoundedBuffersFactory.createStream(StandardCharsets.UTF_8);
5050
ByteBufferSpanPair bufferSpanPair =
51-
new ByteBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
51+
new ByteBufferSpanPair(
52+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
5253
ServletStreamContextAccess.addToInputStreamContext(servletInputStream, bufferSpanPair);
5354

5455
while (servletInputStream.read() != -1) {}
@@ -66,7 +67,8 @@ public void read_callDepth_is_cleared() throws IOException {
6667
BoundedByteArrayOutputStream buffer =
6768
BoundedBuffersFactory.createStream(StandardCharsets.UTF_8);
6869
ByteBufferSpanPair bufferSpanPair =
69-
new ByteBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
70+
new ByteBufferSpanPair(
71+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
7072
ServletStreamContextAccess.addToInputStreamContext(servletInputStream, bufferSpanPair);
7173

7274
while (servletInputStream.read() != -1) {}
@@ -84,13 +86,14 @@ public void read_call_depth_read_calls_read() throws IOException {
8486
BoundedByteArrayOutputStream buffer =
8587
BoundedBuffersFactory.createStream(StandardCharsets.UTF_8);
8688
ByteBufferSpanPair bufferSpanPair =
87-
new ByteBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
89+
new ByteBufferSpanPair(
90+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
8891
ServletStreamContextAccess.addToInputStreamContext(servletInputStream, bufferSpanPair);
8992

9093
servletInputStream.read(new byte[BODY.length()]);
9194
Assertions.assertEquals(BODY.substring(2), buffer.toStringWithSuppliedCharset());
9295
}
9396

94-
private static final TriFunction<Span, String, Map<String, String>, Boolean> NOOP_FILTER =
95-
(span, body, headers) -> false;
97+
private static final QuadFunction<Span, String, Map<String, String>, Map<String, String>, Boolean>
98+
NOOP_FILTER = (span, body, headers, missingAttrs) -> false;
9699
}

instrumentation/servlet/servlet-rw/src/test/java/io/opentelemetry/javaagent/instrumentation/hypertrace/servlet/rw/reader/BufferedReaderInstrumentationTest.java

+15-9
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import java.util.Map;
2525
import org.BufferedReaderPrintWriterContextAccess;
2626
import org.TestBufferedReader;
27-
import org.hypertrace.agent.core.TriFunction;
27+
import org.hypertrace.agent.core.QuadFunction;
2828
import org.hypertrace.agent.core.instrumentation.buffer.*;
2929
import org.hypertrace.agent.testing.AbstractInstrumenterTest;
3030
import org.junit.jupiter.api.Assertions;
@@ -34,8 +34,8 @@ public class BufferedReaderInstrumentationTest extends AbstractInstrumenterTest
3434

3535
private static final String TEST_SPAN_NAME = "foo";
3636
private static final String BODY = "boobar";
37-
private static final TriFunction<Span, String, Map<String, String>, Boolean> NOOP_FILTER =
38-
(span, s, stringStringMap) -> false;
37+
private static final QuadFunction<Span, String, Map<String, String>, Map<String, String>, Boolean>
38+
NOOP_FILTER = (span, s, stringStringMap, missingAttrMap) -> false;
3939

4040
@Test
4141
public void read() throws IOException {
@@ -45,7 +45,8 @@ public void read() throws IOException {
4545

4646
BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter();
4747
CharBufferSpanPair bufferSpanPair =
48-
new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
48+
new CharBufferSpanPair(
49+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
4950

5051
BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext(
5152
bufferedReader, bufferSpanPair);
@@ -63,7 +64,8 @@ public void read_callDepth_isCleared() throws IOException {
6364

6465
BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter();
6566
CharBufferSpanPair bufferSpanPair =
66-
new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
67+
new CharBufferSpanPair(
68+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
6769

6870
BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext(
6971
bufferedReader, bufferSpanPair);
@@ -80,7 +82,8 @@ public void read_char_arr() throws IOException {
8082

8183
BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter();
8284
CharBufferSpanPair bufferSpanPair =
83-
new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
85+
new CharBufferSpanPair(
86+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
8487

8588
BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext(
8689
bufferedReader, bufferSpanPair);
@@ -98,7 +101,8 @@ public void read_callDepth_char_arr() throws IOException {
98101

99102
BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter();
100103
CharBufferSpanPair bufferSpanPair =
101-
new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
104+
new CharBufferSpanPair(
105+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
102106

103107
BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext(
104108
bufferedReader, bufferSpanPair);
@@ -115,7 +119,8 @@ public void read_char_arr_offset() throws IOException {
115119

116120
BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter();
117121
CharBufferSpanPair bufferSpanPair =
118-
new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
122+
new CharBufferSpanPair(
123+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
119124

120125
BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext(
121126
bufferedReader, bufferSpanPair);
@@ -134,7 +139,8 @@ public void readLine() throws IOException {
134139

135140
BoundedCharArrayWriter buffer = BoundedBuffersFactory.createWriter();
136141
CharBufferSpanPair bufferSpanPair =
137-
new CharBufferSpanPair(span, buffer, NOOP_FILTER, Collections.emptyMap());
142+
new CharBufferSpanPair(
143+
span, buffer, NOOP_FILTER, Collections.emptyMap(), Collections.emptyMap());
138144

139145
BufferedReaderPrintWriterContextAccess.addToBufferedReaderContext(
140146
bufferedReader, bufferSpanPair);

0 commit comments

Comments
 (0)