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

[Exploratory view] Core web vitals #100320

Merged
merged 32 commits into from
Jun 2, 2021
Merged

[Exploratory view] Core web vitals #100320

merged 32 commits into from
Jun 2, 2021

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented May 19, 2021 •
edited
Loading

Summary

fix #100318

image

Added core web vitals reporting in exploratory view

Users will be able to do breakdown/filtering on each core web vital

design will probably evolve alongwith rest of the exploratory view, but this is necessary to add basic reporting into the view for web core vitals.

  • Testing for expected behavior is in place
    • User acceptance testing has been carried out to ensure the feature functions as expected within the context of how it will be used
    • Any special/edge cases that need to be manually tested must be documented

@thomasneirynck thomasneirynck added the Team:APM All issues that need APM UI Team support label May 19, 2021
@kibanamachine
Copy link
Contributor

kibanamachine commented May 25, 2021 •
edited
Loading

💔 Build Failed

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observability 280 281 +1

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
lens 154 158 +4

Async chunks

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

id before after diff
observability 458.0KB 462.0KB +4.1KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
lens 17 16 -1
Unknown metric groups

API count

id before after diff
lens 165 169 +4

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 22 20 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 261 149 -112
lens 67 45 -22
licensing 18 15 -3
lists 239 236 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
securitySolution 386 342 -44
stackAlerts 101 95 -6
total -342

History

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

@shahzad31 shahzad31 marked this pull request as ready for review May 25, 2021 14:53
@shahzad31 shahzad31 requested a review from a team May 25, 2021 14:53
@elasticmachine
Copy link
Contributor

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

flash1293
flash1293 approved these changes May 25, 2021
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Kibana app changes LGTM, code review only

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

dominiqueclarke
dominiqueclarke reviewed May 28, 2021
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Since this is a feature, can we use our PR template?

