-
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
Filesystem refactor #31001
Filesystem refactor #31001
Conversation
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
This pull request does not have a backport label. Could you fix it @fearful-symmetry? 🙏
NOTE: |
This pull request is now in conflicts. Could you fix it? 🙏
|
This pull request is now in conflicts. Could you fix it? 🙏
|
/test |
This pull request is now in conflicts. Could you fix it? 🙏
|
libbeat/opt/opt.go
Outdated
@@ -33,15 +33,15 @@ type Int struct { | |||
value int | |||
} |
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.
Do you mind opening a PR to elastic-agent-libs
to update the opt
package there as well?
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.
Oh, it's already in there. Huh.
Looking at this again it's looked like it has changed a fair bit.
What is the bug that was fixed here? It's hard to spot in all the code? Also you need a CHANGELOG entry for it. |
@cmacknz yup, hence why I asked for another review in slack. The original code wasn't respecting |
Alright, changlog added, reverted some of the opt changes. |
This pull request is now in conflicts. Could you fix it? 🙏
|
- name: options | ||
type: keyword | ||
description: > | ||
The options present on the filesystem mount. |
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.
Should this option be mentioned in the CHANGELOG?
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.
LGTM again, I can't spot anything problematic.
This pull request is now in conflicts. Could you fix it? 🙏
|
It seems like this made it into version 8.3.1 as per https://github.com/elastic/beats/blob/v8.3.1/CHANGELOG.asciidoc#L34. |
* first pass at filesystem refactoring * cleanup code * final cleanup * cleanup, docs * license headers * make linter happy * try to hack around linter platform issues * fix function sig * minor fixes, formatting * try to make linter happy * more linter fixes * hopefully the last linter cleanup * try to fix unit tests * fix tests * refactor after merges from elastic-agent libs * make linter happy * try to update deps * okay, try that * fix merge * add changelog, revert opt changes * fix broken merge * mage update (cherry picked from commit 1d4a7ca) # Conflicts: # go.sum # metricbeat/module/system/fields.go # metricbeat/module/system/filesystem/_meta/fields.yml # metricbeat/module/system/filesystem/filesystem.go # metricbeat/module/system/filesystem/filesystem_test.go # metricbeat/module/system/filesystem/helper.go # metricbeat/module/system/fsstat/fsstat.go # metricbeat/module/system/test_system.py
* Filesystem refactor (#31001) * first pass at filesystem refactoring * cleanup code * final cleanup * cleanup, docs * license headers * make linter happy * try to hack around linter platform issues * fix function sig * minor fixes, formatting * try to make linter happy * more linter fixes * hopefully the last linter cleanup * try to fix unit tests * fix tests * refactor after merges from elastic-agent libs * make linter happy * try to update deps * okay, try that * fix merge * add changelog, revert opt changes * fix broken merge * mage update (cherry picked from commit 1d4a7ca) # Conflicts: # go.sum # metricbeat/module/system/fields.go # metricbeat/module/system/filesystem/_meta/fields.yml # metricbeat/module/system/filesystem/filesystem.go # metricbeat/module/system/filesystem/filesystem_test.go # metricbeat/module/system/filesystem/helper.go # metricbeat/module/system/fsstat/fsstat.go # metricbeat/module/system/test_system.py * attempt to clean up backport * update notice * try to make linter happy * still fixing up backport * still un-breaking merge * fix changelog Co-authored-by: Alex K <8418476+fearful-symmetry@users.noreply.github.com> Co-authored-by: Alex Kristiansen <alex.kristiansen@elastic.co>
* first pass at filesystem refactoring * cleanup code * final cleanup * cleanup, docs * license headers * make linter happy * try to hack around linter platform issues * fix function sig * minor fixes, formatting * try to make linter happy * more linter fixes * hopefully the last linter cleanup * try to fix unit tests * fix tests * refactor after merges from elastic-agent libs * make linter happy * try to update deps * okay, try that * fix merge * add changelog, revert opt changes * fix broken merge * mage update
What does this PR do?
This is a refactor of the system/filesystem and system/fsstat metricsets for the purpose of fixing #30590 Also also continuing to depricate the
gosigar
library.I'm still in the process of testing this, but I'm currently juggling 3 other things so I at least need to get the PR in so I don't loose track of what I'm doing.
Why is it important?
This fixes a bug, removes code from metricbeat, and removes deprecated gosigar code.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Fixes #30590