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

[APM] Make optimised loading of service inventory opt-in #128471

Merged
merged 10 commits into from
Mar 28, 2022
Merged

[APM] Make optimised loading of service inventory opt-in #128471

merged 10 commits into from
Mar 28, 2022

Conversation

dgieselaar
Copy link
Member

Closes #127294.

Adds an Kibana Advanced Setting for the pre-loading of services in the service inventory, and makes it opt-in. If enabled, we'll sort by health status first and service name second, and if disabled, which is the default after this PR, we'll sort by health status first, and throughput second.

@dgieselaar dgieselaar added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes v8.2.0 labels Mar 24, 2022
@dgieselaar dgieselaar requested a review from a team as a code owner March 24, 2022 11:14
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@dgieselaar
Copy link
Member Author

@elasticmachine merge upstream

MiriamAparicio
MiriamAparicio approved these changes Mar 28, 2022
View reviewed changes
Copy link
Contributor

@MiriamAparicio MiriamAparicio left a comment

Choose a reason for hiding this comment

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

LGTM 🌟

gbamparop
gbamparop reviewed Mar 28, 2022
View reviewed changes
x-pack/plugins/apm/public/components/app/service_inventory/service_list/order_service_items.ts Outdated
item.environments?.join(', ').toLowerCase() ?? '',
[ServiceInventoryFieldName.TransactionType]: (item) =>
item.transactionType ?? '',
[ServiceInventoryFieldName.Latency]: (item) => item.latency ?? 0,
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 no longer relevant and the default is now 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point, it's not relevant for throughput I think (because it will always be reported as 0), but for latency it will be.

Copy link
Member Author

Choose a reason for hiding this comment

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

and definitely failure rate

gbamparop
gbamparop approved these changes Mar 28, 2022
View reviewed changes
Copy link
Contributor

@gbamparop gbamparop left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for adding this!

@dgieselaar dgieselaar enabled auto-merge (squash) March 28, 2022 14:36
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1185 1187 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
observability 365 366 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 2.9MB 2.9MB +1.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observability 91.1KB 91.2KB +158.0B
Unknown metric groups

API count

id before after diff
observability 368 369 +1

History

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

@dgieselaar dgieselaar merged commit 56c65bc into elastic:main Mar 28, 2022
@dgieselaar dgieselaar deleted the prefer-throughput-as-tiebreaker branch March 28, 2022 16:04
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 30, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 128471 or prevent reminders by adding the backport:skip label.

@spalger spalger added the backport:skip This commit does not require backporting label Mar 30, 2022
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbamparop gbamparop gbamparop approved these changes

@MiriamAparicio MiriamAparicio MiriamAparicio approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[APM] Set default preferred sorting for service inventory in advanced settings
7 participants
@dgieselaar @elasticmachine @kibana-ci @kibanamachine @gbamparop @MiriamAparicio @spalger