...bservability/public/components/shared/exploratory_view/configurations/constants/constants.ts
@@ -105,4 +107,9 @@ export const ReportToDataTypeMap: Record<ReportViewTypeId, AppDataType> = {
mem: 'infra_metrics',
logs: 'infra_logs',
cpu: 'infra_metrics',
cwv: 'ux',
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you include this next to the other ux bits, just for easy referencing in the future.

dominiqueclarke
dominiqueclarke reviewed May 28, 2021
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great! I really love how composable you've made this architecture. A few things I noticed about the UX:

Long y axis column names may need to be truncated, as they take up a large portion of the chart space, especially in mobile view

Screen Shot 2021-05-28 at 8 57 33 AM

Screen Shot 2021-05-28 at 8 56 47 AM

When first integrating with the chart, it's not immediately clear which metric I'm looking at. As you can see, the metric dropdown starts undefined, but under the hood we're defining a default. The user in this case doesn't know what they're looking at. The default metric should be shown in the dropdown, but I also feel like perhaps we could update the title with the metric or some combination of the applied settings.
Screen Shot 2021-05-28 at 8 58 29 AM

@shahzad31
Copy link
Contributor Author

@dominiqueclarke fixed the issue with unselected metric, regarding the smart title, i think we do have some plans to generate smart title based on selected report definition, but i think it will be done in more extensive way in a separate PR.

Also for the long y axis labels, i have created an issues in lens recently #100231

@shahzad31 shahzad31 requested a review from dominiqueclarke May 28, 2021 17:03
@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

dominiqueclarke
dominiqueclarke approved these changes Jun 1, 2021
View reviewed changes
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observability 288 289 +1

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
lens 156 160 +4

Async chunks

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

id before after diff
observability 461.2KB 465.4KB +4.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
lens 17 16 -1
Unknown metric groups

API count

id before after diff
lens 167 171 +4

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 22 20 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 261 149 -112
lens 67 45 -22
licensing 18 15 -3
lists 239 236 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
securitySolution 390 346 -44
stackAlerts 101 95 -6
total -342

History

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

cc @shahzad31

@shahzad31 shahzad31 merged commit b8f6bf5 into elastic:master Jun 2, 2021
@shahzad31 shahzad31 deleted the core-web-vitals branch June 2, 2021 10:41
@shahzad31 shahzad31 added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 2, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 2, 2021
@shahzad31 @kibanamachine
[Exploratory view] Core web vitals (elastic#100320)
98471ee 
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Jun 2, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 2, 2021
@jloleysens
Merge branch 'reporting/new-png-pdf-report-type' of github.com:jloley…
f34d980 
…sens/kibana into reporting/new-png-pdf-report-type

* 'reporting/new-png-pdf-report-type' of github.com:jloleysens/kibana: (46 commits)
  [Security Solution] Add Ransomware canary advanced policy option (elastic#101068)
  [Exploratory view] Core web vitals (elastic#100320)
  [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034)
  [Lens] Use a setter function for the dimension panel (elastic#101123)
  [Index Patterns] Fix return saved index pattern object (elastic#101051)
  [CI] For PRs, build TS refs before public api docs check (elastic#100791)
  [Maps] fix line and polygon label regression (elastic#101085)
  Migrate CCR to new ES JS client. (elastic#100131)
  [Canvas] Switch Canvas to use React Router (elastic#100579)
  [Expressions] Use table column ID instead of name when set (elastic#99724)
  [DOCS] Updates docs landing page (elastic#100749)
  [DOCS] Corrects typo in step 3 (elastic#101079)
  [DOCS] Updates runtime example in Discover (elastic#100926)
  Migrate kibana.autocomplete config to data plugin (elastic#100586)
  [Uptime] New width/delay definition for waterfall sidebar item tooltip (elastic#100147)
  [FTR] Use importExport for saved_object/basic archive (elastic#100244)
  [Fleet] Better input for multi text input in agent policy builder (elastic#101020)
  [CI] Buildkite support with Baseline pipeline (elastic#100492)
  [Reporting/Telemetry] Do not send telemetry if we are in screenshot mode (elastic#100388)
  Create API keys with metadata (elastic#100682)
  ...
kibanamachine added a commit that referenced this pull request Jun 2, 2021
@kibanamachine @shahzad31
[Exploratory view] Core web vitals (#100320) (#101144)
54b8d28 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Shahzad <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 2, 2021
@gmmorris
Merge branch 'master' into task-manager/capacity-estimation
2b9bd67 
* master: (68 commits)
  Unskip advanced settings a11y test (elastic#100558)
  [App Search] Crawler Landing Page (elastic#100822)
  [DOCS] Clarify when to use kbn clean (elastic#101155)
  change label behavior (elastic#100991)
  skip flaky suite (elastic#101126)
  Fix cases plugin ownership (elastic#101073)
  [Home] Adding file upload to add data page (elastic#100863)
  [ML] Functional tests - reenable categorization tests (elastic#101137)
  [DOCS] Adds server.uuid to settings docs (elastic#101121)
  Fix newsfeed unread notifications always on when reloading Kibana (elastic#100357)
  [Lens] Time shift metrics (elastic#98781)
  [Deprecations service] make `correctiveActions.manualSteps` required (elastic#100997)
  Add "Risk Matrix" section to the PR template (elastic#100649)
  [Maps] spatially filter by all geo fields (elastic#100735)
  [Security Solution] Add Ransomware canary advanced policy option (elastic#101068)
  [Exploratory view] Core web vitals (elastic#100320)
  [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034)
  [Lens] Use a setter function for the dimension panel (elastic#101123)
  [Index Patterns] Fix return saved index pattern object (elastic#101051)
  [CI] For PRs, build TS refs before public api docs check (elastic#100791)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominiqueclarke dominiqueclarke dominiqueclarke approved these changes

@flash1293 flash1293 flash1293 approved these changes

@justinkambic justinkambic Awaiting requested review from justinkambic

Assignees

@shahzad31 shahzad31

Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Exploratory View] Add web core vital reporting
6 participants
@shahzad31 @kibanamachine @elasticmachine @flash1293 @dominiqueclarke @thomasneirynck