Printing process warning's via the appropriate loggers #8746

kobelb · 2016-10-19T10:49:48Z

Node v6.6.0 by default writes warnings to STDERR per https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_warning

This modification would by default suppress these warnings from being written to STDERR by using the --no-warnings flag, and then we'd selectively write these errors using the proper logging method.

Addresses: #8746

kobelb · 2016-10-19T10:51:00Z

bin/kibana

@@ -21,4 +21,4 @@ if [ ! -x "$NODE" ]; then
  exit 1
 fi

-exec "${NODE}" $NODE_OPTIONS "${DIR}/src/cli" ${@}


I haven't been able to find a way to selectively add this flag, besides potentially determining if the --dev flag is provided.

I think adding the flag all the time is fine, since we are forwarding all the messages via the server logger.

kobelb · 2016-10-19T10:51:53Z

bin/kibana-plugin

@@ -21,4 +21,4 @@ if [ ! -x "$NODE" ]; then
  exit 1
 fi

-exec "${NODE}" $NODE_OPTIONS "${DIR}/src/cli_plugin" ${@}
+exec "${NODE}" $NODE_OPTIONS --no-warnings "${DIR}/src/cli_plugin" ${@}


The kibana-plugin is currently not displaying the warnings at all, currently investigating the best way to determine if "we're in production"

The cli plugin command has it's own logger with it's own opinions about when messages should and shouldn't be logged (controlled with --silent and --quiet) https://github.com/elastic/kibana/blob/master/src/cli_plugin/lib/logger.js

We should probably wire this up in that cli

kobelb · 2016-10-19T10:53:04Z

src/server/warnings/index.js

@@ -0,0 +1,9 @@
+export default function (kbnServer, server, config) {


I'm assuming that we only have one instance of the kbnServer per 'process' (including when we're clustering) so we won't be subscribing to this event twice and possibly getting duplicate warnings.

I'm assuming that we only have one instance of the kbnServer per 'process' (including when we're clustering) so we won't be subscribing to this event twice and possibly getting duplicate warnings.

Not necessarily, but I don't think there is a better option right now.

kobelb · 2016-10-19T17:09:49Z

@spalger @tylersmalley should we consider switching the approach to the one that tyler has here: 0865c55

The main benefit that I see from Tyler's approach, is that warnings that are emitted before we hit the place where I'm registering the process.on('warning', ...) will be written to STDERR; with the trade-off that when they are written, they won't be in our standard 'format'.

spalger · 2016-10-19T17:15:06Z

src/server/warnings/index.js

@@ -0,0 +1,9 @@
+export default function (kbnServer, server, config) {


I'm assuming that we only have one instance of the kbnServer per 'process' (including when we're clustering) so we won't be subscribing to this event twice and possibly getting duplicate warnings.

Not necessarily, but I don't think there is a better option right now.

spalger · 2016-10-19T17:15:34Z

src/server/warnings/index.js

