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

Add --log-format server flag to configure output format #73

Merged

Conversation

phillipj
Copy link
Contributor

These changes allows the logger format to be specified when starting the
server, either to text (default) or json.

I'm also interested in adding support for the fluentd format, though that would mean adding a 3rd party dependency: https://github.com/joonix/log.

Any thoughts about the changes as is and/or possible new dependency?

Refs #52

Copy link
Contributor

@nckturner nckturner left a comment

Choose a reason for hiding this comment

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

This is fine with me as is, but I'd prefer not to add the other dependency you mentioned. (Sorry about the long wait for feedback). You'll need to sign the linuxfoundation CLA now though, to get this merged since we moved to kubernetes-sigs.

@phillipj
Copy link
Contributor Author

Thanks for reviewing! You not wanting to add a new dependency for this is totally understandable, no problem for me dropping that idea.

Pretty sure I got that CLA signed now, at least that's what I interpret from the verification email titled You just signed CNCF Individual Contributor License Agreement. The inline status in this PR hasn't updated though, but that might take a little while? 🤷‍♀️

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 27, 2018
@nckturner
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 27, 2018
@nckturner
Copy link
Contributor

Can you rebase @phillipj?

These changes allows the logger format to be specified when starting the
server, either to `text` (default) or `json`.

Signed-off-by: Phillip Johnsen <johphi@gmail.com>
@phillipj phillipj force-pushed the configurable-log-formatter branch from a889142 to fb5a6de Compare July 3, 2018 18:23
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 3, 2018
@phillipj
Copy link
Contributor Author

phillipj commented Jul 3, 2018

Sure thing, rebased and force pushed -- conflicts were trivial enough that a 3-way merge did the trick.

@nckturner
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 3, 2018
@nckturner nckturner merged commit a5bb28c into kubernetes-sigs:master Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants