[Reporting] deprecate capture.viewport setting from reporting config as unused (#114019)

* [Reporting] remove capture.viewport setting from reporting config

* update snapshots

* update snapshot

* add helpful version comment

* self-review

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Tim Sullivan 2021-10-12 12:42:38 -07:00 committed by GitHub
parent 0de7012bf2
commit 3c8662f9fa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 28 additions and 37 deletions

View file

@ -59,6 +59,11 @@ export const LAYOUT_TYPES = {
PRINT: 'print',
};
export const DEFAULT_VIEWPORT = {
width: 1950,
height: 1200,
};
// Export Type Definitions
export const CSV_REPORT_TYPE = 'CSV';
export const CSV_JOB_TYPE = 'csv_searchsource';

View file

@ -6,18 +6,17 @@
*/
import { CaptureConfig } from '../../../../server/types';
import { DEFAULT_VIEWPORT } from '../../../../common/constants';
type ViewportConfig = CaptureConfig['viewport'];
type BrowserConfig = CaptureConfig['browser']['chromium'];
interface LaunchArgs {
userDataDir: string;
viewport: ViewportConfig;
disableSandbox: BrowserConfig['disableSandbox'];
proxy: BrowserConfig['proxy'];
}
export const args = ({ userDataDir, viewport, disableSandbox, proxy: proxyConfig }: LaunchArgs) => {
export const args = ({ userDataDir, disableSandbox, proxy: proxyConfig }: LaunchArgs) => {
const flags = [
// Disable built-in Google Translate service
'--disable-translate',
@ -45,7 +44,7 @@ export const args = ({ userDataDir, viewport, disableSandbox, proxy: proxyConfig
// NOTE: setting the window size does NOT set the viewport size: viewport and window size are different.
// The viewport may later need to be resized depending on the position of the clip area.
// These numbers come from the job parameters, so this is a close guess.
`--window-size=${Math.floor(viewport.width)},${Math.floor(viewport.height)}`,
`--window-size=${Math.floor(DEFAULT_VIEWPORT.width)},${Math.floor(DEFAULT_VIEWPORT.height)}`,
// allow screenshot clip region to go outside of the viewport
`--mainFrameClipsContent=false`,
];

View file

@ -5,10 +5,10 @@
* 2.0.
*/
import apm from 'elastic-apm-node';
import { i18n } from '@kbn/i18n';
import { getDataPath } from '@kbn/utils';
import del from 'del';
import apm from 'elastic-apm-node';
import fs from 'fs';
import path from 'path';
import puppeteer from 'puppeteer';
@ -23,7 +23,7 @@ import { LevelLogger } from '../../../lib';
import { safeChildProcess } from '../../safe_child_process';
import { HeadlessChromiumDriver } from '../driver';
import { args } from './args';
import { Metrics, getMetrics } from './metrics';
import { getMetrics, Metrics } from './metrics';
// Puppeteer type definitions do not match the documentation.
// See https://pptr.dev/#?product=Puppeteer&version=v8.0.0&show=api-puppeteerlaunchoptions
@ -38,14 +38,13 @@ declare module 'puppeteer' {
}
type BrowserConfig = CaptureConfig['browser']['chromium'];
type ViewportConfig = CaptureConfig['viewport'];
export class HeadlessChromiumDriverFactory {
private binaryPath: string;
private captureConfig: CaptureConfig;
private browserConfig: BrowserConfig;
private userDataDir: string;
private getChromiumArgs: (viewport: ViewportConfig) => string[];
private getChromiumArgs: () => string[];
private core: ReportingCore;
constructor(core: ReportingCore, binaryPath: string, logger: LevelLogger) {
@ -60,10 +59,9 @@ export class HeadlessChromiumDriverFactory {
}
this.userDataDir = fs.mkdtempSync(path.join(getDataPath(), 'chromium-'));
this.getChromiumArgs = (viewport: ViewportConfig) =>
this.getChromiumArgs = () =>
args({
userDataDir: this.userDataDir,
viewport,
disableSandbox: this.browserConfig.disableSandbox,
proxy: this.browserConfig.proxy,
});
@ -75,7 +73,7 @@ export class HeadlessChromiumDriverFactory {
* Return an observable to objects which will drive screenshot capture for a page
*/
createPage(
{ viewport, browserTimezone }: { viewport: ViewportConfig; browserTimezone?: string },
{ browserTimezone }: { browserTimezone?: string },
pLogger: LevelLogger
): Rx.Observable<{ driver: HeadlessChromiumDriver; exit$: Rx.Observable<never> }> {
// FIXME: 'create' is deprecated
@ -83,7 +81,7 @@ export class HeadlessChromiumDriverFactory {
const logger = pLogger.clone(['browser-driver']);
logger.info(`Creating browser page driver`);
const chromiumArgs = this.getChromiumArgs(viewport);
const chromiumArgs = this.getChromiumArgs();
logger.debug(`Chromium launch args set to: ${chromiumArgs}`);
let browser: puppeteer.Browser;

View file

@ -74,7 +74,6 @@ export const browserStartLogs = (
const kbnArgs = args({
userDataDir,
viewport: { width: 800, height: 600 },
disableSandbox,
proxy,
});

View file

@ -20,6 +20,7 @@ export const config: PluginConfigDescriptor<ReportingConfigType> = {
unused('capture.browser.chromium.maxScreenshotDimension'), // unused since 7.8
unused('poll.jobCompletionNotifier.intervalErrorMultiplier'), // unused since 7.10
unused('poll.jobsRefresh.intervalErrorMultiplier'), // unused since 7.10
unused('capture.viewport'), // deprecated as unused since 7.16
(settings, fromPath, addDeprecation) => {
const reporting = get(settings, fromPath);
if (reporting?.index) {

View file

@ -63,10 +63,6 @@ describe('Reporting Config Schema', () => {
"renderComplete": "PT30S",
"waitForElements": "PT30S",
},
"viewport": Object {
"height": 1200,
"width": 1950,
},
"zoom": 2,
},
"csv": Object {
@ -168,10 +164,6 @@ describe('Reporting Config Schema', () => {
"renderComplete": "PT30S",
"waitForElements": "PT30S",
},
"viewport": Object {
"height": 1200,
"width": 1950,
},
"zoom": 2,
},
"csv": Object {

View file

@ -74,10 +74,6 @@ const CaptureSchema = schema.object({
}),
}),
zoom: schema.number({ defaultValue: 2 }),
viewport: schema.object({
width: schema.number({ defaultValue: 1950 }),
height: schema.number({ defaultValue: 1200 }),
}),
loadDelay: schema.oneOf([schema.number(), schema.duration()], {
defaultValue: moment.duration({ seconds: 3 }),
}),

View file

@ -67,6 +67,10 @@ describe('Create Layout', () => {
"timefilterDurationAttribute": "data-shared-timefilter-duration",
},
"useReportingBranding": true,
"viewport": Object {
"height": 1200,
"width": 1950,
},
}
`);
});

View file

@ -9,7 +9,7 @@ import path from 'path';
import { PageOrientation, PredefinedPageSize } from 'pdfmake/interfaces';
import { EvaluateFn, SerializableOrJSHandle } from 'puppeteer';
import { LevelLogger } from '../';
import { LAYOUT_TYPES } from '../../../common/constants';
import { DEFAULT_VIEWPORT, LAYOUT_TYPES } from '../../../common/constants';
import { Size } from '../../../common/types';
import { HeadlessChromiumDriver } from '../../browsers';
import { CaptureConfig } from '../../types';
@ -22,7 +22,8 @@ export class PrintLayout extends Layout implements LayoutInstance {
screenshot: '[data-shared-item]', // override '[data-shared-items-container]'
};
public readonly groupCount = 2;
private captureConfig: CaptureConfig;
private readonly captureConfig: CaptureConfig;
private readonly viewport = DEFAULT_VIEWPORT;
constructor(captureConfig: CaptureConfig) {
super(LAYOUT_TYPES.PRINT);
@ -34,7 +35,7 @@ export class PrintLayout extends Layout implements LayoutInstance {
}
public getBrowserViewport() {
return this.captureConfig.viewport;
return this.viewport;
}
public getBrowserZoom() {
@ -44,8 +45,8 @@ export class PrintLayout extends Layout implements LayoutInstance {
public getViewport(itemsCount: number) {
return {
zoom: this.captureConfig.zoom,
width: this.captureConfig.viewport.width,
height: this.captureConfig.viewport.height * itemsCount,
width: this.viewport.width,
height: this.viewport.height * itemsCount,
};
}
@ -56,8 +57,8 @@ export class PrintLayout extends Layout implements LayoutInstance {
logger.debug('positioning elements');
const elementSize: Size = {
width: this.captureConfig.viewport.width / this.captureConfig.zoom,
height: this.captureConfig.viewport.height / this.captureConfig.zoom,
width: this.viewport.width / this.captureConfig.zoom,
height: this.viewport.height / this.captureConfig.zoom,
};
const evalOptions: { fn: EvaluateFn; args: SerializableOrJSHandle[] } = {
fn: (selector: string, height: number, width: number) => {

View file

@ -45,10 +45,7 @@ export function getScreenshots$(
const apmTrans = apm.startTransaction(`reporting screenshot pipeline`, 'reporting');
const apmCreatePage = apmTrans?.startSpan('create_page', 'wait');
const create$ = browserDriverFactory.createPage(
{ viewport: layout.getBrowserViewport(), browserTimezone },
logger
);
const create$ = browserDriverFactory.createPage({ browserTimezone }, logger);
return create$.pipe(
mergeMap(({ driver, exit$ }) => {

View file

@ -114,7 +114,6 @@ export const createMockBrowserDriverFactory = async (
autoDownload: false,
},
networkPolicy: { enabled: true, rules: [] },
viewport: { width: 800, height: 600 },
loadDelay: moment.duration(2, 's'),
zoom: 2,
maxAttempts: 1,