+  }
+
+  process.on('warning', (warning) => {
+    server.log(['warning'], warning);


We should add a 'process' tag so that these warnings can be identified specifically

spalger · 2016-10-19T17:19:46Z

bin/kibana-plugin

@@ -21,4 +21,4 @@ if [ ! -x "$NODE" ]; then
  exit 1
 fi

-exec "${NODE}" $NODE_OPTIONS "${DIR}/src/cli_plugin" ${@}
+exec "${NODE}" $NODE_OPTIONS --no-warnings "${DIR}/src/cli_plugin" ${@}


The cli plugin command has it's own logger with it's own opinions about when messages should and shouldn't be logged (controlled with --silent and --quiet) https://github.com/elastic/kibana/blob/master/src/cli_plugin/lib/logger.js

We should probably wire this up in that cli

spalger · 2016-10-19T17:20:21Z

bin/kibana

@@ -21,4 +21,4 @@ if [ ! -x "$NODE" ]; then
  exit 1
 fi

-exec "${NODE}" $NODE_OPTIONS "${DIR}/src/cli" ${@}


I think adding the flag all the time is fine, since we are forwarding all the messages via the server logger.

tylersmalley

LGTM

As an added benefit we actually get the stack trace, abiet not very helpful in this case.

--------- **Commit 1:** Printing process warning's via the appropriate logger * Original sha: 96ef75b * Authored by = <[email protected]> on 2016-10-19T10:41:45Z

[backport] PR #8746 to 5.0

[backport] PR #8746 to 5.x

jbudz · 2016-11-28T20:32:04Z

Does this need to be backported to 4.7?

kobelb · 2016-11-28T20:33:31Z

@jbudz if we're upgrading Node to 6.x, yup.

Node 6.6+ writes warnings to stderr rather than stdout, but we want those warnings to piped through as warnings in our Kibana logger instead.

--------- **Commit 1:** Printing process warning's via the appropriate logger * Original sha: f464a80786985c596a8f1155164a044c5f0fa39f [formerly 96ef75b] * Authored by = <[email protected]> on 2016-10-19T10:41:45Z Former-commit-id: 32cbd7c

[backport] PR elastic#8746 to 5.x Former-commit-id: 1171461

kobelb commented Oct 19, 2016

View reviewed changes

spalger self-assigned this Oct 19, 2016

spalger suggested changes Oct 19, 2016

View reviewed changes

Printing process warning's via the appropriate logger

96ef75b

kobelb force-pushed the nodejs-warnings branch from 6f4aaac to 96ef75b Compare October 19, 2016 18:10

kobelb added v5.0.0 review v5.1.0 v6.0.0 labels Oct 19, 2016

kobelb changed the title ~~Only displaying warnings for the kbn_server in dev using server.log~~ Printing process warning's via the appropriate loggers Oct 19, 2016

spalger approved these changes Oct 19, 2016

View reviewed changes

tylersmalley self-assigned this Oct 19, 2016

tylersmalley approved these changes Oct 19, 2016

View reviewed changes

kobelb merged commit 3e3332f into elastic:master Oct 19, 2016

elastic-jasper added a commit that referenced this pull request Oct 19, 2016

Backport PR #8746

11249b4

--------- **Commit 1:** Printing process warning's via the appropriate logger * Original sha: 96ef75b * Authored by = <[email protected]> on 2016-10-19T10:41:45Z

This was referenced Oct 19, 2016

[backport] PR #8746 to 5.0 - Printing process warning's via the appropriate loggers #8755

Merged

[backport] PR #8746 to 5.1 #8756

Closed

elastic-jasper added a commit that referenced this pull request Oct 19, 2016

Backport PR #8746

32cbd7c

--------- **Commit 1:** Printing process warning's via the appropriate logger * Original sha: 96ef75b * Authored by = <[email protected]> on 2016-10-19T10:41:45Z

elastic-jasper mentioned this pull request Oct 19, 2016

[backport] PR #8746 to 5.x - Printing process warning's via the appropriate loggers #8757

Merged

kobelb deleted the nodejs-warnings branch October 19, 2016 18:58

kobelb added a commit that referenced this pull request Oct 19, 2016

Merge pull request #8755 from elastic/jasper/backport/8746/5.0

4f2ba78

[backport] PR #8746 to 5.0

kobelb added a commit that referenced this pull request Oct 19, 2016

Merge pull request #8757 from elastic/jasper/backport/8746/5.x

1171461

[backport] PR #8746 to 5.x

epixa added v5.1.1 and removed v5.1.0 labels Dec 8, 2016

epixa mentioned this pull request Dec 22, 2016

[4.6] Printing process warning's via the appropriate loggers #9610

Merged

epixa added the v4.6.4 label Dec 22, 2016

epixa mentioned this pull request Dec 22, 2016

"Error: write ECANCELED" in logs, after refreshing a page #9609

Closed

epixa added a commit that referenced this pull request Dec 22, 2016

server: do not treat logged warnings as errors (#8746) (#9610)

9c54454

Node 6.6+ writes warnings to stderr rather than stdout, but we want those warnings to piped through as warnings in our Kibana logger instead.

airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017

Merge pull request elastic#8757 from elastic/jasper/backport/8746/5.x

163ac5d

[backport] PR elastic#8746 to 5.x Former-commit-id: 1171461

tylersmalley mentioned this pull request Mar 29, 2018

Prevent logging deprecation warnings in production #17457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Printing process warning's via the appropriate loggers #8746

Printing process warning's via the appropriate loggers #8746

Uh oh!

kobelb commented Oct 19, 2016 •

edited

Loading

Uh oh!

kobelb Oct 19, 2016 •

edited

Loading

Uh oh!

spalger Oct 19, 2016

Uh oh!

kobelb Oct 19, 2016

Uh oh!

spalger Oct 19, 2016

Uh oh!

kobelb Oct 19, 2016

Uh oh!

spalger Oct 19, 2016 •

edited

Loading

Uh oh!

kobelb commented Oct 19, 2016

Uh oh!

spalger Oct 19, 2016 •

edited

Loading

Uh oh!

spalger Oct 19, 2016

Uh oh!

spalger Oct 19, 2016

Uh oh!

spalger Oct 19, 2016

Uh oh!

tylersmalley left a comment

Uh oh!

jbudz commented Nov 28, 2016

Uh oh!

kobelb commented Nov 28, 2016

Uh oh!

Uh oh!

		@@ -0,0 +1,9 @@
		export default function (kbnServer, server, config) {

Printing process warning's via the appropriate loggers #8746

Printing process warning's via the appropriate loggers #8746

Uh oh!

Conversation

kobelb commented Oct 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kobelb Oct 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spalger Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

kobelb Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

spalger Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

kobelb Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

spalger Oct 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobelb commented Oct 19, 2016

Uh oh!

spalger Oct 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spalger Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

spalger Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

spalger Oct 19, 2016

Choose a reason for hiding this comment

Uh oh!

tylersmalley left a comment

Choose a reason for hiding this comment

Uh oh!

jbudz commented Nov 28, 2016

Uh oh!

kobelb commented Nov 28, 2016

Uh oh!

Uh oh!

kobelb commented Oct 19, 2016 •

edited

Loading

kobelb Oct 19, 2016 •

edited

Loading

spalger Oct 19, 2016 •

edited

Loading

spalger Oct 19, 2016 •

edited

Loading