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] APM integration for baseline performance measurements #59967

Merged
merged 27 commits into from
May 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2521d76
fix cluster_client
tsullivan Mar 11, 2020
912a4ad
apm stuff
tsullivan Mar 10, 2020
a865bcc
fix snapshot
tsullivan Mar 12, 2020
0fc8813
tracker utility for generate_pdf
tsullivan Mar 12, 2020
cbd094b
call apm.startSpan instead of txn.startSpan
tsullivan Mar 12, 2020
c037d4d
Fix async call to end transaction
tsullivan Mar 12, 2020
0aa13e6
fix typescript
tsullivan Mar 12, 2020
c5c80f5
Merge branch 'master' into reporting/performance-measurement
tsullivan Mar 16, 2020
30af06e
remove captuureErrors
tsullivan Mar 16, 2020
b980ba2
Merge branch 'master' into reporting/performance-measurement
tsullivan Mar 20, 2020
1e3c6cc
restore accidental removal
tsullivan Mar 20, 2020
26bf57e
Merge branch 'master' into reporting/performance-measurement
tsullivan Mar 30, 2020
33debce
add startTrace lib
tsullivan Mar 30, 2020
cc57a7a
fix import
tsullivan Mar 30, 2020
498a629
Merge remote-tracking branch 'elastic/master' into reporting/performa…
tsullivan Apr 13, 2020
fd63584
fix imports
tsullivan Apr 13, 2020
2b564ff
Merge branch 'master' into reporting/performance-measurement
tsullivan Apr 13, 2020
890e5f2
Merge branch 'master' into reporting/performance-measurement
tsullivan Apr 28, 2020
3db397f
ts fix
tsullivan Apr 28, 2020
e193ecf
Merge branch 'master' into reporting/performance-measurement
tsullivan Apr 29, 2020
e79e875
fix generate_png to not format base64 to buffer and back to base64
tsullivan Apr 29, 2020
7618e42
Merge branch 'master' into reporting/performance-measurement
elasticmachine Apr 30, 2020
0867800
Merge branch 'master' into reporting/performance-measurement
tsullivan May 5, 2020
4ac5ce0
💅
tsullivan May 5, 2020
6b5c392
revert change to cluster client
tsullivan May 5, 2020
dd9c55b
fix unused translation
tsullivan May 5, 2020
e4495c4
Merge branch 'master' into reporting/performance-measurement
tsullivan May 7, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@

import { i18n } from '@kbn/i18n';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger as Logger, startTrace } from '../../../../server/lib';
import { LayoutInstance } from '../../layouts/layout';
import { AttributesMap, ElementsPositionAndAttribute } from './types';
import { Logger } from '../../../../types';
import { CONTEXT_ELEMENTATTRIBUTES } from './constants';
import { AttributesMap, ElementsPositionAndAttribute } from './types';

