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

Perfmon metricset improvements #26886

Merged
merged 15 commits into from
Aug 5, 2021
Merged

Perfmon metricset improvements #26886

merged 15 commits into from
Aug 5, 2021

Conversation

narph
Copy link
Contributor

@narph narph commented Jul 14, 2021

What does this PR do?

  • adds optional counter refresh perfmon.refresh_wildcard_counters, default is disabled now
  • implements PdhCollectQueryDataEx instead of waiting for at least 1 sec and calling PdhCollectQueryData again. With PdhCollectQueryDataEx we can retrieve the values after they are available.
  • PdhExpandWildCardPath not expanding the wildcard queries from time to time, although the call should be executed twice there was necessary a wait time between the calls, this will block the rest of the calls.
    Instead, a check is done if the query has been expanded, if not, the api is called again.

Why is it important?

  • inconsistency in data retrieval without any exceptions

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.

Event collection looks more consistent, also not a rise in CPU usage or thread/handle count.
image

Related issues

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 14, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 14, 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-05T08:30:41.298+0000

  • Duration: 80 min 6 sec

  • Commit: a9ef8a8

Test stats 🧪

Test Results
Failed 0
Passed 9037
Skipped 2409
Total 11446

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 9037
Skipped 2409
Total 11446

@narph narph self-assigned this Jul 14, 2021
@narph narph added the Team:Platforms Label for the Integrations - Platforms team label Jul 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Platforms)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 14, 2021
@narph narph requested a review from exekias July 14, 2021 14:19
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! left a few comments & questions

metricbeat/helper/windows/pdh/pdh_query_windows.go Outdated Show resolved Hide resolved
metricbeat/module/windows/perfmon/reader.go Outdated Show resolved Hide resolved
metricbeat/module/windows/perfmon/reader.go Outdated Show resolved Hide resolved
metricbeat/module/windows/perfmon/reader.go Outdated Show resolved Hide resolved
@mergify
Copy link
Contributor

mergify bot commented Jul 20, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b perfmon-update upstream/perfmon-update
git merge upstream/master
git push upstream perfmon-update

@narph narph requested a review from exekias July 26, 2021 12:48
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@@ -218,6 +234,7 @@ func (q *Query) ExpandWildCardPath(wildCardPath string) ([]string, error) {
if err == PDH_MORE_DATA {
expdPaths, err = PdhExpandCounterPath(utfPath)
}
expdPaths, err = PdhExpandCounterPath(utfPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this intended? IT would look like you would call PdhExpandCounterPath many times here (both in case of error and no error)

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 was not intended, just for testing purposes, refactored the func

@mergify
Copy link
Contributor

mergify bot commented Jul 27, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b perfmon-update upstream/perfmon-update
git merge upstream/master
git push upstream perfmon-update

@mergify
Copy link
Contributor

mergify bot commented Aug 3, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b perfmon-update upstream/perfmon-update
git merge upstream/master
git push upstream perfmon-update

Comment on lines 194 to 205
- Use PROGRAMDATA environment variable instead of C:\ProgramData for windows install service {pull}22874[22874]
- Fix reporting of cgroup metrics when running under Docker {pull}22879[22879]
- Fix typo in config docs {pull}23185[23185]
- Add FAQ entry for madvdontneed variable {pull}23429[23429]
- Fix panic due to unhandled DeletedFinalStateUnknown in k8s OnDelete {pull}23419[23419]
- Fix error loop with runaway CPU use when the Kafka output encounters some connection errors {pull}23484[23484]
- Fix issue discovering docker containers and metadata after reconnections {pull}24318[24318]
- Fix ILM alias creation when write alias exists and initial index does not exist {pull}26143[26143]
- Omit full index template from errors that occur while loading the template. {pull}25743[25743]
- In the script processor, the `decode_xml` and `decode_xml_wineventlog` processors are now available as `DecodeXML` and `DecodeXMLWineventlog` respectively.
- Fix encoding errors when using the disk queue on nested data with multi-byte characters {pull}26484[26484]
- Improve `perfmon` metricset performance. {pull}26886[26886]
Copy link
Contributor

Choose a reason for hiding this comment

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

something went wrong with the last merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it, thanks

@narph narph merged commit 12c8dae into elastic:master Aug 5, 2021
@narph narph deleted the perfmon-update branch August 5, 2021 09:58
@narph narph added the v7.15.0 label Aug 5, 2021
narph added a commit to narph/beats that referenced this pull request Aug 5, 2021
* work on perfmon improvements

* changelog

* rever changes

* complete checks

* review action

* fmt

* clean up

* docs

* fix tests

* update

* fix tests

* fix changelog

(cherry picked from commit 12c8dae)
narph added a commit that referenced this pull request Aug 10, 2021
* Perfmon metricset improvements (#26886)

* work on perfmon improvements

* changelog

* rever changes

* complete checks

* review action

* fmt

* clean up

* docs

* fix tests

* update

* fix tests

* fix changelog

(cherry picked from commit 12c8dae)

* fix changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Platforms Label for the Integrations - Platforms team v7.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows - perfmon - some performance counters do not retrieve any values if there are too many pdh calls
3 participants