From 23f90bec820bd3d9bee43a2e3319436948f0b0b3 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 30 Mar 2020 15:36:03 +0200 Subject: [PATCH] Adds some new configuration options and remove browser timings (#123) Configuration for rendering args and dumpio setting to ease investigation of problems. Removes browser timings due to possibly async issues. --- default.json | 7 +- dev.json | 8 ++- src/app.ts | 26 ++++--- src/browser/browser.ts | 111 ++++++++++++------------------ src/browser/clustered.ts | 6 +- src/browser/index.ts | 12 ++-- src/browser/reusable.ts | 20 ++---- src/config.ts | 6 +- src/metrics_browser_timings.ts | 49 ------------- src/plugin/grpc-plugin.ts | 17 +---- src/service/http-server.ts | 33 ++++----- src/service/metrics_middleware.ts | 8 ++- 12 files changed, 111 insertions(+), 192 deletions(-) delete mode 100644 src/metrics_browser_timings.ts diff --git a/default.json b/default.json index 248a4c7c..7824e06f 100644 --- a/default.json +++ b/default.json @@ -19,12 +19,15 @@ "timezone": null, "chromeBin": null, "ignoresHttpsErrors": false, - "timingMetrics": false, "mode": "default", "clustering": { "mode": "browser", "maxConcurrency": 5 }, - "verboseLogging": false + "verboseLogging": false, + "dumpio": false, + "args": [ + "--no-sandbox" + ] } } \ No newline at end of file diff --git a/dev.json b/dev.json index 269adbe1..5c9d4e44 100644 --- a/dev.json +++ b/dev.json @@ -19,12 +19,16 @@ "timezone": null, "chromeBin": null, "ignoresHttpsErrors": false, - "timingMetrics": false, "mode": "default", "clustering": { "mode": "browser", "maxConcurrency": 5 }, - "verboseLogging": false + "verboseLogging": false, + "dumpio": true, + "args": [ + "--no-sandbox", + "--disable-setuid-sandbox" + ] } } \ No newline at end of file diff --git a/src/app.ts b/src/app.ts index 92fde976..83640d7c 100644 --- a/src/app.ts +++ b/src/app.ts @@ -4,16 +4,14 @@ import * as _ from 'lodash'; import { GrpcPlugin } from './plugin/grpc-plugin'; import { HttpServer } from './service/http-server'; import { ConsoleLogger, PluginLogger } from './logger'; -import { NoOpBrowserTiming, createBrowser } from './browser'; +import { createBrowser } from './browser'; import * as minimist from 'minimist'; import { defaultPluginConfig, defaultServiceConfig, readJSONFileSync, PluginConfig, ServiceConfig } from './config'; -import { MetricsBrowserTimings } from './metrics_browser_timings'; async function main() { const argv = minimist(process.argv.slice(2)); const env = Object.assign({}, process.env); const command = argv._[0]; - let timings = new NoOpBrowserTiming(); if (command === undefined) { const config: PluginConfig = defaultPluginConfig; @@ -29,7 +27,7 @@ async function main() { } const logger = new PluginLogger(); - const browser = createBrowser(config.rendering, logger, timings); + const browser = createBrowser(config.rendering, logger); const plugin = new GrpcPlugin(config, logger, browser); plugin.start(); } else if (command === 'server') { @@ -47,12 +45,8 @@ async function main() { populateServiceConfigFromEnv(config, env); - if (config.service.metrics.enabled && config.rendering.timingMetrics) { - timings = new MetricsBrowserTimings(); - } - const logger = new ConsoleLogger(config.service.logging); - const browser = createBrowser(config.rendering, logger, timings); + const browser = createBrowser(config.rendering, logger); const server = new HttpServer(config, logger, browser); await server.start(); @@ -136,4 +130,18 @@ function populateServiceConfigFromEnv(config: ServiceConfig, env: NodeJS.Process if (env['RENDERING_VERBOSE_LOGGING']) { config.rendering.verboseLogging = env['RENDERING_VERBOSE_LOGGING'] === 'true'; } + + if (env['RENDERING_DUMPIO']) { + config.rendering.dumpio = env['RENDERING_DUMPIO'] === 'true'; + } + + if (env['RENDERING_ARGS']) { + const args = env['RENDERING_ARGS'] as string; + if (args.length > 0) { + const argsList = args.split(','); + if (argsList.length > 0) { + config.rendering.args = argsList; + } + } + } } diff --git a/src/browser/browser.ts b/src/browser/browser.ts index ca366098..c5b56913 100644 --- a/src/browser/browser.ts +++ b/src/browser/browser.ts @@ -21,43 +21,37 @@ export interface RenderResponse { filePath: string; } -export interface BrowserTimings { - launch(callback: () => Promise): Promise; - newPage(callback: () => Promise): Promise; - navigate(callback: () => Promise): Promise; - panelsRendered(callback: () => Promise): Promise; - screenshot(callback: () => Promise): Promise; -} - -export class NoOpBrowserTiming { - async launch(callback: () => Promise) { - return await callback(); - } - - async newPage(callback: () => Promise) { - return await callback(); - } - - async navigate(callback: () => Promise) { - return await callback(); - } - - async panelsRendered(callback: () => Promise) { - return await callback(); - } - - async screenshot(callback: () => Promise) { - return await callback(); - } -} - export class Browser { - constructor(protected config: RenderingConfig, protected log: Logger, protected timings: BrowserTimings) {} + constructor(protected config: RenderingConfig, protected log: Logger) { + this.log.info( + 'Browser initiated', + 'chromeBin', + this.config.chromeBin, + 'ignoresHttpsErrors', + this.config.ignoresHttpsErrors, + 'timezone', + this.config.timezone, + 'args', + this.config.args, + 'dumpio', + this.config.dumpio, + 'verboseLogging', + this.config.verboseLogging + ); + } async getBrowserVersion(): Promise { - const launcherOptions = this.getLauncherOptions({}); - const browser = await puppeteer.launch(launcherOptions); - return browser.version(); + let browser; + + try { + const launcherOptions = this.getLauncherOptions({}); + browser = await puppeteer.launch(launcherOptions); + return await browser.version(); + } finally { + if (browser) { + await browser.close(); + } + } } async start(): Promise {} @@ -84,7 +78,8 @@ export class Browser { const launcherOptions: any = { env: env, ignoreHTTPSErrors: this.config.ignoresHttpsErrors, - args: ['--no-sandbox'], + dumpio: this.config.dumpio, + args: this.config.args, }; if (this.config.chromeBin) { @@ -101,18 +96,8 @@ export class Browser { try { this.validateOptions(options); const launcherOptions = this.getLauncherOptions(options); - - browser = await this.timings.launch( - async () => - // launch browser - await puppeteer.launch(launcherOptions) - ); - page = await this.timings.newPage( - async () => - // open a new page - await browser.newPage() - ); - + browser = await puppeteer.launch(launcherOptions); + page = await browser.newPage(); this.addPageListeners(page); return await this.takeScreenshot(page, options); @@ -139,32 +124,22 @@ export class Browser { domain: options.domain, }); await page.mouse.move(options.width, options.height); - - await this.timings.navigate(async () => { - // wait until all data was loaded - await page.goto(options.url, { waitUntil: 'networkidle0' }); - }); - - await this.timings.panelsRendered(async () => { - // wait for all panels to render - await page.waitForFunction( - () => { - const panelCount = document.querySelectorAll('.panel').length || document.querySelectorAll('.panel-container').length; - return (window as any).panelsRendered >= panelCount; - }, - { - timeout: options.timeout * 1000, - } - ); - }); + await page.goto(options.url, { waitUntil: 'networkidle0' }); + await page.waitForFunction( + () => { + const panelCount = document.querySelectorAll('.panel').length || document.querySelectorAll('.panel-container').length; + return (window as any).panelsRendered >= panelCount; + }, + { + timeout: options.timeout * 1000, + } + ); if (!options.filePath) { options.filePath = uniqueFilename(os.tmpdir()) + '.png'; } - await this.timings.screenshot(async () => { - await page.screenshot({ path: options.filePath }); - }); + await page.screenshot({ path: options.filePath }); return { filePath: options.filePath }; } diff --git a/src/browser/clustered.ts b/src/browser/clustered.ts index 20619614..993a08e3 100644 --- a/src/browser/clustered.ts +++ b/src/browser/clustered.ts @@ -1,5 +1,5 @@ import { Cluster } from 'puppeteer-cluster'; -import { Browser, RenderResponse, BrowserTimings, RenderOptions } from './browser'; +import { Browser, RenderResponse, RenderOptions } from './browser'; import { Logger } from '../logger'; import { RenderingConfig, ClusteringConfig } from '../config'; @@ -8,8 +8,8 @@ export class ClusteredBrowser extends Browser { clusteringConfig: ClusteringConfig; concurrency: number; - constructor(config: RenderingConfig, log: Logger, timings: BrowserTimings) { - super(config, log, timings); + constructor(config: RenderingConfig, log: Logger) { + super(config, log); this.clusteringConfig = config.clustering; this.concurrency = Cluster.CONCURRENCY_BROWSER; diff --git a/src/browser/index.ts b/src/browser/index.ts index ff7f8ead..ab837480 100644 --- a/src/browser/index.ts +++ b/src/browser/index.ts @@ -1,21 +1,21 @@ import { RenderingConfig } from '../config'; import { Logger } from '../logger'; -import { Browser, BrowserTimings, NoOpBrowserTiming } from './browser'; +import { Browser } from './browser'; import { ClusteredBrowser } from './clustered'; import { ReusableBrowser } from './reusable'; -export function createBrowser(config: RenderingConfig, log: Logger, timings: BrowserTimings): Browser { +export function createBrowser(config: RenderingConfig, log: Logger): Browser { if (config.mode === 'clustered') { log.info('using clustered browser', 'mode', config.clustering.mode, 'maxConcurrency', config.clustering.maxConcurrency); - return new ClusteredBrowser(config, log, timings); + return new ClusteredBrowser(config, log); } if (config.mode === 'reusable') { log.info('using reusable browser'); - return new ReusableBrowser(config, log, timings); + return new ReusableBrowser(config, log); } - return new Browser(config, log, timings); + return new Browser(config, log); } -export { Browser, NoOpBrowserTiming }; +export { Browser }; diff --git a/src/browser/reusable.ts b/src/browser/reusable.ts index 2126bc2d..9a973734 100644 --- a/src/browser/reusable.ts +++ b/src/browser/reusable.ts @@ -1,23 +1,18 @@ import * as puppeteer from 'puppeteer'; -import { Browser, RenderResponse, BrowserTimings, RenderOptions } from './browser'; +import { Browser, RenderResponse, RenderOptions } from './browser'; import { Logger } from '../logger'; import { RenderingConfig } from '../config'; export class ReusableBrowser extends Browser { browser: puppeteer.Browser; - constructor(config: RenderingConfig, log: Logger, timings: BrowserTimings) { - super(config, log, timings); + constructor(config: RenderingConfig, log: Logger) { + super(config, log); } async start(): Promise { const launcherOptions = this.getLauncherOptions({}); - - this.browser = await this.timings.launch( - async () => - // launch browser - await puppeteer.launch(launcherOptions) - ); + this.browser = await puppeteer.launch(launcherOptions); } async render(options: RenderOptions): Promise { @@ -27,12 +22,7 @@ export class ReusableBrowser extends Browser { try { this.validateOptions(options); context = await this.browser.createIncognitoBrowserContext(); - - page = await this.timings.newPage( - async () => - // open a new page - await context.newPage() - ); + page = await context.newPage(); if (options.timezone) { // set timezone diff --git a/src/config.ts b/src/config.ts index 3b4853af..40a67106 100644 --- a/src/config.ts +++ b/src/config.ts @@ -9,10 +9,11 @@ export interface RenderingConfig { timezone?: string; chromeBin?: string; ignoresHttpsErrors: boolean; - timingMetrics: boolean; mode: string; clustering: ClusteringConfig; verboseLogging: boolean; + dumpio: boolean; + args: string[]; } export interface MetricsConfig { @@ -56,13 +57,14 @@ const defaultRenderingConfig: RenderingConfig = { timezone: undefined, chromeBin: undefined, ignoresHttpsErrors: false, - timingMetrics: false, mode: 'default', clustering: { mode: 'browser', maxConcurrency: 5, }, verboseLogging: false, + dumpio: false, + args: ['--no-sandbox'], }; export const defaultServiceConfig: ServiceConfig = { diff --git a/src/metrics_browser_timings.ts b/src/metrics_browser_timings.ts deleted file mode 100644 index 1db23b80..00000000 --- a/src/metrics_browser_timings.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { Browser as PuppeteerBrowser, Page } from 'puppeteer'; -import * as _ from 'lodash'; -import * as promClient from 'prom-client'; -import { Histogram } from 'prom-client'; - -export class MetricsBrowserTimings { - durationHistogram: Histogram; - - constructor() { - this.durationHistogram = new promClient.Histogram({ - name: 'grafana_image_renderer_step_duration_seconds', - help: 'duration histogram of browser steps for rendering an image labeled with: step', - labelNames: ['step'], - buckets: [0.3, 0.5, 1, 2, 3, 5], - }); - } - - async launch(callback: () => Promise) { - const timer = this.durationHistogram.startTimer({ step: 'launch' }); - const browser = await callback(); - timer(); - return browser; - } - - async newPage(callback: () => Promise) { - const timer = this.durationHistogram.startTimer({ step: 'newPage' }); - const page = await callback(); - timer(); - return page; - } - - async navigate(callback: () => Promise) { - const timer = this.durationHistogram.startTimer({ step: 'navigate' }); - await callback(); - timer(); - } - - async panelsRendered(callback: () => Promise) { - const timer = this.durationHistogram.startTimer({ step: 'panelsRendered' }); - await callback(); - timer(); - } - - async screenshot(callback: () => Promise) { - const timer = this.durationHistogram.startTimer({ step: 'screenshot' }); - await callback(); - timer(); - } -} diff --git a/src/plugin/grpc-plugin.ts b/src/plugin/grpc-plugin.ts index 5eb61ca5..13007bdb 100644 --- a/src/plugin/grpc-plugin.ts +++ b/src/plugin/grpc-plugin.ts @@ -51,22 +51,7 @@ export class GrpcPlugin { console.log(`1|1|tcp|${this.config.plugin.grpc.host}:${boundPortNumber}|grpc`); await this.browser.start(); - - if (this.config.rendering.chromeBin) { - this.log.info( - 'Renderer plugin started', - 'grpcHost', - this.config.plugin.grpc.host, - 'grpcPort', - boundPortNumber, - 'chromeBin', - this.config.rendering.chromeBin, - 'ignoreHTTPSErrors', - this.config.rendering.ignoresHttpsErrors - ); - } else { - this.log.info('Renderer plugin started', 'ignoreHttpsErrors', this.config.rendering.ignoresHttpsErrors); - } + this.log.info('Renderer plugin started', 'grpcHost', this.config.plugin.grpc.host, 'grpcPort', boundPortNumber); } check(call, callback) { diff --git a/src/service/http-server.ts b/src/service/http-server.ts index bb0847c7..fcda4ac2 100644 --- a/src/service/http-server.ts +++ b/src/service/http-server.ts @@ -48,14 +48,6 @@ export class HttpServer { return res.status(err.output.statusCode).json(err.output.payload); }); - if (this.config.rendering.chromeBin) { - this.log.info(`Using chromeBin ${this.config.rendering.chromeBin}`); - } - - if (this.config.rendering.ignoresHttpsErrors) { - this.log.info(`Ignoring HTTPS errors`); - } - if (this.config.service.host) { const server = this.app.listen(this.config.service.port, this.config.service.host, () => { const info = server.address() as net.AddressInfo; @@ -68,19 +60,22 @@ export class HttpServer { }); } - const browserInfo = new promClient.Gauge({ - name: 'grafana_image_renderer_browser_info', - help: "A metric with a constant '1 value labeled by version of the browser in use", - labelNames: ['version'], - }); + if (this.config.service.metrics.enabled) { + const browserInfo = new promClient.Gauge({ + name: 'grafana_image_renderer_browser_info', + help: "A metric with a constant '1 value labeled by version of the browser in use", + labelNames: ['version'], + }); - try { - const browserVersion = await this.browser.getBrowserVersion(); - browserInfo.labels(browserVersion).set(1); - } catch { - this.log.error('Failed to get browser version'); - browserInfo.labels('unknown').set(19); + try { + const browserVersion = await this.browser.getBrowserVersion(); + browserInfo.labels(browserVersion).set(1); + } catch { + this.log.error('Failed to get browser version'); + browserInfo.labels('unknown').set(0); + } } + await this.browser.start(); } diff --git a/src/service/metrics_middleware.ts b/src/service/metrics_middleware.ts index b6f1ea39..46cf0c3a 100644 --- a/src/service/metrics_middleware.ts +++ b/src/service/metrics_middleware.ts @@ -9,7 +9,13 @@ export const metricsMiddleware = (config: MetricsConfig, log: Logger) => { }; } - log.info('Metrics enabled'); + log.info( + 'Metrics enabled', + 'collectDefaultMetrics', + config.collectDefaultMetrics, + 'requestDurationBuckets', + config.requestDurationBuckets.join(',') + ); const opts = { httpDurationMetricName: 'grafana_image_renderer_service_http_request_duration_seconds',