Skip to content
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

Adds a "type" field to the filesystem beat #4717

Merged
merged 2 commits into from
Aug 2, 2017

Conversation

cv
Copy link
Contributor

@cv cv commented Jul 20, 2017

Its value is copied from sigar.FileSystem.SysTypeName and passed through so that it becomes possible to filter out overlay, tmpfs or otherwise uninteresting filesystems.

Fixes #3459.

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically on build-eu-00. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cv cv force-pushed the metricbeat/filesystem/add-sys-fs-type branch from 90f093e to 903013d Compare July 20, 2017 11:20
@tsg tsg added Metricbeat Metricbeat review labels Jul 20, 2017
@tsg
Copy link
Contributor

tsg commented Jul 20, 2017

@cv Thanks for working on this. Run make update to solve the check failure.

@cv cv force-pushed the metricbeat/filesystem/add-sys-fs-type branch from d8ba66a to 08eece1 Compare July 20, 2017 14:02
@cv
Copy link
Contributor Author

cv commented Jul 20, 2017

@tsg you're most welcome, and I'm sorry I can't seem to fix the build breakage through make update. I'm still pretty unfamiliar with the build/doc system, so do you have any pointers?

@tsg
Copy link
Contributor

tsg commented Jul 21, 2017

When I run make update top level in your branch, it modifies two files:

	modified:   libbeat/_meta/kibana/5.x/index-pattern/libbeat.json
	modified:   libbeat/_meta/kibana/default/index-pattern/libbeat.json

Do you get the same? Or some error during make update?

@tsg
Copy link
Contributor

tsg commented Jul 21, 2017

make update is our one command to re-create all generated files (includes the notice file, the docs, etc.)

@cv
Copy link
Contributor Author

cv commented Jul 21, 2017

Oh, this could be it. I get no changes when I run it. Could it be an env difference?

Output: https://gist.github.com/cv/4b596841382fb6d4193a86d95ba05d38

@tsg
Copy link
Contributor

tsg commented Jul 21, 2017

Can you try a rebase to master (make sure master is up to date, then git rebase master) and them make update again?

@cv cv force-pushed the metricbeat/filesystem/add-sys-fs-type branch from 4066ab0 to 7c78677 Compare July 21, 2017 14:52
@cv
Copy link
Contributor Author

cv commented Jul 21, 2017

Done… still, no files get updated. I'm going to set up a Linux build environment to see if there are any differences.

@cv cv force-pushed the metricbeat/filesystem/add-sys-fs-type branch 2 times, most recently from 4f75473 to c5bc748 Compare July 21, 2017 19:00
@cv
Copy link
Contributor Author

cv commented Jul 21, 2017

Ok, so when running on Linux (a little docker image did the job), the files do get updated and I get the same result as you, @tsg. I'm wondering why the builds failed even so, as nothing in the output immediately jumps at me.

@tsg
Copy link
Contributor

tsg commented Jul 24, 2017

jenkins, test it

@tsg
Copy link
Contributor

tsg commented Jul 24, 2017

@cv Travis is green now, we had some issues lately with Travis timing out on some of our stuff, so it's flaky at the moment. We're looking for solutions.

@tsg
Copy link
Contributor

tsg commented Jul 24, 2017

@cv can you add a line in the CHANGELOG.asciidoc file, please?

@@ -8762,6 +8762,14 @@ The disk name. For example: `/dev/disk1`


[float]
=== system.filesystem.device_type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking if we shouldn't name this simpler as system.filesystem.type. I usually think of ext4 (for example) as a filesystem type, so that seems to fit. device_type, on the other had, makes me think of hardware.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good rationale. I'll change it in a few minutes.

@cv cv force-pushed the metricbeat/filesystem/add-sys-fs-type branch from 32aac60 to 86abc67 Compare July 24, 2017 16:58
@cv cv changed the title Adds a "device_type" field to the filesystem beat Adds a "type" field to the filesystem beat Jul 25, 2017
@cv
Copy link
Contributor Author

cv commented Jul 27, 2017

This last build looks like the least fail-y so far :)

@tsg, I'd say work here is done, and unless you have any other points you'd like me to work on, can I clean up / rebase the commit history? Or do you typically do that when squashing into master?

@tsg
Copy link
Contributor

tsg commented Jul 28, 2017

@cv Yes, please rebase & squash. I'd do it on merging, but now there's a conflict anyway. Thanks a lot for your work!

Its value is copied from sigar.FileSystem.SysTypeName (on Windows, sigar.FileSystem.TypeName) and passed through so that it becomes possible to filter out `overlay`, `tmpfs` or otherwise uninteresting filesystems.

Fixes elastic#3459.
@cv cv force-pushed the metricbeat/filesystem/add-sys-fs-type branch from 8a34b57 to 3344656 Compare July 31, 2017 03:39
@cv
Copy link
Contributor Author

cv commented Jul 31, 2017

All squashed (except for the merge conflict fix, which I left as its own commit so it can be reviewed).

@andrewkroh andrewkroh merged commit 94a197b into elastic:master Aug 2, 2017
@andrewkroh andrewkroh added the needs_backport PR is waiting to be backported to other branches. label Aug 2, 2017
@cv cv deleted the metricbeat/filesystem/add-sys-fs-type branch August 2, 2017 20:59
@tsg tsg removed the needs_backport PR is waiting to be backported to other branches. label Aug 24, 2017
tsg pushed a commit to tsg/beats that referenced this pull request Aug 24, 2017
Its value is copied from sigar.FileSystem.SysTypeName (on Windows, sigar.FileSystem.TypeName) and passed through so that it becomes possible to filter out `overlay`, `tmpfs` or otherwise uninteresting filesystems.

Fixes elastic#3459.

(cherry picked from commit 94a197b)
andrewkroh pushed a commit that referenced this pull request Aug 24, 2017
…4991)

* Adds a "type" field to the filesystem beat (#4717) (#4717)

Its value is copied from sigar.FileSystem.SysTypeName (on Windows, sigar.FileSystem.TypeName) and passed through so that it becomes possible to filter out `overlay`, `tmpfs` or otherwise uninteresting filesystems.

Fixes #3459.

(cherry picked from commit 94a197b)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…m beat (elastic#4991)

* Adds a "type" field to the filesystem beat (elastic#4717) (elastic#4717)

Its value is copied from sigar.FileSystem.SysTypeName (on Windows, sigar.FileSystem.TypeName) and passed through so that it becomes possible to filter out `overlay`, `tmpfs` or otherwise uninteresting filesystems.

Fixes elastic#3459.

(cherry picked from commit 7e2396a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants