-
Notifications
You must be signed in to change notification settings - Fork 314
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
base: master
Are you sure you want to change the base?
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
|
🎯 Code Coverage 🔗 Commit SHA: 92df76b | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 53 metrics, 12 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.032 s) : 0, 1032057
Total [baseline] (8.664 s) : 0, 8663681
Agent [candidate] (1.017 s) : 0, 1017000
Total [candidate] (8.662 s) : 0, 8661813
section iast
Agent [baseline] (1.168 s) : 0, 1168050
Total [baseline] (9.349 s) : 0, 9349407
Agent [candidate] (1.154 s) : 0, 1154382
Total [candidate] (9.321 s) : 0, 9321180
gantt
title insecure-bank - break down per module: candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.473 ms) : 0, 1473
crashtracking [candidate] (1.444 ms) : 0, 1444
BytebuddyAgent [baseline] (704.488 ms) : 0, 704488
BytebuddyAgent [candidate] (692.491 ms) : 0, 692491
GlobalTracer [baseline] (244.842 ms) : 0, 244842
GlobalTracer [candidate] (242.518 ms) : 0, 242518
AppSec [baseline] (32.365 ms) : 0, 32365
AppSec [candidate] (32.103 ms) : 0, 32103
Debugger [baseline] (6.368 ms) : 0, 6368
Debugger [candidate] (6.238 ms) : 0, 6238
Remote Config [baseline] (676.049 µs) : 0, 676
Remote Config [candidate] (679.627 µs) : 0, 680
Telemetry [baseline] (15.729 ms) : 0, 15729
Telemetry [candidate] (9.327 ms) : 0, 9327
Flare Poller [baseline] (4.831 ms) : 0, 4831
Flare Poller [candidate] (11.055 ms) : 0, 11055
section iast
crashtracking [baseline] (1.482 ms) : 0, 1482
crashtracking [candidate] (1.467 ms) : 0, 1467
BytebuddyAgent [baseline] (829.315 ms) : 0, 829315
BytebuddyAgent [candidate] (816.019 ms) : 0, 816019
GlobalTracer [baseline] (234.966 ms) : 0, 234966
GlobalTracer [candidate] (233.096 ms) : 0, 233096
IAST [baseline] (33.519 ms) : 0, 33519
IAST [candidate] (27.004 ms) : 0, 27004
AppSec [baseline] (28.128 ms) : 0, 28128
AppSec [candidate] (35.496 ms) : 0, 35496
Debugger [baseline] (6.091 ms) : 0, 6091
Debugger [candidate] (6.217 ms) : 0, 6217
Remote Config [baseline] (597.031 µs) : 0, 597
Remote Config [candidate] (604.767 µs) : 0, 605
Telemetry [baseline] (8.382 ms) : 0, 8382
Telemetry [candidate] (8.735 ms) : 0, 8735
Flare Poller [baseline] (4.143 ms) : 0, 4143
Flare Poller [candidate] (4.278 ms) : 0, 4278
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.037 s) : 0, 1037207
Total [baseline] (10.906 s) : 0, 10905528
Agent [candidate] (1.015 s) : 0, 1015306
Total [candidate] (10.596 s) : 0, 10595991
section appsec
Agent [baseline] (1.204 s) : 0, 1203724
Total [baseline] (11.01 s) : 0, 11009954
Agent [candidate] (1.194 s) : 0, 1193519
Total [candidate] (10.796 s) : 0, 10796097
section iast
Agent [baseline] (1.169 s) : 0, 1169360
Total [baseline] (11.147 s) : 0, 11146885
Agent [candidate] (1.168 s) : 0, 1168006
Total [candidate] (11.148 s) : 0, 11147762
section profiling
Agent [baseline] (1.173 s) : 0, 1173415
Total [baseline] (10.944 s) : 0, 10943910
Agent [candidate] (1.17 s) : 0, 1169777
Total [candidate] (10.867 s) : 0, 10866985
gantt
title petclinic - break down per module: candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.478 ms) : 0, 1478
crashtracking [candidate] (1.467 ms) : 0, 1467
BytebuddyAgent [baseline] (707.28 ms) : 0, 707280
BytebuddyAgent [candidate] (692.225 ms) : 0, 692225
GlobalTracer [baseline] (246.534 ms) : 0, 246534
GlobalTracer [candidate] (242.586 ms) : 0, 242586
AppSec [baseline] (32.333 ms) : 0, 32333
AppSec [candidate] (32.144 ms) : 0, 32144
Debugger [baseline] (6.46 ms) : 0, 6460
Debugger [candidate] (6.297 ms) : 0, 6297
Remote Config [baseline] (691.126 µs) : 0, 691
Remote Config [candidate] (675.526 µs) : 0, 676
Telemetry [baseline] (13.952 ms) : 0, 13952
Telemetry [candidate] (9.272 ms) : 0, 9272
Flare Poller [baseline] (7.094 ms) : 0, 7094
Flare Poller [candidate] (9.418 ms) : 0, 9418
section appsec
crashtracking [baseline] (1.483 ms) : 0, 1483
crashtracking [candidate] (1.457 ms) : 0, 1457
BytebuddyAgent [baseline] (725.912 ms) : 0, 725912
BytebuddyAgent [candidate] (716.935 ms) : 0, 716935
GlobalTracer [baseline] (236.136 ms) : 0, 236136
GlobalTracer [candidate] (234.854 ms) : 0, 234854
AppSec [baseline] (174.867 ms) : 0, 174867
AppSec [candidate] (175.09 ms) : 0, 175090
Debugger [baseline] (5.922 ms) : 0, 5922
Debugger [candidate] (6.127 ms) : 0, 6127
Remote Config [baseline] (639.81 µs) : 0, 640
Remote Config [candidate] (631.837 µs) : 0, 632
Telemetry [baseline] (8.477 ms) : 0, 8477
Telemetry [candidate] (8.494 ms) : 0, 8494
Flare Poller [baseline] (3.948 ms) : 0, 3948
Flare Poller [candidate] (3.933 ms) : 0, 3933
IAST [baseline] (25.152 ms) : 0, 25152
IAST [candidate] (24.824 ms) : 0, 24824
section iast
crashtracking [baseline] (1.469 ms) : 0, 1469
crashtracking [candidate] (1.475 ms) : 0, 1475
BytebuddyAgent [baseline] (830.499 ms) : 0, 830499
BytebuddyAgent [candidate] (827.842 ms) : 0, 827842
GlobalTracer [baseline] (235.066 ms) : 0, 235066
GlobalTracer [candidate] (234.524 ms) : 0, 234524
AppSec [baseline] (28.152 ms) : 0, 28152
AppSec [candidate] (35.417 ms) : 0, 35417
Debugger [baseline] (6.124 ms) : 0, 6124
Debugger [candidate] (6.127 ms) : 0, 6127
Remote Config [baseline] (626.163 µs) : 0, 626
Remote Config [candidate] (602.016 µs) : 0, 602
Telemetry [baseline] (8.409 ms) : 0, 8409
Telemetry [candidate] (8.696 ms) : 0, 8696
Flare Poller [baseline] (4.137 ms) : 0, 4137
Flare Poller [candidate] (4.252 ms) : 0, 4252
IAST [baseline] (33.418 ms) : 0, 33418
IAST [candidate] (27.26 ms) : 0, 27260
section profiling
ProfilingAgent [baseline] (109.85 ms) : 0, 109850
ProfilingAgent [candidate] (110.267 ms) : 0, 110267
crashtracking [baseline] (1.463 ms) : 0, 1463
crashtracking [candidate] (1.463 ms) : 0, 1463
BytebuddyAgent [baseline] (725.933 ms) : 0, 725933
BytebuddyAgent [candidate] (724.508 ms) : 0, 724508
GlobalTracer [baseline] (220.615 ms) : 0, 220615
GlobalTracer [candidate] (219.513 ms) : 0, 219513
AppSec [baseline] (32.397 ms) : 0, 32397
AppSec [candidate] (32.473 ms) : 0, 32473
Debugger [baseline] (9.903 ms) : 0, 9903
Debugger [candidate] (7.419 ms) : 0, 7419
Remote Config [baseline] (3.09 ms) : 0, 3090
Remote Config [candidate] (696.885 µs) : 0, 697
Telemetry [baseline] (10.698 ms) : 0, 10698
Telemetry [candidate] (14.346 ms) : 0, 14346
Flare Poller [baseline] (4.129 ms) : 0, 4129
Flare Poller [candidate] (4.909 ms) : 0, 4909
Profiling [baseline] (110.506 ms) : 0, 110506
Profiling [candidate] (110.891 ms) : 0, 110891
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 2 performance regressions! Performance is the same for 8 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section baseline
no_agent (4.326 ms) : 4276, 4375
. : milestone, 4326,
iast (9.886 ms) : 9719, 10054
. : milestone, 9886,
iast_FULL (13.808 ms) : 13529, 14086
. : milestone, 13808,
iast_GLOBAL (10.263 ms) : 10083, 10443
. : milestone, 10263,
profiling (9.124 ms) : 8970, 9278
. : milestone, 9124,
tracing (7.712 ms) : 7594, 7830
. : milestone, 7712,
section candidate
no_agent (4.326 ms) : 4274, 4377
. : milestone, 4326,
iast (9.752 ms) : 9587, 9916
. : milestone, 9752,
iast_FULL (14.144 ms) : 13858, 14429
. : milestone, 14144,
iast_GLOBAL (11.163 ms) : 10965, 11361
. : milestone, 11163,
profiling (8.681 ms) : 8538, 8825
. : milestone, 8681,
tracing (7.562 ms) : 7457, 7668
. : milestone, 7562,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section baseline
no_agent (37.514 ms) : 37209, 37819
. : milestone, 37514,
appsec (47.6 ms) : 47185, 48016
. : milestone, 47600,
code_origins (43.295 ms) : 42927, 43663
. : milestone, 43295,
iast (45.963 ms) : 45568, 46357
. : milestone, 45963,
profiling (49.13 ms) : 48681, 49580
. : milestone, 49130,
tracing (42.442 ms) : 42088, 42797
. : milestone, 42442,
section candidate
no_agent (37.839 ms) : 37534, 38145
. : milestone, 37839,
appsec (49.497 ms) : 49049, 49946
. : milestone, 49497,
code_origins (43.125 ms) : 42762, 43489
. : milestone, 43125,
iast (45.549 ms) : 45151, 45948
. : milestone, 45549,
profiling (46.863 ms) : 46453, 47273
. : milestone, 46863,
tracing (43.626 ms) : 43224, 44027
. : milestone, 43626,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section baseline
no_agent (1.485 ms) : 1474, 1497
. : milestone, 1485,
appsec (3.691 ms) : 3476, 3906
. : milestone, 3691,
iast (2.223 ms) : 2158, 2287
. : milestone, 2223,
iast_GLOBAL (2.259 ms) : 2194, 2324
. : milestone, 2259,
profiling (2.066 ms) : 2014, 2119
. : milestone, 2066,
tracing (2.037 ms) : 1987, 2088
. : milestone, 2037,
section candidate
no_agent (1.483 ms) : 1471, 1495
. : milestone, 1483,
appsec (3.721 ms) : 3504, 3939
. : milestone, 3721,
iast (2.226 ms) : 2161, 2290
. : milestone, 2226,
iast_GLOBAL (2.27 ms) : 2205, 2335
. : milestone, 2270,
profiling (2.087 ms) : 2033, 2141
. : milestone, 2087,
tracing (2.033 ms) : 1982, 2083
. : milestone, 2033,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~92df76b06e, baseline=1.55.0-SNAPSHOT~9f0225c968
dateFormat X
axisFormat %s
section baseline
no_agent (15.512 s) : 15512000, 15512000
. : milestone, 15512000,
appsec (15.038 s) : 15038000, 15038000
. : milestone, 15038000,
iast (18.5 s) : 18500000, 18500000
. : milestone, 18500000,
iast_GLOBAL (18.006 s) : 18006000, 18006000
. : milestone, 18006000,
profiling (15.447 s) : 15447000, 15447000
. : milestone, 15447000,
tracing (15.132 s) : 15132000, 15132000
. : milestone, 15132000,
section candidate
no_agent (15.495 s) : 15495000, 15495000
. : milestone, 15495000,
appsec (14.785 s) : 14785000, 14785000
. : milestone, 14785000,
iast (18.706 s) : 18706000, 18706000
. : milestone, 18706000,
iast_GLOBAL (17.758 s) : 17758000, 17758000
. : milestone, 17758000,
profiling (15.423 s) : 15423000, 15423000
. : milestone, 15423000,
tracing (15.105 s) : 15105000, 15105000
. : milestone, 15105000,
|
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.
🔍 nitpick: Config files are sometimes refer as file ("File.java") and sometimes as class ("File"). It could benefit from uniformity.
-- Have to take a break, will continue the review when I get back --
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.
👏 praise: Thanks for adding some doc about our config system!
❔ question: I could not find mention about the config_norm_rules.json. Should it be part of the doc too or does this part is supposed to go away?
| 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. |
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.
🎯 suggestion: It could be nice to show a list of the various configuration sources and what their purpose are. Typically, the one from CI Vis is not trivial.
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.
@mtoffl01 and I discussed that we wait to add documentation for Sources until Stable/Declarative/Hands-Off Config has a finalized term to be used publicly.
Documentation about sources will likely be a new README that is linked from this page.
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.
I think it's alright to document this now in dd-trace-java docs; in our conversations, I meant that public docs (docs.datadoghq.com) must wait.
The name for this file-based config stuff is definitely, officially Declarative Config 😁 .
Happy to help craft the sources doc if need be.
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.
What could actually be good is if we mention the sources here, and link to the public docs with more info. I don't think devs generally need to be super concerned about the sources when adding new configurations, but a link could always be good to provide more info. WDYT?
docs/add_new_configurations.md
Outdated
| 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. | ||
| 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. | ||
| 3. If relevant, add the default value of the configuration in a static field in `ConfigDefaults.java`. |
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.
💭 thought: Isn't "relevant" ambiguous? Can you explain what would be the cases where having a default makes sense and where is doesn't? Giving examples could work too.
| 2. Add the definition of the configuration to the appropriate class based on its related product. | ||
| 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. | ||
| 3. If relevant, add the default value of the configuration in a static field in `ConfigDefaults.java`. | ||
| 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. |
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.
🎯 suggestion: Should we call out that the field should be final and not change during runtime?
We can add a mention to remote config, dynamic config and snapshot and have them documented later. That would be outside of this PR scope 😉
docs/add_new_configurations.md
Outdated
| 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. |
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.
🎯 suggestion: Having code example to illustrate could be helpful here. Link to the source file too.
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.
All suggestions, no blockers. Thanks for doing this!
| ## Adding Configurations | ||
|
|
||
| Check [Adding a New Configuration](docs/add_new_configurations.md) for instructions on adding a new configuration. |
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.
The term "Configuration" may be obvious to us, but may not be to someone less familiar with the repo. You can clarify by changing the terminology to "Configuration option" or "Configuration setting," or linking to documentation to show examples of what we mean by "Configuration"
| 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. |
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.
I think it's alright to document this now in dd-trace-java docs; in our conversations, I meant that public docs (docs.datadoghq.com) must wait.
The name for this file-based config stuff is definitely, officially Declarative Config 😁 .
Happy to help craft the sources doc if need be.
| 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. |
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.
I'd suggest adding a note here to discourage using the ConfigProvider to get config values; to always use Config getters instead.
| In order to properly add a new configuration in the library, follow the below steps. | ||
| 1. Determine whether this configuration exists in another tracing library 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 tracing library, reuse the name of the existing configuration. If the configuration already exists in the Java Library, utilize that instead. | ||
| 2. Add the definition of the configuration to the appropriate class based on its related product. |
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.
| 2. Add the definition of the configuration to the appropriate class based on its related product. | |
| 2. Add the definition of the configuration to the appropriate "Config" class based on its related product. |
| 1. Determine whether this configuration exists in another tracing library 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 tracing library, reuse the name of the existing configuration. If the configuration already exists in the Java Library, utilize that instead. | ||
| 2. Add the definition of the configuration to the appropriate class based on its related product. | ||
| 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. |
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.
By "words" you mean "words in the name of the config"?
If yes: How about prepending the sentence with: "When choosing a variable name for the new configuration, consider..."
An example always helps 😁
| 2. Add the definition of the configuration to the appropriate class based on its related product. | ||
| 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. | ||
| 3. If a non-null default value for the configuration exists, define the value in a static field in `ConfigDefaults.java`. | ||
| 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. |
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.
"relevant" in "call the relevant helper" is very vague here. What we really mean is, choose the correct getter based on configuration value type. I'd suggest rewording or giving an example.
| 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. | ||
| 3. If a non-null default value for the configuration exists, define the value in a static field in `ConfigDefaults.java`. | ||
| 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. | ||
| 5. Create a getter for the field for other classes to access the value of the configuration. |
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.
| 5. Create a getter for the field for other classes to access the value of the configuration. | |
| 5. Create a getter for the field on the `Config` class for other classes to access the value of the configuration. |
| 3. If a non-null default value for the configuration exists, define the value in a static field in `ConfigDefaults.java`. | ||
| 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. | ||
| 5. Create a getter for the field for other classes to access the value of the configuration. | ||
| 6. Add the configuration to the `toString()` method or logging. |
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.
I think you mean on the Config class? Should clarify.
| } | ||
| } | ||
| ``` | ||
| 8. If the configuration is unique to all tracing libraries, 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 library. |
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.
| 8. If the configuration is unique to all tracing libraries, 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 library. | |
| 8. If the configuration is unique to this library, 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 library. |
|
|
||
| ## 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 comment
The 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. 😄
What Does This Do
There is currently no documentation in the tracer on how to add a new configuration in
dd-trace-java. With the addition of Config Inversion, docs are necessary to describe how to properly create new configurations in the tracer.Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]