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

Jump to bottom

[Fleet] Fix agent status count #95099

Merged
merged 2 commits into from
Mar 23, 2021
Merged

[Fleet] Fix agent status count #95099

merged 2 commits into from
Mar 23, 2021

Conversation

nchaulet
Copy link
Member

Summary

Resolve #95037
Fix a bug introduced here #94632

Changing the total of the method getAgent was incorrect and it's causing a bug where agent status in the listing status bar is not shown.

I also added a test for that endpoint /agent-status

https://user-images.githubusercontent.com/77374876/111969737-903c0900-8b20-11eb-9efa-1cd55b66cfd2.png

@nchaulet nchaulet added bug Fixes for quality problems that affect the customer experience v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Mar 22, 2021
@nchaulet nchaulet requested a review from a team as a code owner March 22, 2021 19:37
@nchaulet nchaulet self-assigned this Mar 22, 2021
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Mar 22, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 22, 2021
jen-huang
jen-huang approved these changes Mar 22, 2021
View reviewed changes
Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

🚀

x-pack/test/fleet_api_integration/apis/agents/status.ts
@@ -0,0 +1,86 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding tests!

@nchaulet nchaulet enabled auto-merge (squash) March 22, 2021 20:44
jfsiii
jfsiii reviewed Mar 23, 2021
View reviewed changes
x-pack/plugins/fleet/server/services/agents/crud.ts
@@ -138,7 +139,7 @@ export async function getAgentsByKuery(

return {
agents,
total: agents.length,
total: res.body.hits.total.value,
Copy link
Contributor

Choose a reason for hiding this comment

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

The way I interpret lines 134-138 are that if showUpgradeable is true, agents will be narrowed to only include upgradeable agents.

What would this return in that case? It appears the count will before the full/initial list, but agents would be a list of only upgradeable agents. i.e. potentially an array of 3 items but a total: 5.

It's totally possible I misunderstand what's happening here. Thanks for researching and sending a PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

showUpgradeable is a little weird here as the filtering is happening when we get the results,I am wondering if we can implement the show upgradeable as a filter in the ES query it will make things easier, otherwise I guess it's ok to have an inaccurate total for the show upgradeable filter an create an issue for that.

@jfsiii jfsiii self-requested a review March 23, 2021 14:01
jfsiii
jfsiii approved these changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

My first review should have been 🚢 , too. Thanks again for the fix & test

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @nchaulet

@nchaulet nchaulet merged commit 76b5520 into elastic:master Mar 23, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 23, 2021
@nchaulet @kibanamachine
[Fleet] Fix agent status count (elastic#95099)
fb13a05
@kibanamachine kibanamachine mentioned this pull request Mar 23, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #95182

This backport PR will be merged automatically after passing CI.

@nchaulet nchaulet deleted the fix-agent-statuses branch March 23, 2021 15:35
kibanamachine added a commit that referenced this pull request Mar 23, 2021
@kibanamachine @nchaulet
[Fleet] Fix agent status count (#95099) (#95182)
f242fb5 
Co-authored-by: Nicolas Chaulet <nicolas.chaulet@elastic.co>
@nchaulet nchaulet mentioned this pull request Mar 23, 2021
Closed
@amolnater-qasource
Copy link

Hi @EricDavisX
We have revalidated this issue on 7.13.0 Snapshot Kibana Cloud Environment and found it fixed.

Observations:

  • Updated Agent count on Agents tab, Overview tab, Agent policies tab and Integration>Details tab.

Screenshots:
KPI

Build details:

Build: 39868
Commit: 7977d493ce4fedd35b9c79435be4b6c2c6559345

Thanks
QAS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jen-huang jen-huang jen-huang approved these changes

@jfsiii jfsiii jfsiii approved these changes

Assignees

@nchaulet nchaulet

Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

KPI count not updated on "Agents" tab.
6 participants
@nchaulet @elasticmachine @kibanamachine @amolnater-qasource @jfsiii @jen-huang