export const getElementPositionAndAttributes = async (
browser: HeadlessBrowser,
layout: LayoutInstance,
logger: Logger
): Promise<ElementsPositionAndAttribute[] | null> => {
const endTrace = startTrace('get_element_position_data', 'read');
const { screenshot: screenshotSelector } = layout.selectors; // data-shared-items-container
let elementsPositionAndAttributes: ElementsPositionAndAttribute[] | null;
try {
Expand Down Expand Up @@ -69,5 +70,7 @@ export const getElementPositionAndAttributes = async (
elementsPositionAndAttributes = null;
}

endTrace();

return elementsPositionAndAttributes;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { i18n } from '@kbn/i18n';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger } from '../../../../server/lib';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { CaptureConfig } from '../../../../server/types';
import { LayoutInstance } from '../../layouts/layout';
import { CONTEXT_GETNUMBEROFITEMS, CONTEXT_READMETADATA } from './constants';
Expand All @@ -17,6 +17,7 @@ export const getNumberOfItems = async (
layout: LayoutInstance,
logger: LevelLogger
): Promise<number> => {
const endTrace = startTrace('get_number_of_items', 'read');
const { renderComplete: renderCompleteSelector, itemsCountAttribute } = layout.selectors;
let itemsCount: number;

Expand Down Expand Up @@ -70,5 +71,7 @@ export const getNumberOfItems = async (
itemsCount = 1;
}

endTrace();

return itemsCount;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,9 @@

import { i18n } from '@kbn/i18n';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger } from '../../../../server/lib';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { Screenshot, ElementsPositionAndAttribute } from './types';

const getAsyncDurationLogger = (logger: LevelLogger) => {
return async (description: string, promise: Promise<any>) => {
const start = Date.now();
const result = await promise;
logger.debug(
i18n.translate('xpack.reporting.screencapture.asyncTook', {
defaultMessage: '{description} took {took}ms',
values: {
description,
took: Date.now() - start,
},
})
);
return result;
};
};

export const getScreenshots = async (
browser: HeadlessBrowser,
elementsPositionAndAttributes: ElementsPositionAndAttribute[],
Expand All @@ -37,21 +20,20 @@ export const getScreenshots = async (
})
);

const asyncDurationLogger = getAsyncDurationLogger(logger);
const screenshots: Screenshot[] = [];

for (let i = 0; i < elementsPositionAndAttributes.length; i++) {
const endTrace = startTrace('get_screenshots', 'read');
const item = elementsPositionAndAttributes[i];
const base64EncodedData = await asyncDurationLogger(
Copy link
Member Author

@tsullivan tsullivan Mar 12, 2020

Choose a reason for hiding this comment

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

asyncDurationLogger was hiding a type error, because we're resolving a string, not a Buffer

Copy link
Member Author

Choose a reason for hiding this comment

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

More context about this: since we'll have APM to measure the duration of all these modules, we no longer need this helper function. After having removed it, I noticed the surrounding code treated the helper function return value as a Buffer but it is actually a string of base64 content.

`screenshot #${i + 1}`,
browser.screenshot(item.position)
);
const base64EncodedData = await browser.screenshot(item.position);

screenshots.push({
base64EncodedData,
title: item.attributes.title,
description: item.attributes.description,
});

endTrace();
}

logger.info(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger } from '../../../../server/lib';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { LayoutInstance } from '../../layouts/layout';
import { CONTEXT_GETTIMERANGE } from './constants';
import { TimeRange } from './types';
Expand All @@ -15,6 +15,7 @@ export const getTimeRange = async (
layout: LayoutInstance,
logger: LevelLogger
): Promise<TimeRange | null> => {
const endTrace = startTrace('get_time_range', 'read');
logger.debug('getting timeRange');

const timeRange: TimeRange | null = await browser.evaluate(
Expand Down Expand Up @@ -45,5 +46,7 @@ export const getTimeRange = async (
logger.debug('no timeRange');
}

endTrace();

return timeRange;
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import { i18n } from '@kbn/i18n';
import fs from 'fs';
import { promisify } from 'util';
import { LevelLogger } from '../../../../server/lib';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { Layout } from '../../layouts/layout';
import { CONTEXT_INJECTCSS } from './constants';

Expand All @@ -19,6 +19,7 @@ export const injectCustomCss = async (
layout: Layout,
logger: LevelLogger
): Promise<void> => {
const endTrace = startTrace('inject_css', 'correction');
logger.debug(
i18n.translate('xpack.reporting.screencapture.injectingCss', {
defaultMessage: 'injecting custom css',
Expand Down Expand Up @@ -49,4 +50,6 @@ export const injectCustomCss = async (
})
);
}

endTrace();
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,18 @@
* you may not use this file except in compliance with the Elastic License.
*/

import apm from 'elastic-apm-node';
import * as Rx from 'rxjs';
import { catchError, concatMap, first, mergeMap, take, takeUntil, toArray } from 'rxjs/operators';
import {
catchError,
concatMap,
first,
mergeMap,
take,
takeUntil,
tap,
toArray,
} from 'rxjs/operators';
import { CaptureConfig } from '../../../../server/types';
import { DEFAULT_PAGELOAD_SELECTOR } from '../../constants';
import { HeadlessChromiumDriverFactory } from '../../../../types';
Expand Down Expand Up @@ -41,13 +51,17 @@ export function screenshotsObservableFactory(
layout,
browserTimezone,
}: ScreenshotObservableOpts): Rx.Observable<ScreenshotResults[]> {
const apmTrans = apm.startTransaction(`reporting screenshot pipeline`, 'reporting');

const apmCreatePage = apmTrans?.startSpan('create_page', 'wait');
const create$ = browserDriverFactory.createPage(
{ viewport: layout.getBrowserViewport(), browserTimezone },
logger
);

return create$.pipe(
mergeMap(({ driver, exit$ }) => {
if (apmCreatePage) apmCreatePage.end();
return Rx.from(urls).pipe(
concatMap((url, index) => {
const setup$: Rx.Observable<ScreenSetupData> = Rx.of(1).pipe(
Expand Down Expand Up @@ -81,10 +95,12 @@ export function screenshotsObservableFactory(
// allows for them to be displayed properly in many cases
await injectCustomCss(driver, layout, logger);

const apmPositionElements = apmTrans?.startSpan('position_elements', 'correction');
if (layout.positionElements) {
// position panel elements for print layout
await layout.positionElements(driver, logger);
}
if (apmPositionElements) apmPositionElements.end();

await waitForRenderComplete(captureConfig, driver, layout, logger);
}),
Expand Down Expand Up @@ -125,7 +141,10 @@ export function screenshotsObservableFactory(
toArray()
);
}),
first()
first(),
tap(() => {
if (apmTrans) apmTrans.end();
})
);
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { i18n } from '@kbn/i18n';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger } from '../../../../server/lib';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { CaptureConfig } from '../../../../server/types';
import { ConditionalHeaders } from '../../../../types';

Expand All @@ -18,6 +18,7 @@ export const openUrl = async (
conditionalHeaders: ConditionalHeaders,
logger: LevelLogger
): Promise<void> => {
const endTrace = startTrace('open_url', 'wait');
try {
await browser.open(
url,
Expand All @@ -32,11 +33,10 @@ export const openUrl = async (
throw new Error(
i18n.translate('xpack.reporting.screencapture.couldntLoadKibana', {
defaultMessage: `An error occurred when trying to open the Kibana URL. You may need to increase '{configKey}'. {error}`,
values: {
configKey: 'xpack.reporting.capture.timeouts.openUrl',
error: err,
},
values: { configKey: 'xpack.reporting.capture.timeouts.openUrl', error: err },
})
);
}

endTrace();
};
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface ElementsPositionAndAttribute {
}

export interface Screenshot {
base64EncodedData: Buffer;
base64EncodedData: string;
title: string;
description: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { i18n } from '@kbn/i18n';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger } from '../../../../server/lib';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { CaptureConfig } from '../../../../server/types';
import { LayoutInstance } from '../../layouts/layout';
import { CONTEXT_WAITFORRENDER } from './constants';
Expand All @@ -17,6 +17,8 @@ export const waitForRenderComplete = async (
layout: LayoutInstance,
logger: LevelLogger
) => {
const endTrace = startTrace('wait_for_render', 'wait');

logger.debug(
i18n.translate('xpack.reporting.screencapture.waitingForRenderComplete', {
defaultMessage: 'waiting for rendering to complete',
Expand Down Expand Up @@ -76,5 +78,7 @@ export const waitForRenderComplete = async (
defaultMessage: 'rendering is complete',
})
);

endTrace();
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { i18n } from '@kbn/i18n';
import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/browsers';
import { LevelLogger } from '../../../../server/lib';
import { LevelLogger, startTrace } from '../../../../server/lib';
import { CaptureConfig } from '../../../../server/types';
import { LayoutInstance } from '../../layouts/layout';
import { CONTEXT_WAITFORELEMENTSTOBEINDOM } from './constants';
Expand All @@ -29,6 +29,7 @@ export const waitForVisualizations = async (
layout: LayoutInstance,
logger: LevelLogger
): Promise<void> => {
const endTrace = startTrace('wait_for_visualizations', 'wait');
const { renderComplete: renderCompleteSelector } = layout.selectors;

logger.debug(
Expand Down Expand Up @@ -63,4 +64,6 @@ export const waitForVisualizations = async (
})
);
}

endTrace();
};
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ test(`returns content_type of application/png`, async () => {
const encryptedHeaders = await encryptHeaders({});

const generatePngObservable = (await generatePngObservableFactory(mockReporting)) as jest.Mock;
generatePngObservable.mockReturnValue(Rx.of(Buffer.from('')));
generatePngObservable.mockReturnValue(Rx.of('foo'));

const { content_type: contentType } = await executeJob(
'pngJobId',
Expand All @@ -137,10 +137,10 @@ test(`returns content_type of application/png`, async () => {
});

test(`returns content of generatePng getBuffer base64 encoded`, async () => {
const testContent = 'test content';
const testContent = 'raw string from get_screenhots';

const generatePngObservable = (await generatePngObservableFactory(mockReporting)) as jest.Mock;
generatePngObservable.mockReturnValue(Rx.of({ buffer: Buffer.from(testContent) }));
generatePngObservable.mockReturnValue(Rx.of({ base64: testContent }));

const executeJob = await executeJobFactory(mockReporting, getMockLogger());
const encryptedHeaders = await encryptHeaders({});
Expand All @@ -150,5 +150,5 @@ test(`returns content of generatePng getBuffer base64 encoded`, async () => {
cancellationToken
);

expect(content).toEqual(Buffer.from(testContent).toString('base64'));
expect(content).toEqual(testContent);
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import apm from 'elastic-apm-node';
import * as Rx from 'rxjs';
import { catchError, map, mergeMap, takeUntil } from 'rxjs/operators';
import { PNG_JOB_TYPE } from '../../../../common/constants';
Expand All @@ -29,6 +30,10 @@ export const executeJobFactory: QueuedPngExecutorFactory = async function execut
const logger = parentLogger.clone([PNG_JOB_TYPE, 'execute']);

return async function executeJob(jobId: string, job: JobDocPayloadPNG, cancellationToken: any) {
const apmTrans = apm.startTransaction('reporting execute_job png', 'reporting');
const apmGetAssets = apmTrans?.startSpan('get_assets', 'setup');
let apmGeneratePng: { end: () => void } | null | undefined;

const generatePngObservable = await generatePngObservableFactory(reporting);
const jobLogger = logger.clone([jobId]);
const process$: Rx.Observable<JobDocOutput> = Rx.of(1).pipe(
Expand All @@ -38,6 +43,9 @@ export const executeJobFactory: QueuedPngExecutorFactory = async function execut
mergeMap(conditionalHeaders => {
const urls = getFullUrls({ config, job });
const hashUrl = urls[0];
if (apmGetAssets) apmGetAssets.end();

apmGeneratePng = apmTrans?.startSpan('generate_png_pipeline', 'execute');
return generatePngObservable(
jobLogger,
hashUrl,
Expand All @@ -46,11 +54,14 @@ export const executeJobFactory: QueuedPngExecutorFactory = async function execut
job.layout
);
}),
map(({ buffer, warnings }) => {
map(({ base64, warnings }) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to rename this - it was very unclear that the return payload actually is a string

if (apmGeneratePng) apmGeneratePng.end();

return {
content_type: 'image/png',
content: buffer.toString('base64'),
size: buffer.byteLength,
content: base64,
size: (base64 && base64.length) || 0,

warnings,
};
}),
Expand Down
Loading