Skip to content

Commit 8e4e88b

Browse files
PerfectSlayerPerfectSlayer
committed
feat(serverless): Keep iterating on design
1 parent 53d17bd commit 8e4e88b

File tree

3 files changed

+38
-146
lines changed

3 files changed

+38
-146
lines changed

‎dd-trace-core/src/main/java/datadog/trace/core/propagation/InferredProxyPropagator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
/** Inferred proxy propagator. Only extract, not meant for injection. */
1616
@ParametersAreNonnullByDefault
1717
public class InferredProxyPropagator implements Propagator {
18-
private static final String INFERRED_PROXY_KEY_PREFIX = "x-dd-proxy-";
18+
private static final String INFERRED_PROXY_KEY_PREFIX = "x-dd-proxy";
1919

2020
@Override
2121
public <C> void inject(Context context, C carrier, CarrierSetter<C> setter) {}

‎dd-trace-core/src/test/java/datadog/trace/core/propagation/InferredProxyPropagatorTests.java

Lines changed: 36 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import static datadog.trace.api.gateway.InferredProxySpan.fromContext;
55
import static org.junit.jupiter.api.Assertions.assertNotNull;
66
import static org.junit.jupiter.api.Assertions.assertNull;
7+
import static org.junit.jupiter.api.Assertions.assertTrue;
8+
import static org.junit.jupiter.params.provider.Arguments.of;
79

810
import datadog.context.Context;
911
import datadog.context.propagation.CarrierVisitor;
@@ -22,182 +24,72 @@
2224

2325
@DisplayName("InferredProxyPropagator Tests")
2426
class InferredProxyPropagatorTests {
25-
private static final String PROXY_SYSTEM_KEY = "x-dd-proxy-system";
27+
private static final String PROXY_SYSTEM_KEY = "x-dd-proxy";
2628
private static final String PROXY_REQUEST_TIME_MS_KEY = "x-dd-proxy-request-time-ms";
2729
private static final String PROXY_PATH_KEY = "x-dd-proxy-path";
2830
private static final String PROXY_HTTP_METHOD_KEY = "x-dd-proxy-httpmethod";
2931
private static final String PROXY_DOMAIN_NAME_KEY = "x-dd-proxy-domain-name";
3032
private static final MapVisitor MAP_VISITOR = new MapVisitor();
3133

32-
static Stream<Arguments> validHeadersProviderForPropagator() {
33-
Map<String, String> allStandard = new HashMap<>();
34-
allStandard.put(PROXY_SYSTEM_KEY, "aws-apigw"); // The only currently supported system
35-
allStandard.put(PROXY_REQUEST_TIME_MS_KEY, "12345");
36-
allStandard.put(PROXY_PATH_KEY, "/foo");
37-
allStandard.put(PROXY_HTTP_METHOD_KEY, "GET");
38-
allStandard.put(PROXY_DOMAIN_NAME_KEY, "api.example.com");
39-
40-
return Stream.of(
41-
Arguments.of(
42-
"all standard headers (aws-apigw)",
43-
allStandard,
44-
"aws-apigw",
45-
"12345",
46-
"/foo",
47-
"GET",
48-
"api.example.com",
49-
null,
50-
null));
51-
}
52-
53-
static Stream<Arguments> invalidOrMissingHeadersProviderForPropagator() { // Renamed
54-
Map<String, String> missingSystem = new HashMap<>();
55-
missingSystem.put(PROXY_REQUEST_TIME_MS_KEY, "12345");
56-
missingSystem.put(PROXY_PATH_KEY, "/foo");
57-
58-
Map<String, String> missingTime = new HashMap<>();
59-
missingTime.put(PROXY_SYSTEM_KEY, "aws-apigw");
60-
missingTime.put(PROXY_PATH_KEY, "/foo");
61-
62-
return Stream.of(
63-
Arguments.of("PROXY_SYSTEM_KEY missing", missingSystem),
64-
Arguments.of("PROXY_REQUEST_TIME_MS_KEY missing", missingTime));
65-
}
66-
6734
private InferredProxyPropagator propagator;
6835

6936
@BeforeEach
7037
void setUp() {
71-
propagator = new InferredProxyPropagator();
38+
this.propagator = new InferredProxyPropagator();
7239
}
7340

74-
@ParameterizedTest(name = "{0}")
75-
@MethodSource("validHeadersProviderForPropagator")
76-
@DisplayName("Should extract InferredProxyContext when valid headers are present")
77-
void testSuccessfulExtraction(
78-
String description,
79-
Map<String, String> headers,
80-
String expectedSystem,
81-
String expectedTimeMs,
82-
String expectedPath,
83-
String expectedMethod,
84-
String expectedDomain,
85-
String expectedExtraKey,
86-
String expectedExtraValue) {
87-
88-
// Now accesses the outer class's propagator instance field
89-
Context extractedOuterContext = this.propagator.extract(root(), headers, MAP_VISITOR);
90-
InferredProxySpan inferredProxySpan = fromContext(extractedOuterContext);
91-
92-
// assertNotNull(
93-
// inferredProxySpan, "InferredProxyContext should not be null for: " + description);
94-
// assertEquals(expectedSystem, inferredProxySpan.getValue(PROXY_SYSTEM_KEY));
95-
// assertEquals(expectedTimeMs, inferredProxySpan.getValue(PROXY_REQUEST_TIME_MS_KEY));
96-
// assertEquals(expectedPath, inferredProxySpan.getValue(PROXY_PATH_KEY));
97-
// assertEquals(expectedMethod, inferredProxySpan.getValue(PROXY_HTTP_METHOD_KEY));
98-
// assertEquals(expectedDomain, inferredProxySpan.getValue(PROXY_DOMAIN_NAME_KEY));
99-
// if (expectedExtraKey != null) {
100-
// assertEquals(expectedExtraValue, inferredProxySpan.getValue(expectedExtraKey));
101-
// }
41+
@Test
42+
@DisplayName("Should extract InferredProxySpan when valid headers are present")
43+
void testSuccessfulExtraction() {
44+
Map<String, String> headers = new HashMap<>();
45+
headers.put(PROXY_SYSTEM_KEY, "aws-apigateway");
46+
headers.put(PROXY_REQUEST_TIME_MS_KEY, "12345");
47+
headers.put(PROXY_PATH_KEY, "/foo");
48+
headers.put(PROXY_HTTP_METHOD_KEY, "GET");
49+
headers.put(PROXY_DOMAIN_NAME_KEY, "api.example.com");
50+
51+
Context context = this.propagator.extract(root(), headers, MAP_VISITOR);
52+
InferredProxySpan inferredProxySpan = fromContext(context);
53+
assertNotNull(inferredProxySpan);
54+
assertTrue(inferredProxySpan.isValid());
10255
}
10356

10457
@ParameterizedTest(name = "{0}")
10558
@MethodSource("invalidOrMissingHeadersProviderForPropagator")
106-
@DisplayName("Should create InferredProxyContext even if some critical headers are missing")
59+
@DisplayName("Should not create InferredProxySpan if some critical headers are missing")
10760
void testExtractionWithMissingCriticalHeaders(String description, Map<String, String> headers) {
10861
Context rootContext = root();
10962
Context extractedOuterContext = this.propagator.extract(rootContext, headers, MAP_VISITOR);
11063
InferredProxySpan inferredProxySpan = fromContext(extractedOuterContext);
111-
112-
assertNotNull(
113-
inferredProxySpan,
114-
"InferredProxyContext should still be created if any x-dd-proxy-* header is present for: "
115-
+ description);
116-
117-
// if (headers.containsKey(PROXY_SYSTEM_KEY)) {
118-
// assertEquals(headers.get(PROXY_SYSTEM_KEY),
119-
// inferredProxySpan.getValue(PROXY_SYSTEM_KEY));
120-
// } else {
121-
// assertNull(inferredProxySpan.getValue(PROXY_SYSTEM_KEY));
122-
// }
123-
// if (headers.containsKey(PROXY_REQUEST_TIME_MS_KEY)) {
124-
// assertEquals(
125-
// headers.get(PROXY_REQUEST_TIME_MS_KEY),
126-
// inferredProxySpan.getValue(PROXY_REQUEST_TIME_MS_KEY));
127-
// } else {
128-
// assertNull(inferredProxySpan.getValue(PROXY_REQUEST_TIME_MS_KEY));
129-
// }
130-
}
131-
132-
@Test
133-
@DisplayName("Should not extract InferredProxyContext if no relevant headers are present")
134-
void testNoRelevantHeaders() {
135-
Map<String, String> carrier = new HashMap<>();
136-
carrier.put("x-unrelated-header", "value");
137-
carrier.put("another-header", "othervalue");
138-
139-
Context extractedOuterContext = this.propagator.extract(root(), carrier, MAP_VISITOR);
140-
InferredProxySpan inferredProxySpan = fromContext(extractedOuterContext);
141-
142-
assertNull(
143-
inferredProxySpan,
144-
"InferredProxyContext should be null if no x-dd-proxy-* headers are found");
64+
assertNull(inferredProxySpan, "Invalid inferred proxy span should not be extracted");
14565
}
14666

147-
@Test
148-
@DisplayName("Extractor should handle multiple proxy headers")
149-
void testMultipleProxyHeaders() {
150-
Map<String, String> carrier = new HashMap<>();
151-
carrier.put(PROXY_SYSTEM_KEY, "aws-apigw");
152-
carrier.put(PROXY_REQUEST_TIME_MS_KEY, "12345");
153-
carrier.put("x-dd-proxy-custom", "value1"); // First proxy header
154-
carrier.put("x-dd-proxy-another", "value2"); // Second proxy header
155-
156-
Context extractedOuterContext = this.propagator.extract(root(), carrier, MAP_VISITOR);
157-
InferredProxySpan inferredProxySpan = fromContext(extractedOuterContext);
158-
159-
assertNotNull(inferredProxySpan);
160-
// Check if both headers were stored (covers extractedContext == null being false)
161-
// assertEquals("value1", inferredProxySpan.getValue("x-dd-proxy-custom"));
162-
// assertEquals("value2", inferredProxySpan.getValue("x-dd-proxy-another"));
163-
// assertEquals("aws-apigw", inferredProxySpan.getValue(PROXY_SYSTEM_KEY));
164-
}
165-
166-
@Test
167-
@DisplayName("Extractor accept method should handle null/empty keys")
168-
void testExtractorAcceptNullEmptyKeys() {
67+
static Stream<Arguments> invalidOrMissingHeadersProviderForPropagator() { // Renamed
68+
Map<String, String> missingSystem = new HashMap<>();
69+
missingSystem.put(PROXY_REQUEST_TIME_MS_KEY, "12345");
70+
missingSystem.put(PROXY_PATH_KEY, "/foo");
16971

170-
// Test null key - HashMap doesn't allow null keys. Standard HTTP visitors
171-
// also typically don't yield null keys. Testing this branch is difficult
172-
// without a custom visitor or modifying the source. Relying on coverage report
173-
// or assuming standard carriers won't provide null keys.
72+
Map<String, String> emptyValue = new HashMap<>();
73+
emptyValue.put(PROXY_SYSTEM_KEY, "");
17474

175-
// Test empty key
176-
Map<String, String> carrierWithEmptyKey = new HashMap<>();
177-
carrierWithEmptyKey.put("", "emptyKeyValue"); // Add empty key
178-
carrierWithEmptyKey.put(PROXY_SYSTEM_KEY, "aws-apigw"); // Add a valid key too
75+
Map<String, String> nullValue = new HashMap<>();
76+
nullValue.put(PROXY_SYSTEM_KEY, null);
17977

180-
Context contextAfterEmpty = this.propagator.extract(root(), carrierWithEmptyKey, MAP_VISITOR);
181-
InferredProxySpan inferredProxySpan = fromContext(contextAfterEmpty);
78+
Map<String, String> missingTime = new HashMap<>();
79+
missingTime.put(PROXY_SYSTEM_KEY, "aws-apigw");
80+
missingTime.put(PROXY_PATH_KEY, "/foo");
18281

183-
// The propagator should ignore the empty key entry entirely.
184-
// assertNotNull(inferredProxySpan, "Context should be created due to valid key");
185-
// assertNull(inferredProxySpan.getValue(""), "Empty key should not be stored");
186-
// assertEquals(
187-
// "aws-apigw",
188-
// inferredProxySpan.getValue(PROXY_SYSTEM_KEY),
189-
// "Valid key should still be stored");
190-
// assertEquals(1, inferredProxySpan.size(), "Only valid key should be stored");
82+
return Stream.of(
83+
of("PROXY_SYSTEM_KEY missing", missingSystem),
84+
of("PROXY_SYSTEM_KEY empty", emptyValue),
85+
of("PROXY_SYSTEM_KEY null", nullValue),
86+
of("PROXY_REQUEST_TIME_MS_KEY missing", missingTime));
19187
}
19288

193-
// Simple Map visitor for tests (can remain static or non-static in outer class)
19489
@ParametersAreNonnullByDefault
19590
private static class MapVisitor implements CarrierVisitor<Map<String, String>> {
19691
@Override
19792
public void forEachKeyValue(Map<String, String> carrier, BiConsumer<String, String> visitor) {
198-
if (carrier == null) {
199-
return;
200-
}
20193
carrier.forEach(visitor);
20294
}
20395
}

‎internal-api/src/test/java/datadog/trace/api/gateway/InferredProxySpanTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void testMapConstructor() {
3939
inferredProxySpan.finish();
4040
}
4141

42-
@ParameterizedTest
42+
@ParameterizedTest(name = "{0}")
4343
@DisplayName("Invalid headers should make invalid span")
4444
@MethodSource("invalidHeaders")
4545
void testInvalidHeaders(String useCase, Map<String, String> headers) {

0 commit comments

Comments
 (0)