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

Log peer config upon startup (#4118) #4122

Merged
merged 1 commit into from Mar 31, 2023

Conversation

denyeart
Copy link
Contributor

Log peer config upon startup.
Also log peer environment variables to help troubleshoot peer config.

Log peer config upon startup.
Also log peer environment variables to help troubleshoot peer config.

Signed-off-by: David Enyeart <enyeart@us.ibm.com>
@denyeart denyeart requested a review from a team as a code owner March 29, 2023 12:34
logger.Infof("Peer config with combined core.yaml settings and environment variable overrides:\n%s", settingsYaml)

// Debug logging for peer environment variables
logger.Debugf("Environment variables:")
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry Dave, a nit: any specific reason why we use debug for this but info for the previous?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was debating this myself. In my opinion the exact peer config should be easily knowable by every user and support engineer hence Info level. This is consistent with what orderer node provides but for some reason peer never provided it.
The wider set of environment variables may be less important and potentially could leak sensitive information about the operating environment hence Debug level.
However, showing the environment variables will help a user or support engineer know whether a certain config setting came from core.yaml or from an environment variable override, so it is valuable information when troubleshooting a peer config.
I could go either way, WDYT?

@celder628
Copy link

celder628 commented Mar 30, 2023

A couple of notes.

  1. I have to agree that INFO is the best way of dumping the variables in the log. This prints unambiguously and is a big help to anyone who is reviewing logs. Especially if they are not familiar with the variables being used (support).

  2. The precedence here is also the Ordering nodes which print all variable with INFO at startup.

@denyeart
Copy link
Contributor Author

I talked to celder628, he was talking about the effective config being printed at INFO level in the prior comment. He said that for the raw environment variables DEBUG is sufficient. So I think I'll keep the PR as-is.

@manish-sethi manish-sethi merged commit dfc06c6 into hyperledger:release-2.2 Mar 31, 2023
12 checks passed
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.

None yet

4 participants