Skip to content

Commit 67a39a9

Browse files
eokoneyoelasticmachine
eokoneyo
and
elasticmachine
committed
[Reporting] Improvements to reporting diagnostics (#191790)
## 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 <elasticmachine@users.noreply.github.com> (cherry picked from commit 20dcf5f)
1 parent a0e118d commit 67a39a9

4 files changed

Lines changed: 160 additions & 4 deletions

File tree

‎x-pack/plugins/reporting/public/management/components/report_diagnostic.test.tsx‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import React, { type ComponentProps } from 'react';
9+
import userEvent from '@testing-library/user-event';
10+
import { render, screen, waitFor } from '@testing-library/react';
11+
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
12+
import { ReportDiagnostic } from './report_diagnostic';
13+
14+
const mockedApiClient: jest.Mocked<
15+
Pick<ComponentProps<typeof ReportDiagnostic>['apiClient'], 'verifyBrowser'>
16+
> = {
17+
verifyBrowser: jest.fn(),
18+
};
19+
20+
const defaultProps: Pick<ComponentProps<typeof ReportDiagnostic>, 'apiClient'> = {
21+
// @ts-expect-error we don't need to provide the full apiClient for the test
22+
apiClient: mockedApiClient,
23+
};
24+
25+
const renderComponent = (props: Pick<ComponentProps<typeof ReportDiagnostic>, 'clientConfig'>) => {
26+
render(
27+
<IntlProvider locale="en">
28+
<ReportDiagnostic {...defaultProps} {...props} />
29+
</IntlProvider>
30+
);
31+
};
32+
33+
describe('ReportDiagnostic', () => {
34+
afterEach(() => {
35+
jest.clearAllMocks();
36+
});
37+
38+
it("does not render the component, if image exports aren't supported", () => {
39+
renderComponent({
40+
clientConfig: {
41+
export_types: { pdf: { enabled: false }, png: { enabled: false } },
42+
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
43+
});
44+
45+
expect(screen.queryByTestId('screenshotDiagnosticLink')).not.toBeInTheDocument();
46+
});
47+
48+
it('renders the component if image exports are supported', () => {
49+
renderComponent({
50+
clientConfig: {
51+
export_types: {
52+
png: { enabled: true },
53+
pdf: { enabled: true },
54+
},
55+
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
56+
});
57+
58+
expect(screen.getByTestId('screenshotDiagnosticLink')).toBeInTheDocument();
59+
});
60+
61+
it('renders a callout with a warning if a problem is detected during diagnosis', async () => {
62+
const user = userEvent.setup();
63+
64+
mockedApiClient.verifyBrowser.mockResolvedValue({
65+
success: false,
66+
help: ['help'],
67+
logs: 'logs',
68+
});
69+
70+
renderComponent({
71+
clientConfig: {
72+
export_types: {
73+
png: { enabled: true },
74+
pdf: { enabled: true },
75+
},
76+
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
77+
});
78+
79+
await user.click(screen.getByTestId('screenshotDiagnosticLink'));
80+
81+
await waitFor(() => expect(screen.getByTestId('reportDiagnosisFlyout')).toBeInTheDocument());
82+
83+
user.click(screen.getByTestId('reportingDiagnosticInitiationButton'));
84+
85+
await waitFor(() =>
86+
expect(screen.getByTestId('reportingDiagnosticFailureCallout')).toBeInTheDocument()
87+
);
88+
});
89+
90+
it('renders a success callout if no problem is detected during diagnosis', async () => {
91+
const user = userEvent.setup();
92+
93+
mockedApiClient.verifyBrowser.mockResolvedValue({
94+
success: true,
95+
help: [],
96+
logs: 'logs',
97+
});
98+
99+
renderComponent({
100+
clientConfig: {
101+
export_types: {
102+
png: { enabled: true },
103+
pdf: { enabled: true },
104+
},
105+
} as unknown as ComponentProps<typeof ReportDiagnostic>['clientConfig'],
106+
});
107+
108+
await user.click(screen.getByTestId('screenshotDiagnosticLink'));
109+
110+
await waitFor(() => expect(screen.getByTestId('reportDiagnosisFlyout')).toBeInTheDocument());
111+
112+
user.click(screen.getByTestId('reportingDiagnosticInitiationButton'));
113+
114+
await waitFor(() =>
115+
expect(screen.getByTestId('reportingDiagnosticSuccessCallout')).toBeInTheDocument()
116+
);
117+
});
118+
});

‎x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,17 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => {
100100
if (state.success && chromeStatus === 'complete') {
101101
outcomeCallout = (
102102
<EuiCallOut
103-
id="xpack.reporting.listing.diagnosticSuccessMessage"
103+
data-test-subj="reportingDiagnosticSuccessCallout"
104104
color="success"
105105
title={i18n.translate('xpack.reporting.listing.diagnosticSuccessMessage', {
106106
defaultMessage: 'Everything looks good for screenshot reports to function.',
107107
})}
108108
/>
109109
);
110-
} else if (!state.success && chromeStatus === 'complete') {
110+
} else if (!state.success && chromeStatus === 'danger') {
111111
outcomeCallout = (
112112
<EuiCallOut
113-
id="xpack.reporting.listing.diagnosticFailureTitle"
113+
data-test-subj="reportingDiagnosticFailureCallout"
114114
iconType="warning"
115115
color="danger"
116116
title={i18n.translate('xpack.reporting.listing.diagnosticFailureTitle', {
@@ -121,7 +121,12 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => {
121121
}
122122

123123
flyout = (
124-
<EuiFlyout onClose={closeFlyout} aria-labelledby="reportingHelperTitle" size="m">
124+
<EuiFlyout
125+
onClose={closeFlyout}
126+
data-test-subj="reportDiagnosisFlyout"
127+
aria-labelledby="reportingHelperTitle"
128+
size="m"
129+
>
125130
<EuiFlyoutHeader hasBorder>
126131
<EuiTitle size="m">
127132
<h2>
@@ -161,6 +166,7 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => {
161166
onClick={apiWrapper(() => apiClient.verifyBrowser(), statuses.chromeStatus)}
162167
isLoading={isBusy && chromeStatus === 'incomplete'}
163168
iconType={chromeStatus === 'complete' ? 'check' : undefined}
169+
data-test-subj="reportingDiagnosticInitiationButton"
164170
>
165171
<FormattedMessage
166172
id="xpack.reporting.listing.diagnosticBrowserButton"

‎x-pack/plugins/reporting/server/routes/internal/diagnostic/browser.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ const logsToHelpMapFactory = (docLinks: DocLinksServiceSetup) => ({
3434
defaultMessage: `Unable to use Chromium sandbox. This can be disabled at your own risk with 'xpack.screenshotting.browser.chromium.disableSandbox'. Please see {url}`,
3535
values: { url: docLinks.links.reporting.browserSandboxDependencies },
3636
}),
37+
38+
'Fontconfig error: Cannot load default config file': i18n.translate(
39+
'xpack.reporting.diagnostic.fontconfigError',
40+
{
41+
defaultMessage: `The browser couldn't start properly due to missing system font dependencies. Please see {url}`,
42+
values: { url: docLinks.links.reporting.browserSystemDependencies },
43+
}
44+
),
3745
});
3846

3947
const path = INTERNAL_ROUTES.DIAGNOSE.BROWSER;

‎x-pack/plugins/reporting/server/routes/internal/diagnostic/integration_tests/browser.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,30 @@ describe(`GET ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => {
120120
});
121121
});
122122

123+
it('returns a response including log received from the browser + helpful link on font config error', async () => {
124+
const fontErrorLog = `Fontconfig error: Cannot load default config file: No such file: (null)`;
125+
126+
registerDiagnoseBrowser(core, mockLogger);
127+
128+
await server.start();
129+
screenshotting.diagnose.mockReturnValue(Rx.of(fontErrorLog));
130+
131+
return supertest(httpSetup.server.listener)
132+
.get(INTERNAL_ROUTES.DIAGNOSE.BROWSER)
133+
.expect(200)
134+
.then(({ body }) => {
135+
expect(body).toMatchInlineSnapshot(`
136+
Object {
137+
"help": Array [
138+
"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",
139+
],
140+
"logs": "${fontErrorLog}",
141+
"success": false,
142+
}
143+
`);
144+
});
145+
});
146+
123147
it('logs a message when the browser starts, but then has problems later', async () => {
124148
registerDiagnoseBrowser(core, mockLogger);
125149

0 commit comments

Comments
 (0)