Skip to content

Commit

Permalink
[Screenshotting] instrument for benchmark tests using new EventLogger…
Browse files Browse the repository at this point in the history
… class (elastic#130356)

* use an EventLogger throughout a screenshotting flow

* unique id for each pipeline flow

* fix open_url logging

* add comments

* add unit test

* fix getTimeRangeEnd

* improve logging of thrown errors

* log the number of pixels using zoom

* use elementPositionAndAttributes for logging

* fix tests

* replace multiple methods for logging spans with single log method

* fix test

* fix sessionId not showing in error logs

* prettify message

* more logging improvements

* add specific error logging around get screenshots

* function level comments

* error handling around getting render errors

* ensure original logger.error always still called

* fix error logs not having the right logging context

* more error logging around pdfMaker

* more error logging around re-position elements

* fix test

* fix error re-throw after logging

* use apm to capture the error

* simplify eventLogger api

* single startTransaction method

* shortcut methods for screenshot/pdf event log

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
tsullivan and kibanamachine authored May 6, 2022
1 parent a6f7c46 commit fadd817
Show file tree
Hide file tree
Showing 27 changed files with 914 additions and 296 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/screenshotting/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,5 @@ export {
SCREENSHOTTING_EXPRESSION,
SCREENSHOTTING_EXPRESSION_INPUT,
} from './expression';

export const PLUGIN_ID = 'screenshotting';
14 changes: 8 additions & 6 deletions x-pack/plugins/screenshotting/server/formats/pdf/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
* 2.0.
*/

import { groupBy } from 'lodash';
import type { Values } from '@kbn/utility-types';
import type { Logger, PackageInfo } from '@kbn/core/server';
import { groupBy } from 'lodash';
import type { PackageInfo } from '@kbn/core/server';
import type { LayoutParams } from '../../../common';
import { LayoutTypes } from '../../../common';
import type { Layout } from '../../layouts';
import type { CaptureOptions, CaptureResult, CaptureMetrics } from '../../screenshots';
import type { CaptureMetrics, CaptureOptions, CaptureResult } from '../../screenshots';
import { EventLogger, Transactions } from '../../screenshots/event_logger';
import { pngsToPdf } from './pdf_maker';

/**
Expand Down Expand Up @@ -92,7 +93,7 @@ function getTimeRange(results: CaptureResult['results']) {
}

export async function toPdf(
logger: Logger,
eventLogger: EventLogger,
packageInfo: PackageInfo,
layout: Layout,
{ logo, title }: PdfScreenshotOptions,
Expand All @@ -106,7 +107,7 @@ export async function toPdf(
layout,
logo,
packageInfo,
logger,
eventLogger,
});

return {
Expand All @@ -119,7 +120,8 @@ export async function toPdf(
renderErrors: results.flatMap(({ renderErrors }) => renderErrors ?? []),
};
} catch (error) {
logger.error(`Could not generate the PDF buffer!`);
eventLogger.kbnLogger.error(`Could not generate the PDF buffer!`);
eventLogger.error(error, Transactions.PDF);

throw error;
}
Expand Down
62 changes: 34 additions & 28 deletions x-pack/plugins/screenshotting/server/formats/pdf/pdf_maker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@
* 2.0.
*/

import type { Logger, PackageInfo } from '@kbn/core/server';
import { PdfMaker } from './pdfmaker';
import type { PackageInfo } from '@kbn/core/server';
import type { Layout } from '../../../layouts';
import { getTracker } from './tracker';
import type { CaptureResult } from '../../../screenshots';
import { Actions, EventLogger, Transactions } from '../../../screenshots/event_logger';
import { PdfMaker } from './pdfmaker';

interface PngsToPdfArgs {
results: CaptureResult['results'];
layout: Layout;
packageInfo: PackageInfo;
logger: Logger;
eventLogger: EventLogger;
logo?: string;
title?: string;
}
Expand All @@ -26,37 +26,43 @@ export async function pngsToPdf({
logo,
title,
packageInfo,
logger,
eventLogger,
}: PngsToPdfArgs): Promise<{ buffer: Buffer; pages: number }> {
const pdfMaker = new PdfMaker(layout, logo, packageInfo, logger);
const tracker = getTracker();
if (title) {
pdfMaker.setTitle(title);
}
results.forEach((result) => {
result.screenshots.forEach((png) => {
tracker.startAddImage();
pdfMaker.addImage(png.data, {
title: png.title ?? undefined,
description: png.description ?? undefined,
});
tracker.endAddImage();
});
});
const { kbnLogger } = eventLogger;
const transactionEnd = eventLogger.startTransaction(Transactions.PDF);

let buffer: Uint8Array | null = null;
let pdfMaker: PdfMaker | null = null;
try {
tracker.startCompile();
pdfMaker = new PdfMaker(layout, logo, packageInfo, kbnLogger);
if (title) {
pdfMaker.setTitle(title);
}
results.forEach((result) => {
result.screenshots.forEach((png) => {
const spanEnd = eventLogger.logPdfEvent(
'add image to PDF file',
Actions.ADD_IMAGE,
'output'
);
pdfMaker?.addImage(png.data, {
title: png.title ?? undefined,
description: png.description ?? undefined,
});
spanEnd();
});
});

const spanEnd = eventLogger.logPdfEvent('compile PDF file', Actions.COMPILE, 'output');
buffer = await pdfMaker.generate();
tracker.endCompile();
spanEnd();

const byteLength = buffer?.byteLength ?? 0;
logger.debug(`PDF buffer byte length: ${byteLength}`);
tracker.setByteLength(byteLength);
} catch (err) {
throw err;
} finally {
tracker.end();
transactionEnd({ labels: { byte_length_pdf: byteLength, pdf_pages: pdfMaker.getPageCount() } });
} catch (error) {
kbnLogger.error(error);
eventLogger.error(error, Actions.COMPILE);
throw error;
}

return { buffer: Buffer.from(buffer.buffer), pages: pdfMaker.getPageCount() };
Expand Down

This file was deleted.

4 changes: 1 addition & 3 deletions x-pack/plugins/screenshotting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,9 @@ export class ScreenshottingPlugin implements Plugin<void, ScreenshottingStart, S

this.screenshots = (async () => {
const browserDriverFactory = await this.browserDriverFactory;
const logger = this.logger.get('screenshot');

return new Screenshots(
browserDriverFactory,
logger,
this.logger,
this.packageInfo,
http,
this.config,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit fadd817

Please sign in to comment.