Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[ETCM-107] Backport logging configuration #667

Merged
merged 1 commit into from
Sep 17, 2020

Conversation

kapke
Copy link
Contributor

@kapke kapke commented Sep 14, 2020

Description

Backport Logging configuration to allow:

  • logs in JSON (currently disabled, cc @mirkoAlic)
  • setting logs configuration within application.conf
  • automatic adjustment of logs placement depending on datadir setting (important for Luna)

Important changes introduced

This PR changes logging configuration in a way that may affect deployments. How can I check whether that's true?

Testing

  1. Check whether logs are in same place as previously
  2. Check whether logs follow settings under mantis.logging

@kapke kapke added the BREAKS CONFIG Affects the default configuration label Sep 14, 2020
@kapke kapke requested review from mmrozek and mirkoAlic September 14, 2020 11:06
@kapke kapke self-assigned this Sep 14, 2020
<appender-ref ref="FILE" />
<appender-ref ref="METRICS" />
Copy link
Contributor

Choose a reason for hiding this comment

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

should be included now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not! I did remove appender but forgot to remove ref to it

Copy link
Contributor

@mirkoAlic mirkoAlic left a comment

Choose a reason for hiding this comment

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

(only one minor comment/doubt) LGTM!

@@ -536,6 +536,17 @@ mantis {
# Iff true, any errors during metrics client operations will be logged.
log-errors = true
}

logging {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we expose this config in the universal folder also? Maybe the file logging.conf will be useful.

@kapke kapke force-pushed the etcm-107-backport-logging-configuration branch 2 times, most recently from 94797d5 to e1a6002 Compare September 16, 2020 12:46
Copy link
Contributor

@mmrozek mmrozek left a comment

Choose a reason for hiding this comment

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

LGTM

@kapke kapke force-pushed the etcm-107-backport-logging-configuration branch from e1a6002 to ab4348f Compare September 17, 2020 11:06
@kapke kapke force-pushed the etcm-107-backport-logging-configuration branch from ab4348f to 4004d8a Compare September 17, 2020 11:20
@kapke kapke merged commit a51e27b into develop Sep 17, 2020
@kapke kapke deleted the etcm-107-backport-logging-configuration branch September 17, 2020 12:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
BREAKS CONFIG Affects the default configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants