Skip to content

Commit

Permalink
[Reporting] Improvements to reporting diagnostics (#191790)
Browse files Browse the repository at this point in the history
## Summary

Closes #186434

This PR adds a search within the output log whilst attempting to start
the browser (i.e chromium) bundled with Kibana to ascertain if it
includes a font config error log message as signal in reporting
diagnostics for potential issues with generating a report. This work is
informed from couple of support tickets with customers running the
diagnostics tool, getting a confirmation that the reporting system is
functionality whilst missing font issues was actually preventing reports
from being generated on the linux platform.

Also fixes an issue that prevented visual feedback for when an error is
detected during diagnosis.

#### Visuals
![ScreenRecording2024-09-26at15 04 19-ezgif
com-video-to-gif-converter](https://github.com/user-attachments/assets/c0320f2a-6802-435b-bae7-535f878113b3)



<!-- ### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces&mdash;unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes&mdash;Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
-->

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
eokoneyo and elasticmachine authored Oct 7, 2024
1 parent 3c1c646 commit 20dcf5f
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { type ComponentProps } from 'react';
import userEvent from '@testing-library/user-event';
import { render, screen, waitFor } from '@testing-library/react';
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
import { ReportDiagnostic } from './report_diagnostic';

const mockedApiClient: jest.Mocked<
Pick<ComponentProps<typeof ReportDiagnostic>['apiClient'], 'verifyBrowser'>
> = {
verifyBrowser: jest.fn(),
};

const defaultProps: Pick<ComponentProps<typeof ReportDiagnostic>, 'apiClient'> = {
// @ts-expect-error we don't need to provide the full apiClient for the test
apiClient: mockedApiClient,
};

const renderComponent = (props: Pick<ComponentProps<typeof ReportDiagnostic>, 'clientConfig'>) => {
render(
<IntlProvider locale="en">
<ReportDiagnostic {...defaultProps} {...props} />
</IntlProvider>
);
};

describe('ReportDiagnostic', () => {
afterEach(() => {
jest.clearAllMocks();
});

it("does not render the component, if image exports aren't supported", () => {
renderComponent({
clientConfig: {
export_types: { pdf: { enabled: false }, png: { enabled: false } },
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
});

expect(screen.queryByTestId('screenshotDiagnosticLink')).not.toBeInTheDocument();
});

it('renders the component if image exports are supported', () => {
renderComponent({
clientConfig: {
export_types: {
png: { enabled: true },
pdf: { enabled: true },
},
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
});

expect(screen.getByTestId('screenshotDiagnosticLink')).toBeInTheDocument();
});

it('renders a callout with a warning if a problem is detected during diagnosis', async () => {
const user = userEvent.setup();

mockedApiClient.verifyBrowser.mockResolvedValue({
success: false,
help: ['help'],
logs: 'logs',
});

renderComponent({
clientConfig: {
export_types: {
png: { enabled: true },
pdf: { enabled: true },
},
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
});

await user.click(screen.getByTestId('screenshotDiagnosticLink'));

await waitFor(() => expect(screen.getByTestId('reportDiagnosisFlyout')).toBeInTheDocument());

user.click(screen.getByTestId('reportingDiagnosticInitiationButton'));

await waitFor(() =>
expect(screen.getByTestId('reportingDiagnosticFailureCallout')).toBeInTheDocument()
);
});

it('renders a success callout if no problem is detected during diagnosis', async () => {
const user = userEvent.setup();

mockedApiClient.verifyBrowser.mockResolvedValue({
success: true,
help: [],
logs: 'logs',
});

renderComponent({
clientConfig: {
export_types: {
png: { enabled: true },
pdf: { enabled: true },
},
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
});

await user.click(screen.getByTestId('screenshotDiagnosticLink'));

await waitFor(() => expect(screen.getByTestId('reportDiagnosisFlyout')).toBeInTheDocument());

user.click(screen.getByTestId('reportingDiagnosticInitiationButton'));

await waitFor(() =>
expect(screen.getByTestId('reportingDiagnosticSuccessCallout')).toBeInTheDocument()
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,17 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => {
if (state.success && chromeStatus === 'complete') {
outcomeCallout = (
<EuiCallOut
id="xpack.reporting.listing.diagnosticSuccessMessage"
data-test-subj="reportingDiagnosticSuccessCallout"
color="success"
title={i18n.translate('xpack.reporting.listing.diagnosticSuccessMessage', {
defaultMessage: 'Everything looks good for screenshot reports to function.',
})}
/>
);
} else if (!state.success && chromeStatus === 'complete') {
} else if (!state.success && chromeStatus === 'danger') {
outcomeCallout = (
<EuiCallOut
id="xpack.reporting.listing.diagnosticFailureTitle"
data-test-subj="reportingDiagnosticFailureCallout"
iconType="warning"
color="danger"
title={i18n.translate('xpack.reporting.listing.diagnosticFailureTitle', {
Expand All @@ -121,7 +121,12 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => {
}

flyout = (
<EuiFlyout onClose={closeFlyout} aria-labelledby="reportingHelperTitle" size="m">
<EuiFlyout
onClose={closeFlyout}
data-test-subj="reportDiagnosisFlyout"
aria-labelledby="reportingHelperTitle"
size="m"
>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2>
Expand Down Expand Up @@ -161,6 +166,7 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => {
onClick={apiWrapper(() => apiClient.verifyBrowser(), statuses.chromeStatus)}
isLoading={isBusy && chromeStatus === 'incomplete'}
iconType={chromeStatus === 'complete' ? 'check' : undefined}
data-test-subj="reportingDiagnosticInitiationButton"
>
<FormattedMessage
id="xpack.reporting.listing.diagnosticBrowserButton"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ const logsToHelpMapFactory = (docLinks: DocLinksServiceSetup) => ({
defaultMessage: `Unable to use Chromium sandbox. This can be disabled at your own risk with 'xpack.screenshotting.browser.chromium.disableSandbox'. Please see {url}`,
values: { url: docLinks.links.reporting.browserSandboxDependencies },
}),

'Fontconfig error: Cannot load default config file': i18n.translate(
'xpack.reporting.diagnostic.fontconfigError',
{
defaultMessage: `The browser couldn't start properly due to missing system font dependencies. Please see {url}`,
values: { url: docLinks.links.reporting.browserSystemDependencies },
}
),
});

const path = INTERNAL_ROUTES.DIAGNOSE.BROWSER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,30 @@ describe(`GET ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => {
});
});

it('returns a response including log received from the browser + helpful link on font config error', async () => {
const fontErrorLog = `Fontconfig error: Cannot load default config file: No such file: (null)`;

registerDiagnoseBrowser(core, mockLogger);

await server.start();
screenshotting.diagnose.mockReturnValue(Rx.of(fontErrorLog));

return supertest(httpSetup.server.listener)
.get(INTERNAL_ROUTES.DIAGNOSE.BROWSER)
.expect(200)
.then(({ body }) => {
expect(body).toMatchInlineSnapshot(`
Object {
"help": Array [
"The browser couldn't start properly due to missing system font dependencies. Please see https://www.elastic.co/guide/en/kibana/test-branch/secure-reporting.html#install-reporting-packages",
],
"logs": "${fontErrorLog}",
"success": false,
}
`);
});
});

it('logs a message when the browser starts, but then has problems later', async () => {
registerDiagnoseBrowser(core, mockLogger);

Expand Down

0 comments on commit 20dcf5f

Please sign in to comment.