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

Improve meta information in log messages to make it easier to debug jobs #765

Merged
merged 2 commits into from
Aug 1, 2018

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Jul 30, 2018

  • Use . as opposed to /; the former
    is more consistent with K8s style.

  • Add functions for constructing loggers for the pod and unstructured meta
    information. This will allow us to appropriately tag a number of log
    messages with meta information.

  • Update a bunch of log messages which weren't logging info with
    appropriate meta information.

  • Make json log formatting the default; this was the case for v1.
    json logging should be the default because otherwise we lose the meta
    information in the logs. With json logs its always possible to filter/reformat
    the log entries if you don't care about the metainformation.

Related to:
#24 Use logrus
#635


This change is Reviewable

…obs.

* Use <namespace>.<name> as opposed to <namespace>/<name>; the former
  is more consistent with K8s style.

* Add functions for constructing loggers for the pod and unstructured meta
  information. This will allow us to appropriately tag a number of log
  messages with meta information.

* Update a bunch of log messages which weren't logging info with
  appropriate meta information.

* Make json log formatting the default; this was the case for v1.
  json logging should be the default because otherwise we lose the meta
  information in the logs. With json logs its always possible to filter/reformat
  the log entries if you don't care about the metainformation.

Related to:
  kubeflow#24 Use logrus
  kubeflow#635
@jlewi jlewi changed the title Improve meta information in log messages to make it easier to debug j… Improve meta information in log messages to make it easier to debug jobs Jul 30, 2018
@jlewi
Copy link
Contributor Author

jlewi commented Jul 30, 2018

/assign @gaocegege
/assign @kunmingg

@TravisBuddy
Copy link

Travis tests have failed

Hey @jlewi,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

2nd Build

gometalinter --config=linter_config.json --vendor ./...
pkg/controller.v2/controller_logger.go:1::warning: file is not goimported (goimports)

3rd Build

gometalinter --config=linter_config.json --vendor ./...
pkg/controller.v2/controller_logger.go:1::warning: file is not goimported (goimports)

travis_time:end:03105330:start=1532966871058584678,finish=1532967006128977358,duration=135070392680

@coveralls
Copy link

coveralls commented Jul 30, 2018

Coverage Status

Coverage increased (+0.8%) to 57.476% when pulling d5fccf2 on jlewi:json_logging into e66b23d on kubeflow:master.

@gaocegege
Copy link
Member

/lgtm

@gaocegege
Copy link
Member

/retest

@jlewi
Copy link
Contributor Author

jlewi commented Aug 1, 2018

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 90259e7 into kubeflow:master Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants