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.

Sign up for GitHub

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

Update elastic-agent-system-metrics to v0.8.1 #37027

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Update elastic-agent-system-metrics to v0.8.1 #37027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b9a377e
Update elastic-agent-system-metrics to v0.8.0
belimawr Nov 3, 2023
2f3b207
Fix tests
belimawr Nov 6, 2023
ad4675b
Update elastic-agent-system-metrics to v0.8.1
belimawr Nov 3, 2023
3a08548
fix unix tests
belimawr Nov 8, 2023
1480659
Add debug and save logs after test run
belimawr Nov 13, 2023
bab5bb4
Fix python test, remove debug
belimawr Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- Fix CassandraConnectionClosures metric configuration {pull}34742[34742]
- Fix event mapping implementation for statsd module {pull}36925[36925]
- The region and availability_zone ecs fields nested within the cloud field. {pull}37015[37015]
- Fix CPU and memory metrics collection from privileged process on Windows {issue}17314[17314]{pull}37027[37027]

*Osquerybeat*

Expand Down
4 changes: 2 additions & 2 deletions NOTICE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13026,11 +13026,11 @@ these terms.

--------------------------------------------------------------------------------
Dependency : github.com/elastic/elastic-agent-system-metrics
Version: v0.7.0
Version: v0.8.1
Licence type (autodetected): Apache-2.0
--------------------------------------------------------------------------------

Contents of probable licence file $GOMODCACHE/github.com/elastic/elastic-agent-system-metrics@v0.7.0/LICENSE.txt:
Contents of probable licence file $GOMODCACHE/github.com/elastic/elastic-agent-system-metrics@v0.8.1/LICENSE.txt:

Apache License
Version 2.0, January 2004
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ require (
github.com/elastic/elastic-agent-autodiscover v0.6.4
github.com/elastic/elastic-agent-libs v0.6.2
github.com/elastic/elastic-agent-shipper-client v0.5.1-0.20230228231646-f04347b666f3
github.com/elastic/elastic-agent-system-metrics v0.7.0
github.com/elastic/elastic-agent-system-metrics v0.8.1
github.com/elastic/go-elasticsearch/v8 v8.10.0
github.com/elastic/mito v1.6.0
github.com/elastic/toutoumomoma v0.0.0-20221026030040-594ef30cb640
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,8 @@ github.com/elastic/elastic-agent-libs v0.6.2 h1:tE5pFK4y7xm1FtXm+r+63G7STjJAaWh3
github.com/elastic/elastic-agent-libs v0.6.2/go.mod h1:o+EySawBZGeYu49shJxerg2wRCimS1dhrD4As0MS700=
github.com/elastic/elastic-agent-shipper-client v0.5.1-0.20230228231646-f04347b666f3 h1:sb+25XJn/JcC9/VL8HX4r4QXSUq4uTNzGS2kxOE7u1U=
github.com/elastic/elastic-agent-shipper-client v0.5.1-0.20230228231646-f04347b666f3/go.mod h1:rWarFM7qYxJKsi9WcV6ONcFjH/NA3niDNpTxO+8/GVI=
github.com/elastic/elastic-agent-system-metrics v0.7.0 h1:qDLY30UDforSd/TfHfqUDiiHSL6Nu6qLXHsKSxz4OuQ=
github.com/elastic/elastic-agent-system-metrics v0.7.0/go.mod h1:9C1UEfj0P687HAzZepHszN6zXA+2tN2Lx3Osvq1zby8=
github.com/elastic/elastic-agent-system-metrics v0.8.1 h1:eg6actuLeGJlIJFotHRdlAsz/3WhX2G8E0qI301IKBA=
github.com/elastic/elastic-agent-system-metrics v0.8.1/go.mod h1:9C1UEfj0P687HAzZepHszN6zXA+2tN2Lx3Osvq1zby8=
github.com/elastic/elastic-transport-go/v8 v8.0.0-20230329154755-1a3c63de0db6/go.mod h1:87Tcz8IVNe6rVSLdBux1o/PEItLtyabHU3naC7IoqKI=
github.com/elastic/elastic-transport-go/v8 v8.3.0 h1:DJGxovyQLXGr62e9nDMPSxRyWION0Bh6d9eCFBriiHo=
github.com/elastic/elastic-transport-go/v8 v8.3.0/go.mod h1:87Tcz8IVNe6rVSLdBux1o/PEItLtyabHU3naC7IoqKI=
Expand Down
15 changes: 11 additions & 4 deletions metricbeat/module/system/test_system.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@
# cmdline is also part of the system process fields, but it may not be present
# for some kernel level processes. fd is also part of the system process, but
# is not available on all OSes and requires root to read for all processes.
# num_threads may not be readable for some privileged process on Windows,
# cgroup is only available on linux.
SYSTEM_PROCESS_FIELDS = ["cpu", "memory", "state", "num_threads"]
SYSTEM_PROCESS_FIELDS = ["cpu", "memory", "state"]


class Test(metricbeat.BaseTest):
Expand Down Expand Up @@ -420,6 +421,9 @@ def test_process(self):
found_cmdline = False
for evt in output:
process = evt["system"]["process"]
# Not all process will have 'cmdline' due to permission issues,
# especially on Windows. Therefore we ensure at least some of
# them will have it.
found_cmdline |= "cmdline" in process

# Remove 'env' prior to checking documented fields because its keys are dynamic.
Expand All @@ -430,11 +434,13 @@ def test_process(self):
process.pop("cgroup", None)
process.pop("fd", None)
process.pop("cmdline", None)
process.pop("num_threads", None)

self.assertCountEqual(SYSTEM_PROCESS_FIELDS, process.keys())

self.assertTrue(
found_cmdline, "cmdline not found in any process events")
# After iterating over all process, make sure at least one of them had
# the 'cmdline' set.
self.assertTrue(
found_cmdline, "cmdline not found in any process events")
Comment on lines +440 to +443
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fearful-symmetry, could you do a sanity check on this last change? I believe it is ok to change, but I'd like a second opinion before merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the assertTrue() line was just moved out of the loop block to check for the cmdline globally? That seems fine, but wouldn't it require the last event to set found_cmdline |= "cmdline" in process in order to pass? Maybe a counter that needs to be > 1 would work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the assertTrue() line was just moved out of the loop block to check for the cmdline globally?

Yes, that was my intent because it already does an OR operation on itself. Being honest I was very confused by the assertTrue() being inside the loop and having found_cmdline |= "cmdline" in process.

beats/metricbeat/module/system/test_system.py

Lines 421 to 427 in bab5bb4

found_cmdline = False
for evt in output:
process = evt["system"]["process"]
# Not all process will have 'cmdline' due to permission issues,
# especially on Windows. Therefore we ensure at least some of
# them will have it.
found_cmdline |= "cmdline" in process

but wouldn't it require the last event to set found_cmdline |= "cmdline" in process in order to pass?

That's an OR operator, so as long as "cmdline" in process returns True at least once, then found_cmdline will always be True.


@unittest.skipUnless(re.match("(?i)linux|darwin|freebsd", sys.platform), "os")
def test_process_unix(self):
Expand Down Expand Up @@ -486,6 +492,7 @@ def test_process_unix(self):
process.pop("cgroup", None)
process.pop("cmdline", None)
process.pop("fd", None)
process.pop("num_threads", None)

self.assertCountEqual(SYSTEM_PROCESS_FIELDS, process.keys())

Expand Down
Loading