-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Backport update to elastic-agent-go-sysinfo #40507
Backport update to elastic-agent-go-sysinfo #40507
Conversation
Looks like the merge ate the changelog. |
In effect, this is also backporting #40324 b/c of the sysinfo version. Just checking this is desired in a patch? |
hmm, good point. @AndersonQ is that safe to backport? |
Sigh. So elastic/elastic-agent-system-metrics#163 introduced a breaking change which was fixed in main by #40324, so if we don't want to backport that PR, we'll need to manually make some of the changes here. |
yes, it does have a breaking change, but it's reverting a breaking change. |
I don't see a problem backporting them. Are you sure you linked the right |
We could backport #40324 or just add the changelog form it to this backport |
@AndersonQ can you backport that PR, if it's not too much trouble? |
of course |
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
@cmacknz so, this is a mess. In addition to the changes we discussed previously, there's also elastic/elastic-agent-system-metrics#172, which also introduced a ton of breaking changes, and I don't think we'd want to backport all of that to 8.15 as well. I think we should just make a new branch in elastic-agent-system metrics that's around the tag of v0.10.3, backport the logging fix to that branch, and point the 8.15 branch of beats at that, or perhaps some other branch/release related fix, for the sake of getting this fix out before the freeze for 8.15.1. We should also probably just tag the main of elastic-agent-system-metrics as v1.0, so we force ourselves to have some kind of API guarantees, so this doesn't happen again. |
Agreed. I confess I think we should keep a v0 unless it's under active development and we do not know it's final form yet. @fearful-symmetry I opened a PR to allow to set the hostname/fqdn to be lowercase. I'm using globals, well, because it allows to change the behaviour without changing the API, a quick fix to this mess: |
## What does this PR do? So, a previous PR changed the behavior of the process code such that we're now returning non-fatal errors. However, a bunch of the tests were not updated, and are now failing. I'm a little worried by the fact that none of the tests failed initially, as they fail when I run them locally. Part of the problem is that the CI environment often doesn't have any processes running as other users, meaning we won't hit a lot of code that deals with permissions errors. Still, it feels like _something_ should have failed here. Still investigating. ## Why is it important? This is causing tests here to fail: elastic/beats#40507 ## Checklist - [x] My code follows the style guidelines of this project - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I have added an entry in `CHANGELOG.md`
This pull request is now in conflicts. Could you fix it? 🙏
|
Proposed commit message
See #40491
Updates
elastic-agent-system-metrics
to v0.11.1 to remove an erroneous log line.go get
was insistent on updatinggo-sysinfo
as well.