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

Only dump config in debug mode #1508

Closed
wants to merge 1 commit into from
Closed

Conversation

ruudk
Copy link

@ruudk ruudk commented Apr 10, 2020

Doesn't make sense to dump this in normal info level:

INFO[0000] config: {Master: KubeConfig: RequestTimeout:30s .... }

Doesn't make sense to dump this in normal info level.

Signed-off-by: Ruud Kamphuis <ruudk@users.noreply.github.com>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 10, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ruudk
To complete the pull request process, please assign hjacobs
You can assign the PR to them by writing /assign @hjacobs in a comment when ready.

The full list of commands accepted by this bot can be found 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 added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 10, 2020
@ruudk
Copy link
Author

ruudk commented Apr 10, 2020

/assign @hjacobs

@jgrumboe
Copy link
Contributor

I find it very useful.
And since it is only once at startup it doesn't spam logs.
I would leave it at info level.

@ruudk
Copy link
Author

ruudk commented Apr 11, 2020

This is an internal Go struct dumped into the log. It’s not human readable.

If you want this info, you can:

  • use debug log level
  • or describe the pod to see its arguments (config)
  • or create a PR to properly show the config in human readable way

@jgrumboe
Copy link
Contributor

jgrumboe commented Apr 11, 2020

Do you really think it's not human-readable? It's a list of key:values (json-like) which is fine in my opinion.

@ruudk
Copy link
Author

ruudk commented Apr 11, 2020

Yes, it’s very difficult for me to read through huge blobs like that, especially since it’s not formatted with newlines and indentations.

@jgrumboe
Copy link
Contributor

jgrumboe commented Apr 11, 2020

Ok. Got your point.
I could imagine a human readable format (newlines, indentation,...) just printing the relevant global settings (including defaults) and the ones for the configured source and provider in info-level at startup.
Leaving all nonconfigured sources, providers, etc. in the existing dump at debug level.

@Raffo
Copy link
Contributor

Raffo commented Apr 11, 2020

I agree that it is useful and being printed only at startup, not a problem. I would welcome a prettier print of that config.

@Raffo
Copy link
Contributor

Raffo commented May 8, 2020

This has been open long enough and it seems there is no interesting in adding the prettier print. Given that the current print is useful, I'm going to close this PR.

@Raffo Raffo closed this May 8, 2020
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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants