Skip to content

Commit

Permalink
[Reporting] Promote many debug log events to info (elastic#43241) (
Browse files Browse the repository at this point in the history
…elastic#43406)

* [Reporting] Move some log lines out of screenshots/index.ts

* promote some getScreenshots logs from debug to info

* promote some queue lifecycle event logs from debug to info

* Comments and renaming

* more info logging: csv from saved search

* rollback code changes to be done in future PR

* log success above error handling
  • Loading branch information
tsullivan committed Aug 16, 2019
1 parent fd3a70d commit 00e4631
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 55 deletions.
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 @@ -45,9 +45,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 @@ -58,17 +60,22 @@ 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' });
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,
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,
})
);

Expand Down Expand Up @@ -249,7 +251,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

0 comments on commit 00e4631

Please sign in to comment.