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] Promote many debug log events to info #43241

Merged
merged 8 commits into from
Aug 15, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const getNumberOfItems = async (
layout: LayoutInstance,
logger: Logger
): Promise<number> => {
logger.debug('determining how many items we have');
logger.debug('determining how many rendered items to wait for');

// returns the value of the `itemsCountAttribute` if it's there, otherwise
// we just count the number of `itemSelector`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const getScreenshots = async ({
browser: HeadlessBrowser;
elementsPositionAndAttributes: ElementsPositionAndAttribute[];
}): Promise<Screenshot[]> => {
logger.debug(`taking screenshots`);
logger.info(`taking screenshots`);

const asyncDurationLogger = getAsyncDurationLogger(logger);
const screenshots: Screenshot[] = [];
Expand All @@ -45,7 +45,7 @@ export const getScreenshots = async ({
});
}

logger.debug(`screenshots taken: ${screenshots.length}`);
logger.info(`screenshots taken: ${screenshots.length}`);

return screenshots;
};
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,13 @@ export function screenshotsObservableFactory(server: KbnServer) {
layout,
browserTimezone,
}: ScreenshotObservableOpts): Rx.Observable<void> {
logger.debug(`Creating browser driver factory`);

const create$ = browserDriverFactory.create({
viewport: layout.getBrowserViewport(),
browserTimezone,
});

return create$.pipe(
mergeMap(({ driver$, exit$, message$, consoleMessage$ }) => {
logger.debug('Driver factory created');
message$.subscribe((line: string) => {
logger.debug(line, ['browser']);
});
Expand Down Expand Up @@ -92,10 +89,8 @@ export function screenshotsObservableFactory(server: KbnServer) {
({ browser, itemsCount }) => ({ browser, itemsCount })
),
mergeMap(
({ browser, itemsCount }) => {
logger.debug(`waiting for ${itemsCount} to be in the DOM`);
return waitForElementsToBeInDOM(browser, itemsCount, layout);
},
({ browser, itemsCount }) =>
waitForElementsToBeInDOM(browser, itemsCount, layout, logger),
({ browser, itemsCount }) => ({ browser, itemsCount })
),
mergeMap(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,25 @@
*/

import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers/chromium/driver';
import { LevelLogger as Logger } from '../../../../server/lib';
import { LayoutInstance } from '../../layouts/layout';

export const waitForElementsToBeInDOM = async (
browser: HeadlessBrowser,
itemsCount: number,
layout: LayoutInstance
): Promise<void> => {
layout: LayoutInstance,
logger: Logger
): Promise<number> => {
logger.debug(`waiting for ${itemsCount} rendered elements to be in the DOM`);

await browser.waitFor({
fn: selector => {
return document.querySelectorAll(selector).length;
},
args: [layout.selectors.renderComplete],
toEqual: itemsCount,
});

logger.info(`found ${itemsCount} rendered elements in the DOM`);
return itemsCount;
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
*/

import { CSV_JOB_TYPE, PLUGIN_ID } from '../../../common/constants';
import { cryptoFactory } from '../../../server/lib/crypto';
import { oncePerServer } from '../../../server/lib/once_per_server';
import { LevelLogger } from '../../../server/lib/level_logger';
import { cryptoFactory, oncePerServer, LevelLogger } from '../../../server/lib';
import { createGenerateCsv } from './lib/generate_csv';
import { fieldFormatMapFactory } from './lib/field_format_map';
import { i18n } from '@kbn/i18n';
Expand All @@ -16,7 +14,7 @@ function executeJobFn(server) {
const { callWithRequest } = server.plugins.elasticsearch.getCluster('data');
const crypto = cryptoFactory(server);
const config = server.config();
const logger = LevelLogger.createForServer(server, [PLUGIN_ID, CSV_JOB_TYPE]);
const logger = LevelLogger.createForServer(server, [PLUGIN_ID, CSV_JOB_TYPE, 'execute-job']);
const generateCsv = createGenerateCsv(logger);
const serverBasePath = config.get('server.basePath');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { notFound, notImplemented } from 'boom';
import { Request } from 'hapi';
import { get } from 'lodash';

import { PLUGIN_ID } from '../../../../common/constants';
import { PLUGIN_ID, CSV_FROM_SAVEDOBJECT_JOB_TYPE } from '../../../../common/constants';
import { cryptoFactory, LevelLogger, oncePerServer } from '../../../../server/lib';
import { JobDocPayload, JobParams, KbnServer } from '../../../../types';
import {
Expand All @@ -31,7 +31,11 @@ type CreateJobFn = (jobParams: JobParams, headers: any, req: Request) => Promise

function createJobFn(server: KbnServer): CreateJobFn {
const crypto = cryptoFactory(server);
const logger = LevelLogger.createForServer(server, [PLUGIN_ID, 'savedobject-csv']);
const logger = LevelLogger.createForServer(server, [
PLUGIN_ID,
CSV_FROM_SAVEDOBJECT_JOB_TYPE,
'create-job',
]);

return async function createJob(
jobParams: JobParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import { i18n } from '@kbn/i18n';

import { cryptoFactory, LevelLogger, oncePerServer } from '../../../server/lib';
import { JobDocOutputExecuted, JobDocPayload, KbnServer } from '../../../types';
import { CONTENT_TYPE_CSV, PLUGIN_ID } from '../../../common/constants';
import {
CONTENT_TYPE_CSV,
CSV_FROM_SAVEDOBJECT_JOB_TYPE,
PLUGIN_ID,
} from '../../../common/constants';
import { CsvResultFromSearch, createGenerateCsv } from './lib';

interface FakeRequest {
Expand All @@ -20,11 +24,15 @@ interface FakeRequest {

type ExecuteJobFn = (job: JobDocPayload, realRequest?: Request) => Promise<JobDocOutputExecuted>;

function executeJobFn(server: KbnServer): ExecuteJobFn {
function executeJobFactoryFn(server: KbnServer): ExecuteJobFn {
const crypto = cryptoFactory(server);
const config = server.config();
const serverBasePath = config.get('server.basePath');
const logger = LevelLogger.createForServer(server, [PLUGIN_ID, 'savedobject-csv']);
const logger = LevelLogger.createForServer(server, [
PLUGIN_ID,
CSV_FROM_SAVEDOBJECT_JOB_TYPE,
'execute-job',
]);
const generateCsv = createGenerateCsv(logger);

return async function executeJob(
Expand All @@ -38,10 +46,10 @@ function executeJobFn(server: KbnServer): ExecuteJobFn {

let requestObject: Request | FakeRequest;
if (isImmediate && realRequest) {
logger.debug(`executing job from immediate API`);
logger.info(`Executing job from immediate API`);
requestObject = realRequest;
} else {
logger.debug(`executing job async using encrypted headers`);
logger.info(`Executing job async using encrypted headers`);
let decryptedHeaders;
const serializedEncryptedHeaders = job.headers;
try {
Expand Down Expand Up @@ -99,4 +107,4 @@ function executeJobFn(server: KbnServer): ExecuteJobFn {
};
}

export const executeJobFactory = oncePerServer(executeJobFn);
export const executeJobFactory = oncePerServer(executeJobFactoryFn);
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@ interface UrlScreenshot {
function generatePngObservableFn(server: KbnServer) {
const screenshotsObservable = screenshotsObservableFactory(server);
const captureConcurrency = 1;
const createPngWithScreenshots = async ({
urlScreenshots,
}: {
urlScreenshots: UrlScreenshot[];
}) => {

// prettier-ignore
const createPngWithScreenshots = async ({ urlScreenshots }: { urlScreenshots: UrlScreenshot[] }) => {
if (urlScreenshots.length !== 1) {
throw new Error(
`Expected there to be 1 URL screenshot, but there are ${urlScreenshots.length}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ export class HeadlessChromiumDriver {
waitForSelector,
}: { conditionalHeaders: ConditionalHeaders; waitForSelector: string }
) {
this.logger.debug(`opening url ${url}`);
this.logger.info(`opening url ${url}`);
await this.page.setRequestInterception(true);
let interceptedCount = 0;
this.page.on('request', (interceptedRequest: any) => {
let isData = false;
if (this._shouldUseCustomHeaders(conditionalHeaders.conditions, interceptedRequest.url())) {
this.logger.debug(`Using custom headers for ${interceptedRequest.url()}`);
interceptedRequest.continue({
Expand All @@ -62,13 +64,17 @@ export class HeadlessChromiumDriver {
});
} else {
let interceptedUrl = interceptedRequest.url();

if (interceptedUrl.startsWith('data:')) {
// `data:image/xyz;base64` can be very long URLs
interceptedUrl = interceptedUrl.substring(0, 100) + '[truncated]';
isData = true;
}

this.logger.debug(`No custom headers for ${interceptedUrl}`);
interceptedRequest.continue();
}
interceptedCount = interceptedCount + (isData ? 0 : 1);
});

await this.page.goto(url, { waitUntil: 'domcontentloaded' });
Expand All @@ -78,6 +84,7 @@ export class HeadlessChromiumDriver {
}

await this.waitForSelector(waitForSelector);
this.logger.info(`handled ${interceptedCount} page requests`);
}

public async screenshot(elementPosition: ElementPosition) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import * as Rx from 'rxjs';
import { map, share, mergeMap, filter, partition, ignoreElements, tap } from 'rxjs/operators';
import { InnerSubscriber } from 'rxjs/internal/InnerSubscriber';

import { LevelLogger as Logger } from '../../../lib/level_logger';
import { HeadlessChromiumDriver } from '../driver';
import { args, IArgOptions } from './args';
import { safeChildProcess } from '../../safe_child_process';
import { getChromeLogLocation } from '../paths';
import { Logger } from '../../../../types';

type binaryPath = string;
type queueTimeout = number;
Expand All @@ -31,27 +31,27 @@ const compactWhitespace = (str: string) => {

export class HeadlessChromiumDriverFactory {
private binaryPath: binaryPath;
private callerLogger: Logger;
private logger: Logger;
private browserConfig: IBrowserConfig;
private queueTimeout: queueTimeout;

constructor(
binaryPath: binaryPath,
logger: any,
logger: Logger,
browserConfig: IBrowserConfig,
queueTimeout: queueTimeout
) {
this.binaryPath = binaryPath;
this.browserConfig = browserConfig;
this.queueTimeout = queueTimeout;
this.callerLogger = logger;
this.logger = logger;
}

type = 'chromium';

test(
{ viewport, browserTimezone }: { viewport: IArgOptions['viewport']; browserTimezone: string },
logger: any
logger: Logger
) {
const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chromium-'));
const chromiumArgs = args({
Expand Down Expand Up @@ -94,11 +94,13 @@ export class HeadlessChromiumDriverFactory {
exit$: Rx.Observable<never>;
}> {
return Rx.Observable.create(async (observer: InnerSubscriber<any, any>) => {
this.logger.debug(`Creating browser driver factory`);

const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chromium-'));
const chromiumArgs = args({
userDataDir,
viewport,
verboseLogging: this.callerLogger.isVerbose,
verboseLogging: this.logger.isVerbose,
Copy link
Member Author

@tsullivan tsullivan Aug 15, 2019

Choose a reason for hiding this comment

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

verboseLogging should just be removed: it is not a valid chromium arg.

I think I want to NOT do that in this PR

disableSandbox: this.browserConfig.disableSandbox,
proxyConfig: this.browserConfig.proxy,
});
Expand All @@ -125,6 +127,8 @@ export class HeadlessChromiumDriverFactory {
// "TimeoutError: waiting for selector ".application" failed: timeout 30000ms exceeded"
// @ts-ignore outdated typedefs for puppteer
page.setDefaultTimeout(this.queueTimeout);

this.logger.debug(`Browser driver factory created`);
} catch (err) {
observer.error(new Error(`Error spawning Chromium browser: [${err}]`));
throw err;
Expand All @@ -135,14 +139,12 @@ export class HeadlessChromiumDriverFactory {
await browser.close();
},
};
const { terminate$ } = safeChildProcess(this.callerLogger, childProcess);
const { terminate$ } = safeChildProcess(this.logger, childProcess);

// this is adding unsubscribe logic to our observer
// so that if our observer unsubscribes, we terminate our child-process
observer.add(() => {
this.callerLogger.debug(
`The browser process observer has unsubscribed. Closing the browser...`
);
this.logger.debug(`The browser process observer has unsubscribed. Closing the browser...`);
childProcess.kill(); // ignore async
});

Expand All @@ -151,7 +153,7 @@ export class HeadlessChromiumDriverFactory {
terminate$
.pipe(
tap(signal => {
this.callerLogger.debug(`Observer got signal: ${signal}`);
this.logger.debug(`Observer got signal: ${signal}`);
}),
ignoreElements()
)
Expand All @@ -174,7 +176,7 @@ export class HeadlessChromiumDriverFactory {

const driver$ = Rx.of(
new HeadlessChromiumDriver(page, {
logger: this.callerLogger,
logger: this.logger,
inspect: this.browserConfig.inspect,
})
);
Expand Down Expand Up @@ -250,7 +252,7 @@ export class HeadlessChromiumDriverFactory {
exit$,
});

const factoryLogger = this.callerLogger.clone(['chromium-driver-factory']);
const factoryLogger = this.logger.clone(['chromium-driver-factory']);
// unsubscribe logic makes a best-effort attempt to delete the user data directory used by chromium
observer.add(() => {
factoryLogger.debug(`deleting chromium user data directory at [${userDataDir}]`);
Expand Down
4 changes: 1 addition & 3 deletions x-pack/legacy/plugins/reporting/server/lib/create_worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

// @ts-ignore
import { PLUGIN_ID } from '../../common/constants';
import {
ESQueueInstance,
Expand All @@ -16,7 +15,6 @@ import {
} from '../../types';
// @ts-ignore untyped dependency
import { events as esqueueEvents } from './esqueue';
// @ts-ignore untyped dependency
import { LevelLogger } from './level_logger';
// @ts-ignore untyped dependency
import { oncePerServer } from './once_per_server';
Expand All @@ -27,7 +25,7 @@ function createWorkerFn(server: KbnServer) {
const kibanaName = config.get('server.name');
const kibanaId = config.get('server.uuid');
const exportTypesRegistry = server.plugins.reporting.exportTypesRegistry;
const logger = LevelLogger.createForServer(server, [PLUGIN_ID, 'queue', 'worker']);
const logger = LevelLogger.createForServer(server, [PLUGIN_ID, 'queue', 'create-worker']);

// Once more document types are added, this will need to be passed in
return function createWorker(queue: ESQueueInstance) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/reporting/server/lib/enqueue_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function enqueueJobFn(server) {

job.on(esqueueEvents.EVENT_JOB_CREATED, (createdJob) => {
if (createdJob.id === job.id) {
server.log(['reporting', 'debug'], `Saved object to process`);
server.log(['reporting', 'esqueue', 'info'], `Successfully queued job: ${createdJob.id}`);
resolve(job);
}
});
Expand Down
Loading