-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 setting of an environment variable to configure master logs encoding #9296
Conversation
Add setting of env var CHE_LOGS_APPENDERS_IMPL that sets Che master logs producing configuration to deployments for: - docker (che.env) - kubernetes (kubectl) - kubernetes (helm) - openshift (scripts) Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com> Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
@perspectivus1 @guydaichs FYI |
# Choose variant of Che master logs encoding and storing. Default is `plaintext`. | ||
# Another supported value is `json` which makes Che master produce JSON encoded logs | ||
# in system output of PID 1 process of container. Useful for storing logs in systems such as Logstash. | ||
# CHE_LOGS_APPENDERS_IMPL=plaintext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't default value be added into che.properties?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not read from properties
# Choose variant of Che master logs encoding and storing. Default is `plaintext`. | ||
# Another supported value is `json` which makes Che master produce JSON encoded logs | ||
# in system output of PID 1 process of container. Useful for storing logs in systems such as Logstash. | ||
# CHE_LOGS_APPENDERS_IMPL=plaintext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if that is commented, the default value will be used, do we have default value declared in a code? because I do not see it in the che.props
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the default value is declared here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@l0rd is right this value is not read by the code and default value is in xml file of logback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this PR deserves a Doc PR too
@l0rd here is a PR for the docs eclipse-che/che-docs#384 |
@l0rd @riuvshin @sleshchenko @eivantsov I've merged the PR in che-lib so we can proceed with this one. Can you review the PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
Add setting of master logs encoding to che.env.
What issues does this PR fix or reference?
Related to #9275, eclipse-che/che-lib#83
Release Notes
Docs PR
eclipse-che/che-docs#384