telemetry: add StartupEventDetails.Overrides#4563
telemetry: add StartupEventDetails.Overrides#4563onetechnical merged 2 commits intoalgorand:masterfrom
Conversation
| return 0 | ||
| } | ||
|
|
||
| var startupConfigCheckFields = []string{ |
There was a problem hiding this comment.
Consider reporting all overrides so that we don't have to keep updating this in the future (or forgetting to).
There was a problem hiding this comment.
I would prefer to keep it very limited and pre-defined to start, because there are a lot of various ways we automatically change config values at startup time, and there are also config vars that are private or not interesting/useful to know if they were changed.
There was a problem hiding this comment.
Having to remember later on what we do/don't care about can brittle.
There was a problem hiding this comment.
why not adding these add annotations to the config struct, and let GetNonDefaultConfigValues process annotations and not this map stored very separately from the config?
Or at least put in into localTemplate.go with a comment explaining what is it?
There was a problem hiding this comment.
oh a struct tag would be a good idea!
Codecov Report
@@ Coverage Diff @@
## master #4563 +/- ##
==========================================
- Coverage 54.15% 54.09% -0.07%
==========================================
Files 401 401
Lines 51628 51642 +14
==========================================
- Hits 27960 27936 -24
- Misses 21319 21349 +30
- Partials 2349 2357 +8
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
gmalouf
left a comment
There was a problem hiding this comment.
Agree with Will on future proofing overridden configs, but not enough to block moving forward.
|
manual testing, after setting config to: node.log contains: |
Summary
For nodes with telemetry enabled, this adds some additional information at startup time about performance-sensitive configuration defaults that have been overridden.
Test Plan
Added unit test.