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

[Metricbeat] Add Linux pressure metricset #27355

Merged
merged 16 commits into from
Aug 19, 2021

Conversation

b-deam
Copy link
Member

@b-deam b-deam commented Aug 13, 2021

What does this PR do?

This PR aims to introduce an additional metricset that collects Pressure Stall Information (PSI) for CPU, Memory, and IO. These are available from kernel >=4.20.

These metrics are incredibly useful for debugging system performance, and more can be read about these metrics below:

Why is it important?

These are valuable OS metrics that are useful for both debugging and monitoring system performance.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • Tests need work.

How to test this PR locally

  • Pull down, run on a Linux host (ubuntu2004 works from Vagrantfile) that:
    • Is on kernel >=4.20
    • /proc/pressure is available on

Related issues

@b-deam b-deam added enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team :integrations labels Aug 13, 2021
@b-deam b-deam self-assigned this Aug 13, 2021
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Aug 13, 2021
@b-deam b-deam changed the title Linux pressure metricset [Metricbeat] Add Linux pressure metricset Aug 13, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 13, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-08-19T03:21:52.887+0000

  • Duration: 81 min 24 sec

  • Commit: c4cbb2a

Test stats 🧪

Test Results
Failed 0
Passed 9086
Skipped 2418
Total 11504

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 9086
Skipped 2418
Total 11504

Copy link
Contributor

@fearful-symmetry fearful-symmetry left a comment

Choose a reason for hiding this comment

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

This is looking good! Just a few small things, mostly beats cruft, we need to change. If you want, I can take the PR over from here; I don't want to make you feel like you're obligated to spend your time coding to the various metricbeat idioms.

type: group
description: Linux pressure stall information
fields:
- name: cpu.some.10
Copy link
Contributor

Choose a reason for hiding this comment

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

In system/process cgroup data, we're reporting these as 10.pct and so on. We'll also need to add format: percent

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. I tried to follow similar conventions for total, in that it becomes total.time.us - does this make sense here?

metricbeat/module/linux/pressure/pressure.go Outdated Show resolved Hide resolved
metricbeat/module/linux/pressure/pressure.go Outdated Show resolved Hide resolved
@fearful-symmetry fearful-symmetry self-assigned this Aug 13, 2021
@b-deam
Copy link
Member Author

b-deam commented Aug 16, 2021

If you want, I can take the PR over from here; I don't want to make you feel like you're obligated to spend your time coding to the various metricbeat idioms.

Not a problem - I think I've addressed the comments so far but feel free to make any small changes if I've otherwise messed things up there.

@fearful-symmetry
Copy link
Contributor

Real quick, you'll need to run mage fmt update in the main metricbeat directory.

@b-deam
Copy link
Member Author

b-deam commented Aug 16, 2021

Real quick, you'll need to run mage fmt update in the main metricbeat directory.

Hmm.. I tried this but I don't see any changes. Any ideas?

bradleydeam in ~/go/src/github.com/b-deam/beats/metricbeat on branch linux-pressure-metricset $ make update  
mage update
Generated fields.yml for metricbeat to /Users/bradleydeam/go/src/github.com/b-deam/beats/metricbeat/fields.yml
>> Building metricbeat.yml for linux/amd64
>> Building metricbeat.reference.yml for linux/amd64
>> Building metricbeat.docker.yml for linux/amd64
Generated fields.yml for metricbeat to /Users/bradleydeam/go/src/github.com/b-deam/beats/metricbeat/build/fields/fields.all.yml
exec: go list -m
exec: go list -m              

bradleydeam in ~/go/src/github.com/b-deam/beats/metricbeat on branch linux-pressure-metricset $ mage fmt update
>> fmt - go-licenser: Adding missing headers
>> fmt - goimports: Formatting Go code
>> fmt - autopep8: Formatting Python code
Generated fields.yml for metricbeat to /Users/bradleydeam/go/src/github.com/b-deam/beats/metricbeat/fields.yml
>> Building metricbeat.yml for linux/amd64
>> Building metricbeat.reference.yml for linux/amd64
>> Building metricbeat.docker.yml for linux/amd64
Generated fields.yml for metricbeat to /Users/bradleydeam/go/src/github.com/b-deam/beats/metricbeat/build/fields/fields.all.yml
exec: go list -m
exec: go list -m

bradleydeam in ~/go/src/github.com/b-deam/beats/metricbeat on branch linux-pressure-metricset $ git status
On branch linux-pressure-metricset
Your branch is up to date with 'origin/linux-pressure-metricset'.

nothing to commit, working tree clean

@fearful-symmetry
Copy link
Contributor

@b-deam I think you'll need to re-merge upstream changes? A few things may have changed in the doc build.

@b-deam
Copy link
Member Author

b-deam commented Aug 18, 2021

@b-deam I think you'll need to re-merge upstream changes? A few things may have changed in the doc build.

🤦 - ofc. Done. 🤞 tests are good!

@b-deam b-deam marked this pull request as ready for review August 18, 2021 05:25
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

Copy link
Contributor

@fearful-symmetry fearful-symmetry left a comment

Choose a reason for hiding this comment

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

LGTM.

@b-deam
Copy link
Member Author

b-deam commented Aug 19, 2021

LGTM.

I re-ran a test to make sure it all looked good and noticed that I'd messed up the fields/mapping so that the metrics were actually recorded as:

linux.pressure.pressure.*

This should be fixed now..

@b-deam b-deam merged commit e49f257 into elastic:master Aug 19, 2021
@b-deam
Copy link
Member Author

b-deam commented Aug 19, 2021

Thanks for all the help @fearful-symmetry!

Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
* Adds new linux pressure metricset

* Tidy up code

* Be consistent in default config

* Run integration tests

* Make update

* make update v3

* Address comments

* Format tests

* Remove unused imports from tests

* Mage format

* Update changelog

* simplify loop

* Fix fields/mapping
@andresrc andresrc added backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify labels Nov 18, 2021
mergify bot pushed a commit that referenced this pull request Nov 18, 2021
* Adds new linux pressure metricset

* Tidy up code

* Be consistent in default config

* Run integration tests

* Make update

* make update v3

* Address comments

* Format tests

* Remove unused imports from tests

* Mage format

* Update changelog

* simplify loop

* Fix fields/mapping

(cherry picked from commit e49f257)

# Conflicts:
#	metricbeat/docs/modules_list.asciidoc
#	metricbeat/module/linux/fields.go
#	metricbeat/module/linux/pressure/pressure.go
mergify bot pushed a commit that referenced this pull request Nov 18, 2021
* Adds new linux pressure metricset

* Tidy up code

* Be consistent in default config

* Run integration tests

* Make update

* make update v3

* Address comments

* Format tests

* Remove unused imports from tests

* Mage format

* Update changelog

* simplify loop

* Fix fields/mapping

(cherry picked from commit e49f257)

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/docs/modules/linux.asciidoc
#	metricbeat/docs/modules/linux/pressure.asciidoc
#	metricbeat/docs/modules_list.asciidoc
#	metricbeat/include/list_common.go
#	metricbeat/metricbeat.reference.yml
#	metricbeat/module/linux/_meta/config.yml
#	metricbeat/module/linux/fields.go
#	metricbeat/module/linux/pressure/pressure.go
#	metricbeat/module/linux/pressure/pressure_test.go
#	metricbeat/modules.d/linux.yml.disabled
#	x-pack/metricbeat/metricbeat.reference.yml
fearful-symmetry added a commit that referenced this pull request Nov 19, 2021
)

* [Metricbeat] Add Linux pressure metricset (#27355)

* Adds new linux pressure metricset

* Tidy up code

* Be consistent in default config

* Run integration tests

* Make update

* make update v3

* Address comments

* Format tests

* Remove unused imports from tests

* Mage format

* Update changelog

* simplify loop

* Fix fields/mapping

(cherry picked from commit e49f257)

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/docs/modules/linux.asciidoc
#	metricbeat/docs/modules/linux/pressure.asciidoc
#	metricbeat/docs/modules_list.asciidoc
#	metricbeat/include/list_common.go
#	metricbeat/metricbeat.reference.yml
#	metricbeat/module/linux/_meta/config.yml
#	metricbeat/module/linux/fields.go
#	metricbeat/module/linux/pressure/pressure.go
#	metricbeat/module/linux/pressure/pressure_test.go
#	metricbeat/modules.d/linux.yml.disabled
#	x-pack/metricbeat/metricbeat.reference.yml

* fix merge

Co-authored-by: Brad Deam <54515790+b-deam@users.noreply.github.com>
Co-authored-by: Alex Kristiansen <alex.kristiansen@elastic.co>
fearful-symmetry pushed a commit that referenced this pull request Nov 19, 2021
…9036)

* [Metricbeat] Add Linux pressure metricset (#27355)

* Adds new linux pressure metricset

* Tidy up code

* Be consistent in default config

* Run integration tests

* Make update

* make update v3

* Address comments

* Format tests

* Remove unused imports from tests

* Mage format

* Update changelog

* simplify loop

* Fix fields/mapping

(cherry picked from commit e49f257)

# Conflicts:
#	metricbeat/docs/modules_list.asciidoc
#	metricbeat/module/linux/fields.go
#	metricbeat/module/linux/pressure/pressure.go

* Fix conflicts

* Formatting

Co-authored-by: Brad Deam <54515790+b-deam@users.noreply.github.com>
Co-authored-by: Brad Deam <bradley.deam@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify enhancement :integrations Metricbeat Metricbeat Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metricbeat][System module] psi: pressure stall information
4 participants