Stable configuration draft #8328
Conversation
* Add Stable config source * Add snakeyaml to parse file
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
|
|
||
| private static final HashMap<String, String> parseStableConfig(String filePath) { | ||
| Yaml yaml = new Yaml(new SafeConstructor(new LoaderOptions())); | ||
| InputStream input = new FileInputStream(new File(filePath)); |
There was a problem hiding this comment.
🔵 Code Quality Violation
Do not initialize FileInputStream. Use a Files.newInputStream or Files.newOutputStream instead (...read more)
The classes that creates FileInputStream and FileOutputStream triggers too much garbage collection. Instead, use methods from the nio package that cause less garbage collection.
Learn More
| } | ||
| } | ||
|
|
||
| private static final HashMap<String, String> parseStableConfig(String filePath) { |
There was a problem hiding this comment.
⚪ Code Quality Violation
| private static final HashMap<String, String> parseStableConfig(String filePath) { | |
| private static final Map<String, String> parseStableConfig(String filePath) { |
Avoid using a specific implementation type; use the more general Map instead (...read more)
Relying on particular implementation types, such as, HashSet or LinkedList can limit your adaptability to embrace alternative implementations in the future, particularly as your requirements change and your code needs to undergo changes.
It is recommended to opt for general types such as Set or List when declaring variables and parameters.
|
|
||
| private class StableConfig { | ||
| private String config_id; | ||
| private HashMap<String, Object> apm_configuration_default; |
There was a problem hiding this comment.
⚪ Code Quality Violation
| private HashMap<String, Object> apm_configuration_default; | |
| private Map<String, Object> apm_configuration_default; |
Avoid using a specific implementation type; use the more general Map instead (...read more)
Relying on particular implementation types, such as, HashSet or LinkedList can limit your adaptability to embrace alternative implementations in the future, particularly as your requirements change and your code needs to undergo changes.
It is recommended to opt for general types such as Set or List when declaring variables and parameters.
| private String config_id; | ||
| private HashMap<String, Object> apm_configuration_default; | ||
|
|
||
| private void setApmConfigurationDefault(HashMap<String, Object> m) { |
There was a problem hiding this comment.
⚪ Code Quality Violation
| private void setApmConfigurationDefault(HashMap<String, Object> m) { | |
| private void setApmConfigurationDefault(Map<String, Object> m) { |
Avoid using a specific implementation type; use the more general Map instead (...read more)
Relying on particular implementation types, such as, HashSet or LinkedList can limit your adaptability to embrace alternative implementations in the future, particularly as your requirements change and your code needs to undergo changes.
It is recommended to opt for general types such as Set or List when declaring variables and parameters.
| static final String LOCAL_STABLE_CONFIGURATION_PATH = "/etc/datadog-agent/datadog_apm.yaml"; | ||
|
|
||
| final ConfigOrigin fileOrigin; | ||
| HashMap<String, String> configuration; |
There was a problem hiding this comment.
⚪ Code Quality Violation
| HashMap<String, String> configuration; | |
| Map<String, String> configuration; |
Avoid using a specific implementation type; use the more general Map instead (...read more)
Relying on particular implementation types, such as, HashSet or LinkedList can limit your adaptability to embrace alternative implementations in the future, particularly as your requirements change and your code needs to undergo changes.
It is recommended to opt for general types such as Set or List when declaring variables and parameters.
| StableConfigSource(String file, ConfigOrigin origin) { | ||
| this.fileOrigin = origin; | ||
| try { | ||
| configuration = parseStableConfig(file); | ||
| } catch (Exception e) { | ||
| configuration = new HashMap(); | ||
| } | ||
| } |
There was a problem hiding this comment.
⚪ Code Quality Violation
Consider adding super() or this() to your constructor (...read more)
In Java, it is suggested to call super() in an extended class. This rule will report a violation if both a call to super() and an overloaded constructor are absent.
| try { | ||
| configuration = parseStableConfig(file); | ||
| } catch (Exception e) { | ||
| configuration = new HashMap(); |
There was a problem hiding this comment.
You should still be able to declare configuration as final, javac is smart enough to figure out only a single assignment happens here. For an empty map (if you change the declared type to Map instead of HashMap) you can also use Map.of() or Collections.emptyMap(). Finally, we should at least report the exception if the file exists but is not readable/parseable.
| private static final HashMap<String, String> parseStableConfig(String filePath) { | ||
| Yaml yaml = new Yaml(new SafeConstructor(new LoaderOptions())); | ||
| InputStream input = new FileInputStream(new File(filePath)); | ||
| Object data = yaml.load(input); |
There was a problem hiding this comment.
This is probably leaking a file descriptor. You need to make sure to close the InputStream. Usually, the best to do that is using try ( InputStream input = ... ) {
}
|
|
||
| private class StableConfig { | ||
| private String config_id; | ||
| private HashMap<String, Object> apm_configuration_default; |
There was a problem hiding this comment.
Please follow Java naming conventions -- use camel case not underscores
What Does This Do
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]