Add fleet config change logger #4050
Conversation
|
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
|
This pull request does not have a backport label. Could you fix it @fearful-symmetry? 🙏
NOTE: |
| if c.logger.IsDebug() { | ||
| c.checkAndLogUpdate() | ||
| } |
There was a problem hiding this comment.
IMO this is valuable even when not in debug mode, policy changes aren't that frequent so the actual amortized cost of this should be low.
I'd vote we turn this on by default.
There was a problem hiding this comment.
Yah, seems useful, but I was kinda worried about overhead, particularly if we start adding info on component updates. Should probably run some benchmarks and see what it looks like.
|
Alright, completely refactored this to properly print updated component/units, and also clean it up. There's still a few more tests to add, but other than that, should be good. |
| type compCheck struct { | ||
| inNew bool | ||
| inLast bool | ||
|
|
||
| diffUnits map[string]diffCheck | ||
| } |
There was a problem hiding this comment.
[Nit]
Whys is compCheck defined only here, but diffCheck in the package level? I think it'd be more clear if they both were defined together at the package level.
There was a problem hiding this comment.
Yah, that was done entirely because compCheck isn't used outside of one method, and diffCheck is. I didn't want to clutter up coordinator.go.
| if outDiff, ok := outDiffMap[outName]; ok { | ||
| outDiff.inLast = true | ||
| outDiffMap[outName] = outDiff | ||
| } else { | ||
| outDiffMap[outName] = diffCheck{inLast: true} | ||
| } |
There was a problem hiding this comment.
are you checking if it exists to "append" the inLast: true without changing the other values?
There was a problem hiding this comment.
yah, the logic is to update if it exists, add otherwise.
|
test failed on windows, but not for the reasons expected? |
AndersonQ
dismissed
their stale review
I'll be on PTO for tne next 3 work days
|
@cmacknz @fearful-symmetry I'll be on PTO until next Tue, so feel free to proceed without my review. I've already removed my "request changes" |
|
98.2% Coverage on New Code
0.0% Duplication on New Code
closes #3406
What does this PR do?
This is a basic implementation of #3406. When triggered, it prints a change detailing added and removed units:
Right now this PR is fairly basic and does what we want it to do, so I figured I'd ask if there's anything else we think we should do here. We could drill down even farther, and diff individual streams in an input, etc. The downside here is that diffing an array is kind of complex, so we might be incurring a large cost for complex configs.
Example log lines:
Why is it important?
We need debug data showing us what config has changed before a config is applied.
Checklist
./changelog/fragmentsusing the changelog tool