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

Reporting/diagnostics #74314

Merged
merged 41 commits into from
Sep 9, 2020
Merged

Reporting/diagnostics #74314

merged 41 commits into from
Sep 9, 2020

Conversation

joelgriffith
Copy link
Contributor

@joelgriffith joelgriffith commented Aug 4, 2020

Summary

Checks successfully:

reporting-check

Checks fail:

reporting-fail

Adds a new diagnostic tool to reporting. Currently, this tool checks:

  • If reporting configuration isn't out of order (no size mismatches between kibana and elasticsearch).
  • If chromium can startup, and the logs don't contain known defects.
  • If we can screenshot Kibana properly.

Checklist

Delete any items that are not applicable to this PR.

@joelgriffith joelgriffith added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:enhancement v7.10.0 v8.0.0 labels Aug 4, 2020
@tsullivan
Copy link
Member

Add /diagnose/screenshot API

For an API to test screenshot, I recommend importing generate_png in the route handler and using generatePngObservable.

@joelgriffith
Copy link
Contributor Author

Screenshot tests (the hardest) are done, moving onto the others + fixing i18n issues.

@joelgriffith
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

Need a "no usable sandbox" message

@tsullivan
Copy link
Member

I tested out this branch with:

  • building the Kibana linux package: yarn build --no-oss --skip-os-packages
  • copy the build to a vagrant instance running Centos 7.4 (trying to see a no usable sandbox message)
  • add no extra packages on top of the base OS

The diagnostic message I got simply stated Browser exited abnormally during startup, but it should have shown a message about the default font not being found.

image

[float]
[[reporting-troubleshooting-diagnostics]]
=== Reporting Diagnostics
Reporting comes with a built-in utility to try to automatically find common issues. When Kibana is running, navigate to the Report Listing page, and click the "Run reporting diagnostics..." button. This will open up a diagnostic tool that checks various parts of the Kibana deployment to come up with any relevant recommendations.
Copy link
Member

Choose a reason for hiding this comment

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

Any thoughts or feelings about having the label just be Reporting diagnostics instead of the longer phrase?


// Some chromium errors don't show up until a few seconds after startup
// hence why we wait for at least 5 seconds to let them surface
const log$ = fromEvent(rl, 'line');
Copy link
Member

@tsullivan tsullivan Sep 8, 2020

Choose a reason for hiding this comment

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

I think it would be great to tap the log lines to logger.info. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, that sounds good to me

@tsullivan
Copy link
Member

Screenshots from my testing:

No default font:
image

Couldn't detect sandbox:
image

Not sure how to test the NSS dependency

@joelgriffith
Copy link
Contributor Author

Screen Shot 2020-09-09 at 11 25 26 AM

I had to remove a "log => message" mapping since it printed duplicate messages. Afterwards, this was the message I got when libnss is missing.


// Some chromium errors don't show up until a few seconds after startup
// hence why we wait for at least 5 seconds to let them surface
const log$ = fromEvent(rl, 'line').pipe(tap((log) => logger.info(`Chromium log "${log}"`)));
Copy link
Member

@tsullivan tsullivan Sep 9, 2020

Choose a reason for hiding this comment

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

I hate to be picky, but I would like to see this teased out a bit more. What about:

Suggested change
const log$ = fromEvent(rl, 'line').pipe(tap((log) => logger.info(`Chromium log "${log}"`)));
const processLogger = logger.clone(['chromium-stderr']);
const log$ = fromEvent(rl, 'line').pipe(
tap((message: unknown) => {
if (typeof message === 'string') {
processLogger.info(message);
}
})
);

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

I'd like to check if we have consensus with a wider team on the text of the link label. Let's do that after merge though

LGTM!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
reporting 45 +1 44

page load bundle size

id value diff baseline
reporting 303.8KB +13.0KB 290.8KB

distributable file count

id value diff baseline
default 45475 +2 45473

History

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

@joelgriffith joelgriffith merged commit 9edb8d9 into elastic:master Sep 9, 2020
joelgriffith pushed a commit that referenced this pull request Sep 10, 2020
* WIP: Adding in new reporting diag tool

* WIP: chrome-binary test + log capturing/error handling

* More wip on diagnostic tool

* More work adding in diagnose routes

* Alter link in description + minor rename of chrome => browser

* Wiring UI to API + some polish on UI flow

