Skip to content

[Reporting] Log the chromium child process PID#44947

Closed
tsullivan wants to merge 3 commits intoelastic:masterfrom
tsullivan:reporting/log-pid
Closed

[Reporting] Log the chromium child process PID#44947
tsullivan wants to merge 3 commits intoelastic:masterfrom
tsullivan:reporting/log-pid

Conversation

@tsullivan
Copy link
Member

@tsullivan tsullivan commented Sep 5, 2019

Summary

While testing methods for getting the stdin and stdout streams from Puppeteer's child Chromium process, it occurred to me that, although that it hard, it is easy to get the PID of that process. Such a thing was previously thought to be hard. That's probably due to the relative new-ness of the browser.process() method.

Theoretically, this could be a step for having a very nice Monitoring of Reporting solution.

This also replaces some any type defs that were in place for HeadlessChromiumDriver objects.

Release Note (Enhancement): Added debug-level logging of the headless browser process ID that is spawned to take screenshots of Kibana for PDF and PNG reports.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

@tsullivan tsullivan added review zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead low hanging fruit v8.0.0 v7.5.0 labels Sep 5, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services

ignoreHTTPSErrors: true,
args: chromiumArgs,
env: {
TZ: browserTimezone,
Copy link
Member Author

Choose a reason for hiding this comment

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

browserTimezone wasn't getting passed

logger,
browserConfig,
queueTimeout
): HeadlessChromiumDriverFactory {
Copy link
Member Author

Choose a reason for hiding this comment

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

oops, this isn't a TS file

viewport,
verboseLogging: this.logger.isVerbose,
disableSandbox: this.browserConfig.disableSandbox,
proxyConfig: this.browserConfig.proxy,
Copy link
Member Author

@tsullivan tsullivan Sep 5, 2019

Choose a reason for hiding this comment

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

This might have been hiding a bug. If it is meant to match the Reporting Settings, then it should be proxy: https://www.elastic.co/guide/en/kibana/current/reporting-settings-kb.html#reporting-chromium-settings

} as LaunchOptions)
.then((browser: Browser) => {
const childProcess: ChildProcess = browser.process();
logger.debug(`Test browser process launched with PID: ${childProcess.pid}`);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is 1/2 of the actual changes. Everything else is due to increase strictness of Typescript

} as LaunchOptions);

// log the child process PID
const childProcess: ChildProcess = browser.process();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is 2/2 of the actual changes. Everything else is due to increase strictness of Typescript

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

flags.push('--no-sandbox');
}

// Dead code?
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like a bug to wrap this in a conditional. I think we want the logging flags hardcoded to "on":

https://www.chromium.org/for-testers/enable-logging

@tsullivan
Copy link
Member Author

I don't think we need the enhancement of browser process PID logged. Closing this and will re-open with just the Typescript improvements

@tsullivan
Copy link
Member Author

The browser PID can be introduced in this PR: #45483

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:enhancement review v7.5.0 v8.0.0 zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants