-
Notifications
You must be signed in to change notification settings - Fork 319
Match Hands Off Config selectors on process_arguments value #9201
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 11 commits
2df85ac
314525a
b051555
c094176
a58bd09
db6e5c7
5865dc8
2f93fad
ebf333b
75f6dae
cfb37db
a8ff152
6f1ea89
db73932
12c673d
4f4c5bc
2cf1092
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 |
|---|---|---|
|
|
@@ -13,7 +13,6 @@ | |
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.function.BiPredicate; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
|
|
@@ -104,35 +103,35 @@ private static boolean doesRuleMatch(Rule rule) { | |
| return true; // Return true if all selectors match | ||
| } | ||
|
|
||
| private static boolean validOperatorForLanguageOrigin(String operator) { | ||
| operator = operator.toLowerCase(); | ||
| // "exists" is not valid | ||
| switch (operator) { | ||
| case "equals": | ||
| case "starts_with": | ||
| case "ends_with": | ||
| case "contains": | ||
| return true; | ||
| default: | ||
| return false; | ||
| private static boolean matchOperator(String value, String operator, List<String> matches) { | ||
| // not sure if these are nullable, but the semantics makes sense | ||
| // and that will save us from a NPE | ||
| if (value == null || operator == null) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private static boolean checkEnvMatches( | ||
| List<String> values, List<String> matches, BiPredicate<String, String> compareFunc) { | ||
| // envValue shouldn't be null, but doing an extra check to avoid NullPointerException on | ||
| // compareFunc.test | ||
| if (values == null) { | ||
| if (operator.equals("exists")) { | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return true; | ||
| } | ||
| if (matches == null) { | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
| } | ||
| value = value.toLowerCase(); | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| for (String match : matches) { | ||
| if (match == null) { | ||
| continue; | ||
| } | ||
| for (String value : values) { | ||
| if (compareFunc.test(value, match.toLowerCase())) { | ||
| return true; | ||
| } | ||
| match = match.toLowerCase(); | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| switch (operator) { | ||
| case "equals": | ||
| return value.equals(match); | ||
| case "starts_with": | ||
| return value.startsWith(match); | ||
| case "ends_with": | ||
| return value.endsWith(match); | ||
| case "contains": | ||
| return value.contains(match); | ||
| default: | ||
|
Comment on lines
+123
to
+132
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. Can't we use method reference here and loop on the matches with instead? |
||
| return false; | ||
| } | ||
| } | ||
| return false; | ||
|
|
@@ -141,53 +140,35 @@ private static boolean checkEnvMatches( | |
| // We do all of the case insensitivity modifications in this function, because each selector will | ||
| // be viewed just once | ||
| static boolean selectorMatch(String origin, List<String> matches, String operator, String key) { | ||
| if (operator == null) { | ||
| return false; | ||
| } | ||
| operator = operator.toLowerCase(); | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| switch (origin.toLowerCase()) { | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| case "language": | ||
| if (!validOperatorForLanguageOrigin(operator)) { | ||
| if (operator.equals("exists")) { | ||
mtoffl01 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
| } | ||
|
||
| for (String entry : matches) { | ||
| // loose match on any reference to "*java*" | ||
| if (entry.toLowerCase().contains("java")) { | ||
| return true; | ||
| } | ||
| } | ||
| return matchOperator("java", operator, matches); | ||
| case "environment_variables": | ||
| if (key == null) { | ||
| return false; | ||
| } | ||
| String envValue = System.getenv(key.toUpperCase()); | ||
| if (envValue == null) { | ||
| return matchOperator(envValue, operator, matches); | ||
| case "process_arguments": | ||
| if (key == null) { | ||
| return false; | ||
| } | ||
| envValue = envValue.toLowerCase(); | ||
| switch (operator.toLowerCase()) { | ||
| case "exists": | ||
| // We don't care about the value | ||
| return true; | ||
| case "equals": | ||
| return checkEnvMatches( | ||
| Collections.singletonList(envValue), matches, String::equalsIgnoreCase); | ||
| case "starts_with": | ||
| return checkEnvMatches( | ||
| Collections.singletonList(envValue), matches, String::startsWith); | ||
| case "ends_with": | ||
| return checkEnvMatches(Collections.singletonList(envValue), matches, String::endsWith); | ||
| case "contains": | ||
| return checkEnvMatches(Collections.singletonList(envValue), matches, String::contains); | ||
| default: | ||
| return false; | ||
| } | ||
| case "process_arguments": | ||
| // TODO: flesh out the meaning of each operator for process_arguments | ||
| if (!key.startsWith("-D")) { | ||
| log.warn( | ||
| "Ignoring unsupported process_arguments entry in selector match, '{}'. Only system properties specified with the '-D' prefix are supported.", | ||
| key); | ||
| return false; | ||
| } | ||
| // Cut the -D prefix | ||
| return System.getProperty(key.substring(2)) != null; | ||
| String argValue = System.getProperty(key.substring(2)); | ||
| return matchOperator(argValue, operator, matches); | ||
| case "tags": | ||
| // TODO: Support this down the line (Must define the source of "tags" first) | ||
| return false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,12 +62,40 @@ apm_configuration_rules: | |
| Files.delete(filePath) | ||
| } | ||
|
|
||
| def "test parse and template"() { | ||
| when: | ||
| Path filePath = Files.createTempFile("testFile_", ".yaml") | ||
| then: | ||
| if (filePath == null) { | ||
| throw new AssertionError("Failed to create: " + filePath) | ||
| } | ||
|
Comment on lines
+68
to
+71
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.
If the YAML is constant, store it in test resources instead. |
||
|
|
||
| when: | ||
| String yaml = """ | ||
| apm_configuration_rules: | ||
| - selectors: | ||
| - origin: process_arguments | ||
| key: "-Dtest_parse_and_template" | ||
| operator: exists | ||
| configuration: | ||
| DD_SERVICE: {{process_arguments['-Dtest_parse_and_template']}} | ||
| """ | ||
| System.setProperty("test_parse_and_template", "myservice") | ||
| Files.write(filePath, yaml.getBytes()) | ||
| StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) | ||
|
|
||
| then: | ||
| cfg.get("DD_SERVICE") == "myservice" | ||
| } | ||
|
|
||
| def "test selectorMatch"() { | ||
| when: | ||
| // Env vars | ||
| injectEnvConfig("DD_PROFILING_ENABLED", "true") | ||
| injectEnvConfig("DD_SERVICE", "mysvc") | ||
| injectEnvConfig("DD_TAGS", "team:apm,component:web") | ||
| System.setProperty("test_selectorMatch", "value1") | ||
|
|
||
| def match = StableConfigParser.selectorMatch(origin, matches, operator, key) | ||
|
|
||
| then: | ||
|
|
@@ -83,6 +111,7 @@ apm_configuration_rules: | |
| "language" | ["java"] | "exists" | "" | false | ||
| "language" | ["java"] | "something unexpected" | "" | false | ||
| "environment_variables" | [] | "exists" | "DD_TAGS" | true | ||
| "environment_variables" | null | "exists" | "DD_TAGS" | true | ||
| "environment_variables" | ["team:apm"] | "contains" | "DD_TAGS" | true | ||
| "ENVIRONMENT_VARIABLES" | ["TeAm:ApM"] | "CoNtAiNs" | "Dd_TaGs" | true // check case insensitivity | ||
| "environment_variables" | ["team:apm"] | "equals" | "DD_TAGS" | false | ||
|
|
@@ -96,6 +125,13 @@ apm_configuration_rules: | |
| "environment_variables" | ["svc"] | "contains" | "DD_SERVICE" | true | ||
| "environment_variables" | ["other"] | "contains" | "DD_SERVICE" | false | ||
| "environment_variables" | [null] | "contains" | "DD_SERVICE" | false | ||
| "environment_variables" | [] | "equals" | null | false | ||
| "environment_variables" | null | "equals" | "DD_SERVICE" | false | ||
| "language" | ["java"] | null | "" | false | ||
| "process_arguments" | null | "exists" | "-Dtest_selectorMatch" | true | ||
| "process_arguments" | null | "exists" | "-Darg2" | false | ||
| "process_arguments" | ["value1"] | "equals" | "-Dtest_selectorMatch" | true | ||
| "process_arguments" | ["value2"] | "equals" | "-Dtest_selectorMatch" | false | ||
| } | ||
|
|
||
| def "test duplicate entries not allowed"() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.