* WIP: Add in screenshot diag route

* Adding in screenshot diag route, hooking up client to it

* Add missing lib check + memory check

* Working screenshot test + config check for RAM

* Small test helper consolidation + screenshot diag test

* Delete old i18n translations

* PR feedback, browser tests, rename, re-organize import statements and lite fixes

* Lite rename for consistency

* Remove old validate check i18n

* Add config check

* i18n all the things!

* Docs on diagnostics tool

* Fixes, better readability, spelling and more for diagnostic tool

* Translate a few error messages

* Rename of test => start_logs for clarity. Move to observables

* Gathering logs even during process exit or crash

* Adds a test case for the browser exiting during the diag check

* Tap into browser logs for checking output

* Rename asciidoc diag id

* Remove duplicate shared object message

* Add better comment as to why we merge events + wait for a period of time

* Cloning logger for mirroring browser stderr to kibana output

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 10, 2020
* master: (25 commits)
  [bugfix] Replace panel flyout opens 2 flyouts (elastic#76931)
  clean up test (elastic#76887)
  [Enterprise Search] Update shared API request handler (elastic#77112)
  [Maps] convert ESAggSource to TS (elastic#76999)
  Add plugin status API - take 2 (elastic#76732)
  Adds lens as a readable saved object for read-only dashboard users (elastic#77067)
  Skip checking for the reserved realm (elastic#76687)
  Reporting/diagnostics (elastic#74314)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  [ML] Add decision path charts to exploration results table (elastic#73561)
  Bump eventemitter3 from 4.0.0 to 4.0.7 (elastic#77016)
  [Ingest Pipelines] Add descriptions for ingest processors K-S (elastic#76981)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 10, 2020
* master: (41 commits)
  [bugfix] Replace panel flyout opens 2 flyouts (elastic#76931)
  clean up test (elastic#76887)
  [Enterprise Search] Update shared API request handler (elastic#77112)
  [Maps] convert ESAggSource to TS (elastic#76999)
  Add plugin status API - take 2 (elastic#76732)
  Adds lens as a readable saved object for read-only dashboard users (elastic#77067)
  Skip checking for the reserved realm (elastic#76687)
  Reporting/diagnostics (elastic#74314)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  [ML] Add decision path charts to exploration results table (elastic#73561)
  Bump eventemitter3 from 4.0.0 to 4.0.7 (elastic#77016)
  [Ingest Pipelines] Add descriptions for ingest processors K-S (elastic#76981)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 10, 2020
…rok/new-patterns-component-use-array

* 'master' of github.com:elastic/kibana: (39 commits)
  [APM] Always load esarchives from common (elastic#77139)
  [Ingest Manager] Handle Legacy ES client errors (elastic#76865)
  [Docs] URL Drilldown (elastic#76529)
  [bugfix] Replace panel flyout opens 2 flyouts (elastic#76931)
  clean up test (elastic#76887)
  [Enterprise Search] Update shared API request handler (elastic#77112)
  [Maps] convert ESAggSource to TS (elastic#76999)
  Add plugin status API - take 2 (elastic#76732)
  Adds lens as a readable saved object for read-only dashboard users (elastic#77067)
  Skip checking for the reserved realm (elastic#76687)
  Reporting/diagnostics (elastic#74314)
  Reporting/Test: unskip non-screenshot tests (elastic#77088)
  Move metrics to setup and add cgroup metrics (elastic#76730)
  [Enterprise Search] Add Overview landing page/plugin (elastic#76734)
  First pass. Change TS type. Update OpenAPI (elastic#76434)
  [CI] Balance xpack ci groups a bit (elastic#77068)
  [Security_solution][Detections] Refactor signal ancestry to allow multiple parents (elastic#76531)
  [Maps] convert MetricsEditor to TS (elastic#76727)
  IndexMigrator: fix non blocking migration wrapper promise rejection (elastic#77018)
  [Enterprise Search] Update config data endpoint to v2 (elastic#76970)
  ...

# Conflicts:
#	src/plugins/es_ui_shared/static/forms/hook_form_lib/components/use_array.ts
@stacey-gammon stacey-gammon added the ReleaseStatus Item of high enough importance that it should be called out in release status meetings label Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:enhancement ReleaseStatus Item of high enough importance that it should be called out in release status meetings v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants