Skip to content

Commit a0321b6

Browse files
ojproductionsojproductions
authored
fix aws request/response payload tagging (#9887)
* check for empty env when setting cloud payload tagging * parse cloud payload tags during config * add branch coverage for pathcursor * apply pr suggestions
1 parent 5f0a539 commit a0321b6

File tree

11 files changed

+231
-70
lines changed

11 files changed

+231
-70
lines changed

‎dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/PayloadTagsProcessor.java‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
package datadog.trace.core.tagprocessor;
22

3+
import static datadog.trace.util.json.JsonPathParser.parseJsonPaths;
4+
35
import datadog.trace.api.Config;
46
import datadog.trace.api.ConfigDefaults;
57
import datadog.trace.api.TagMap;
68
import datadog.trace.api.telemetry.LogCollector;
79
import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
810
import datadog.trace.core.DDSpanContext;
11+
import datadog.trace.core.util.JsonStreamParser;
912
import datadog.trace.payloadtags.PayloadTagsData;
10-
import datadog.trace.payloadtags.json.JsonPath;
11-
import datadog.trace.payloadtags.json.JsonPathParser;
12-
import datadog.trace.payloadtags.json.JsonStreamParser;
13-
import datadog.trace.payloadtags.json.PathCursor;
13+
import datadog.trace.util.json.JsonPath;
14+
import datadog.trace.util.json.PathCursor;
1415
import java.io.InputStream;
1516
import java.util.ArrayList;
16-
import java.util.Collections;
1717
import java.util.HashMap;
1818
import java.util.List;
1919
import java.util.Map;
@@ -38,7 +38,7 @@ public static PayloadTagsProcessor create(Config config) {
3838
new RedactionRules.Builder()
3939
.addRedactionJsonPaths(ConfigDefaults.DEFAULT_CLOUD_COMMON_PAYLOAD_TAGGING)
4040
.addRedactionJsonPaths(ConfigDefaults.DEFAULT_CLOUD_REQUEST_PAYLOAD_TAGGING)
41-
.addRedactionJsonPaths(config.getCloudRequestPayloadTagging())
41+
.addParsedRedactionJsonPaths(config.getCloudRequestPayloadTagging())
4242
.build());
4343
}
4444
if (config.isCloudResponsePayloadTaggingEnabled()) {
@@ -47,7 +47,7 @@ public static PayloadTagsProcessor create(Config config) {
4747
new RedactionRules.Builder()
4848
.addRedactionJsonPaths(ConfigDefaults.DEFAULT_CLOUD_COMMON_PAYLOAD_TAGGING)
4949
.addRedactionJsonPaths(ConfigDefaults.DEFAULT_CLOUD_RESPONSE_PAYLOAD_TAGGING)
50-
.addRedactionJsonPaths(config.getCloudResponsePayloadTagging())
50+
.addParsedRedactionJsonPaths(config.getCloudResponsePayloadTagging())
5151
.build());
5252
}
5353
if (redactionRulesByTagPrefix.isEmpty()) {
@@ -145,20 +145,13 @@ public RedactionRules.Builder addRedactionJsonPaths(List<String> jsonPaths) {
145145
return this;
146146
}
147147

148-
private static List<JsonPath> parseJsonPaths(List<String> rules) {
149-
if (rules.isEmpty() || rules.size() == 1 && rules.get(0).equalsIgnoreCase("all")) {
150-
return Collections.emptyList();
151-
}
152-
List<JsonPath> result = new ArrayList<>(rules.size());
153-
for (String rule : rules) {
154-
try {
155-
JsonPath jp = JsonPathParser.parse(rule);
156-
result.add(jp);
157-
} catch (Exception ex) {
158-
log.warn("Skipping failed to parse redaction rule '{}'. {}", rule, ex.getMessage());
159-
}
148+
public RedactionRules.Builder addParsedRedactionJsonPaths(List<JsonPath> jsonPaths) {
149+
if (null == jsonPaths) {
150+
log.warn("Provided JsonPaths list is null, skipping.");
151+
return this;
160152
}
161-
return result;
153+
this.redactionRules.addAll(jsonPaths);
154+
return this;
162155
}
163156

164157
RedactionRules build() {

‎dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonStreamParser.java‎ renamed to ‎dd-trace-core/src/main/java/datadog/trace/core/util/JsonStreamParser.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
package datadog.trace.payloadtags.json;
1+
package datadog.trace.core.util;
22

33
import com.squareup.moshi.JsonReader;
4+
import datadog.trace.util.json.PathCursor;
45
import java.io.ByteArrayInputStream;
56
import java.io.IOException;
67
import java.io.InputStream;

‎dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/PayloadTagsProcessorTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package datadog.trace.core.tagprocessor
33
import com.squareup.moshi.JsonWriter
44
import datadog.trace.payloadtags.PayloadTagsData
55
import datadog.trace.payloadtags.PayloadTagsData.PathAndValue
6-
import datadog.trace.payloadtags.json.PathCursor
6+
import datadog.trace.util.json.PathCursor
77
import datadog.trace.test.util.DDSpecification
88
import datadog.trace.api.Config
99
import okio.Buffer

‎internal-api/src/main/java/datadog/trace/api/Config.java‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,7 @@
673673
import static datadog.trace.util.CollectionUtils.tryMakeImmutableList;
674674
import static datadog.trace.util.CollectionUtils.tryMakeImmutableSet;
675675
import static datadog.trace.util.ConfigStrings.propertyNameToEnvironmentVariableName;
676+
import static datadog.trace.util.json.JsonPathParser.parseJsonPaths;
676677

677678
import datadog.environment.JavaVirtualMachine;
678679
import datadog.environment.OperatingSystem;
@@ -704,6 +705,7 @@
704705
import datadog.trace.util.PidHelper;
705706
import datadog.trace.util.RandomUtils;
706707
import datadog.trace.util.Strings;
708+
import datadog.trace.util.json.JsonPath;
707709
import datadog.trace.util.throwable.FatalAgentMisconfigurationError;
708710
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
709711
import java.io.BufferedReader;
@@ -1283,8 +1285,8 @@ public static String getHostName() {
12831285
private final String agentlessLogSubmissionProduct;
12841286

12851287
private final Set<String> cloudPayloadTaggingServices;
1286-
@Nullable private final List<String> cloudRequestPayloadTagging;
1287-
@Nullable private final List<String> cloudResponsePayloadTagging;
1288+
@Nullable private final List<JsonPath> cloudRequestPayloadTagging;
1289+
@Nullable private final List<JsonPath> cloudResponsePayloadTagging;
12881290
private final int cloudPayloadTaggingMaxDepth;
12891291
private final int cloudPayloadTaggingMaxTags;
12901292

@@ -2861,10 +2863,39 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
28612863
this.cloudPayloadTaggingServices =
28622864
configProvider.getSet(
28632865
TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES, DEFAULT_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES);
2864-
this.cloudRequestPayloadTagging =
2866+
2867+
List<String> cloudReqPayloadTaggingConf =
28652868
configProvider.getList(TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING, null);
2866-
this.cloudResponsePayloadTagging =
2869+
if (null == cloudReqPayloadTaggingConf) {
2870+
// if no configuration is provided, disable payload tagging
2871+
this.cloudRequestPayloadTagging = null;
2872+
} else if (cloudReqPayloadTaggingConf.size() == 1
2873+
&& cloudReqPayloadTaggingConf.get(0).equalsIgnoreCase("all")) {
2874+
// if "all" is specified enable all JSON paths
2875+
this.cloudRequestPayloadTagging = Collections.emptyList();
2876+
} else {
2877+
// parse and validate JSON paths. if none are valid, disable payload tagging
2878+
List<JsonPath> validRequestJsonPaths = parseJsonPaths(cloudReqPayloadTaggingConf);
2879+
this.cloudRequestPayloadTagging =
2880+
validRequestJsonPaths.isEmpty() ? null : validRequestJsonPaths;
2881+
}
2882+
2883+
List<String> cloudRespPayloadTaggingConf =
28672884
configProvider.getList(TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING, null);
2885+
if (null == cloudRespPayloadTaggingConf) {
2886+
// if no configuration is provided, disable payload tagging
2887+
this.cloudResponsePayloadTagging = null;
2888+
} else if (cloudRespPayloadTaggingConf.size() == 1
2889+
&& cloudRespPayloadTaggingConf.get(0).equalsIgnoreCase("all")) {
2890+
// if "all" is specified enable all JSON paths
2891+
this.cloudResponsePayloadTagging = Collections.emptyList();
2892+
} else {
2893+
// parse and validate JSON paths. if none are valid, disable payload tagging
2894+
List<JsonPath> validResponseJsonPaths = parseJsonPaths(cloudRespPayloadTaggingConf);
2895+
this.cloudResponsePayloadTagging =
2896+
validResponseJsonPaths.isEmpty() ? null : validResponseJsonPaths;
2897+
}
2898+
28682899
this.cloudPayloadTaggingMaxDepth =
28692900
configProvider.getInteger(TRACE_CLOUD_PAYLOAD_TAGGING_MAX_DEPTH, 10);
28702901
this.cloudPayloadTaggingMaxTags =
@@ -5315,7 +5346,7 @@ public boolean isCloudPayloadTaggingEnabled() {
53155346
return isCloudRequestPayloadTaggingEnabled() || isCloudResponsePayloadTaggingEnabled();
53165347
}
53175348

5318-
public List<String> getCloudRequestPayloadTagging() {
5349+
public List<JsonPath> getCloudRequestPayloadTagging() {
53195350
return cloudRequestPayloadTagging == null
53205351
? Collections.emptyList()
53215352
: cloudRequestPayloadTagging;
@@ -5325,7 +5356,7 @@ public boolean isCloudRequestPayloadTaggingEnabled() {
53255356
return cloudRequestPayloadTagging != null;
53265357
}
53275358

5328-
public List<String> getCloudResponsePayloadTagging() {
5359+
public List<JsonPath> getCloudResponsePayloadTagging() {
53295360
return cloudResponsePayloadTagging == null
53305361
? Collections.emptyList()
53315362
: cloudResponsePayloadTagging;

‎dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonPath.java‎ renamed to ‎internal-api/src/main/java/datadog/trace/util/json/JsonPath.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package datadog.trace.payloadtags.json;
1+
package datadog.trace.util.json;
22

33
import java.util.ArrayList;
44
import java.util.Collection;

‎dd-trace-core/src/main/java/datadog/trace/payloadtags/json/JsonPathParser.java‎ renamed to ‎internal-api/src/main/java/datadog/trace/util/json/JsonPathParser.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
1-
package datadog.trace.payloadtags.json;
1+
package datadog.trace.util.json;
22

33
import static java.lang.Character.isDigit;
44

5+
import java.util.ArrayList;
6+
import java.util.Collections;
7+
import java.util.List;
8+
import org.slf4j.Logger;
9+
import org.slf4j.LoggerFactory;
10+
511
public class JsonPathParser {
612

713
public static final class ParseError extends Exception {
@@ -26,6 +32,24 @@ public ParseError(CharSequence path, int position, String error) {
2632
private static final char DOUBLE_QUOTE = '"';
2733
private static final char ESC = '\\';
2834

35+
private static final Logger log = LoggerFactory.getLogger(JsonPathParser.class);
36+
37+
public static List<JsonPath> parseJsonPaths(List<String> rules) {
38+
if (null == rules || rules.isEmpty()) {
39+
return Collections.emptyList();
40+
}
41+
List<JsonPath> result = new ArrayList<>(rules.size());
42+
for (String rule : rules) {
43+
try {
44+
JsonPath jp = parse(rule);
45+
result.add(jp);
46+
} catch (Exception ex) {
47+
log.warn("Failed to parse redaction rule '{}'. {}. Skipping rule.", rule, ex.getMessage());
48+
}
49+
}
50+
return result;
51+
}
52+
2953
public static JsonPath parse(String path) throws ParseError {
3054
Cursor cur = new Cursor(path);
3155

‎dd-trace-core/src/main/java/datadog/trace/payloadtags/json/PathCursor.java‎ renamed to ‎internal-api/src/main/java/datadog/trace/util/json/PathCursor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package datadog.trace.payloadtags.json;
1+
package datadog.trace.util.json;
22

33
import java.util.Arrays;
44

0 commit comments

Comments
 (0)