Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#18922 closed defect (fixed)

configure logging via properties file

Reported by: iwakeh Owned by: iwakeh
Priority: Medium Milestone: CollecTor 1.0.0
Component: Metrics/CollecTor Version:
Severity: Normal Keywords: ctip operation
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

CollecTor's logging should be configured using a properties file, which could be reconfigured by operators as needed.

The class LoggingConfiguration should be removed.

Child Tickets

Change History (7)

comment:1 Changed 4 years ago by iwakeh

This is a defect, b/c the current configuration doesn't work as designed, for example many "sun" logging statements of level FINE are logged, but shouldn't.

comment:2 Changed 4 years ago by iwakeh

Keywords: operation added

comment:3 Changed 4 years ago by iwakeh

Status: newneeds_review

I replaced the hard-coded logging config by configuration via with logging.properties;
shell scripts are adapted to only having to change one location;
and a hint about how to configure logging in even more detail is added to run-all.
The default is to log into one log file; changes can be made by configuration now.

Please review branch task-18922.

The logging replacement (i.e. logback, slf4j, etc) will be discussed and implemented in ticket #19015.

Last edited 4 years ago by iwakeh (previous) (diff)

comment:4 Changed 4 years ago by karsten

Few questions before merging:

  • Can we have separate log files for each module? These modules may be executed concurrently, and using a single log file might cause problems and also make the log file less readable.
  • Can we merge bin/logging-properties with the new config file, or would that confuse the Java Logging API? What about future logging frameworks we're going to switch to?
  • Would you want to remove the five files bin/run-* and only keep the new bin/run-all that accepts a module name as parameter? Or is that a change we should postpone?

comment:5 in reply to:  4 Changed 4 years ago by iwakeh

Replying to karsten:

Few questions before merging:

  • Can we have separate log files for each module? These modules may be executed concurrently, and using a single log file might cause problems and also make the log file less readable.

Yes. The way to have different log-files is hidden in a comment in run-all:

# this would use a special log config for each module
# java -Xmx2g -Djava.util.logging.config.file=./bin/logging.$TOKEN.properties -jar collector-0.9.0-dev.jar $TOKEN

This makes each module use different logging properties, i.e.
logging.relaydescs.properties for relaydescs,
logging.bridgedescs.properties for bridgedescs, etc.
These logging properties can just be a copy of logging.properties each with a different entry for the log file name, but of course also other differences (level or excluded packages ...)

  • Can we merge bin/logging-properties with the new config file, or would that confuse the Java Logging API? What about future logging frameworks we're going to switch to?

Well, if I understand the question correctly: that I would have to try in order to give a definite answer. Usually java util logging ignores what it doesn't understand, but I would not rely on that.

Logging should be separated from the runtime configuration. And, because of the upcoming change of logging frameworks I wouldn't want to merge the logging properties with other configuration.

For a switch to slf4j and logback, for example, we would remove all java util logging properties and have an XML logging config like for Onionoo. In that case, just one file for all the different modules and logging purposes (i.e. monitoring, diagnostics, and statistics).

  • Would you want to remove the five files bin/run-* and only keep the new bin/run-all that accepts a module name as parameter? Or is that a change we should postpone?

Well, I intended to make things backward compatible by keeping the old shell scripts. So the crontab entries don't need to be touched.
And, currently run-all expects the old shell-script name (incl. the run- part) and extracts the module name (see variable TOKEN).

comment:6 Changed 4 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Alright. I'm not fully convinced that having up to five logging properties files in addition to a separate config file is the most convenient way to do this, but I don't want to block this until we have a better logging framework and start script in place. I just merged your branch as is, also because it's clearly better than the current hard-coded stuff. Thanks! Closing.

comment:7 Changed 4 years ago by iwakeh

Milestone: CollecTor 1.0.0

Added to milestone for first release.

Note: See TracTickets for help on using tickets.