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

Normalize the logging messages for easier filtering in kibana #180

Merged
merged 2 commits into from
Feb 8, 2018

Conversation

Pchelolo
Copy link
Contributor

@Pchelolo Pchelolo commented Feb 8, 2018

The additional information has been moved to separate fields in logs,
like this we can easily filter out mesages about worker deaths and
filter a sequence of events that lead to the death of a particular
worker.

Bug: https://phabricator.wikimedia.org/T186761
cc @wikimedia/services @berndsi @mdholloway

The additional information has been moved to separate fields in logs,
like this we can easily filter out mesages about worker deaths and
filter a sequence of events that lead to the death of a particular
worker.

Bug: https://phabricator.wikimedia.org/T186761
lib/master.js Outdated
&& this._firstWorkerStartupAttempts++ >= STARTUP_ATTEMPTS_LIMIT) {
// We tried to start the first worker 3 times, but never succeed.
// Give up.
this._logger.log('fatal/service-runner/master',
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to have the exit code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@d00rman done

@d00rman
Copy link
Contributor

d00rman commented Feb 8, 2018

LGTM modulo small nit

@d00rman d00rman merged commit 6e27533 into wikimedia:master Feb 8, 2018
@Pchelolo
Copy link
Contributor Author

Pchelolo commented Feb 8, 2018

Tagged and published as service-runner@2.5.1

@d00rman
Copy link
Contributor

d00rman commented Feb 8, 2018

That was fast!

@berndsi
Copy link
Contributor

berndsi commented Feb 8, 2018

@Pchelolo I think you wanted to use msg instead of message. I'm getting duplicate message fields:

{"name":"service-mobileapp-node","hostname":"box","pid":6131,"level":30,"message":"worker shutting down","worker_pid":6131,"levelPath":"info/service-runner/worker","msg":"worker shutting down","time":"2018-02-08T16:36:24.717Z","v":0}

Also, it looks like the pid was already there: pid and worker_pid have the same value. If one was the master pid then it would make sense to have two.

@Pchelolo
Copy link
Contributor Author

Pchelolo commented Feb 8, 2018

Oh holy hell I thought we've fixed the msg/message thing. Will fix.

On the worker_pid thing - this particular log entry is from the worker, so yes, the worker pid will match with pid. but In case you want to filter logs to know the whole sequence of logs coming from a specific worker - including the worker_pid is helpful cause some of the messages will be coming from the master where the pid is different.

wmfgerrit pushed a commit to wikimedia/mediawiki-services-mobileapps that referenced this pull request Feb 8, 2018
Changed one log message in our code to drop the pid from the message
string since it is already included in the `pid` field.

Changed the string also to start with lower case to be consistent with
the change in wikimedia/service-runner#180.
FWIW, updated the version in package.json. That is mostly for
documentation purposes since we would get that update automatically
anyways.

Change-Id: I1406cfb479f0d689ed4b56b467dc5328a70d68a5
@berndsi
Copy link
Contributor

berndsi commented Feb 9, 2018

Then it's ok to keep the worker_pid. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants