-
Notifications
You must be signed in to change notification settings - Fork 315
Add Docs on How to Add a Configuration #9835
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 2 commits
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 |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # Add a New Configuration | ||
|
|
||
| This doc will walk through how to properly add and document a new configuration in the Java Tracer. | ||
|
|
||
| ## Where Configurations Live | ||
|
|
||
| All configurations in the Java Tracer are defined in `dd-trace-api/src/main/java/datadog/trace/api/config`. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Configurations are separated into different files based on the product they are related to. e.g. `CiVisiblity` related configurations live in `CiVisibilityConfig`, `Tracer` related in `TracerConfig`, etc. | ||
| Default values for configurations live in `ConfigDefaults.java`. | ||
|
|
||
| Configuration values are read in `Config.java`, which utilizes helper methods in `ConfigProvider.java` to fetch the final value for a configuration after searching through all Configuration Sources and determining the final value based on Source priority. | ||
|
||
| `Config.java` also includes getters that can be used in other classes to get the value of a configuration. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| ## Adding a Configuration | ||
|
|
||
| In order to properly add a new configuration in the tracer, follow the below steps. | ||
| 1. Determine whether this configuration exists in another tracer in the [Feature Parity Dashboard](https://feature-parity.us1.prod.dog/#/configurations?viewType=configurations). Developers can search by Environment Variable name or description of the configuration. | ||
| 1. If the configuration exists in a separate tracer, reuse the name of the existing configuration. If the configuration already exists in the Java Tracer, utilize that instead. | ||
| 2. Add the definition of the configuration to the appropriate class based on its related product. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 1. Consider separating words by `.` and compound words by `-`. Note that `dd.` or `DD_` should not belong in the configuration definition as the base definitions are normalized to include those prefixes when querying the varying Configuration Sources. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 3. If relevant, add the default value of the configuration in a static field in `ConfigDefaults.java`. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 4. Create a local field in `Config.java` to represent the configuration. In the constructor of `Config.java`, call the relevant helper from `ConfigProvider.java` to query and assign the value of the configuration. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 5. Create a getter for the field for other classes to access the value of the configuration. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 6. Add the configuration to the `toString()` method or logging. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 7. Add the Environment Variable name of the configuration to the `supportedConfigurations` key of `metadata/supported-configurations.json` in the format of `ENV_VAR: ["VERSION", ...]`. If the configuration already existed in another tracer, add the version listed on the Feature Parity Dashboard. If introducing a new configuration, provide a version of `A`. | ||
| 1. If there are aliases of the Environment Variable, add them to the `aliases` key of the file. | ||
mhlidd marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 8. If the configuration is unique to all tracers, add it to the [Feature Parity Dashboard](https://feature-parity.us1.prod.dog/#/configurations?viewType=configurations). This ensures that we have good documentation of all configurations supported in the tracer. | ||
|
|
||
| For details on adding environment variables to `metadata/supported-configurations.json` or the Feature Parity Dashboard, refer to this [document](https://datadoghq.atlassian.net/wiki/spaces/APMINT/pages/5372248222/APM+-+Centralized+Configuration+Config+inversion#dd-trace-java). | ||
|
|
||
| ## Verifying the Configuration | ||
|
|
||
| To verify a configuration has been correctly added, developers can modify existing test cases in `ConfigTest.groovy` to set the value of the configuration with various sources and confirm the internal value is correctly set in `Config.java`. | ||
|
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. Would be sick to have a new test populate for the new config automatically... maybe someday. 😄 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 that "automated generated tests" will have real value if it's only testing "getter" behavior to comply with the code coverage policy. On thing I would like at mid term is to have:
It does not have to be a god class with all config from it, it can be split by products too. 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. Sounds like this could be something Config Inversion/Registry could be very useful for with our new v2 format. :) |
||
| Optionally, new test cases can be added for testing specific to the behavior of a configuration. | ||
Uh oh!
There was an error while loading. Please reload this page.