-
Notifications
You must be signed in to change notification settings - Fork 333
Stable Config improvements #9259
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 9 commits
370e894
df4da2f
7049d36
e730cbe
5b639aa
85d11cf
6ab4b6c
007f58a
ac69f2d
473c59b
feb1ba0
f93546a
780aee3
ac5e05a
bcebbcb
0baccfd
957d9c9
b9870bd
5019584
e8d6e8f
1fefb17
058cbda
5460c8a
37da711
5dac7f8
5104ac5
13bef8d
2706bca
4dde216
bfd5d7b
16d5f36
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 |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName; | ||
|
|
||
| import datadog.trace.api.ConfigOrigin; | ||
| import datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException; | ||
| import java.io.File; | ||
| import java.util.Collections; | ||
| import java.util.Map; | ||
|
|
@@ -37,11 +38,21 @@ public final class StableConfigSource extends ConfigProvider.Source { | |
| try { | ||
| log.debug("Stable configuration file found at path: {}", file); | ||
| cfg = StableConfigParser.parse(filePath); | ||
| } catch (Throwable e) { | ||
| log.warn( | ||
| "Encountered the following exception when attempting to read stable configuration file at path: {}, dropping configs.", | ||
| file, | ||
| e); | ||
| } catch (StableConfigMappingException | ||
| | IllegalArgumentException | ||
| | ClassCastException | ||
| | NullPointerException e) { | ||
|
mtoffl01 marked this conversation as resolved.
Outdated
AlexeyKuznetsov-DD marked this conversation as resolved.
Outdated
|
||
| log.warn("YAML mapping error in stable configuration file {}: {}", filePath, e.getMessage()); | ||
| cfg = StableConfig.EMPTY; | ||
| } catch (Exception e) { | ||
|
AlexeyKuznetsov-DD marked this conversation as resolved.
Outdated
|
||
| if (log.isDebugEnabled()) { | ||
| log.error("Unexpected error while reading stable configuration file {}: {}", filePath, e); | ||
| } else { | ||
|
Comment on lines
+48
to
+52
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. Not sure I understand this
Contributor
Author
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. As for the As for the placeholder: I was not aware, thanks for sharing. 🤔 I will change this accordingly, based on what we decide on the above.
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. got it! make sense in debug to print stack trace |
||
| log.error( | ||
| "Unexpected error while reading stable configuration file {}: {}", | ||
| filePath, | ||
| e.getMessage()); | ||
| } | ||
|
mtoffl01 marked this conversation as resolved.
|
||
| cfg = StableConfig.EMPTY; | ||
| } | ||
| this.config = cfg; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package datadog.trace.bootstrap.config.provider.stableconfig; | ||
|
|
||
| public class StableConfigMappingException extends RuntimeException { | ||
| public StableConfigMappingException(String message) { | ||
| super(message); | ||
| } | ||
|
|
||
| public static String safeToString(Object value) { | ||
|
mtoffl01 marked this conversation as resolved.
Outdated
|
||
| if (value == null) return "null"; | ||
| String str = value.toString(); | ||
| if (str.length() > 100) { | ||
|
mtoffl01 marked this conversation as resolved.
Outdated
|
||
| return str.substring(0, 100) + "...(truncated)"; | ||
| } | ||
|
AlexeyKuznetsov-DD marked this conversation as resolved.
|
||
| return str; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package datadog.trace.bootstrap.config.provider | ||
|
|
||
| import datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException | ||
| import spock.lang.Specification | ||
|
|
||
| class StableConfigMappingExceptionTest extends Specification { | ||
|
|
||
| def "constructors work as expected"() { | ||
| when: | ||
| def ex1 = new StableConfigMappingException("msg") | ||
| def ex2 = new StableConfigMappingException("msg2") | ||
|
|
||
| then: | ||
| ex1.message == "msg" | ||
| ex1.cause == null | ||
| ex2.message == "msg2" | ||
| } | ||
|
|
||
| def "safeToString handles null"() { | ||
| expect: | ||
| StableConfigMappingException.safeToString(null) == "null" | ||
| } | ||
|
|
||
| def "safeToString handles short string"() { | ||
| expect: | ||
| StableConfigMappingException.safeToString("short string") == "short string" | ||
| } | ||
|
|
||
| def "safeToString handles long string"() { | ||
| given: | ||
| def longStr = "a" * 101 | ||
| expect: | ||
| StableConfigMappingException.safeToString(longStr) == ("a" * 100) + "...(truncated)" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