Opened 9 months ago

Closed 8 months ago

#33549 closed enhancement (fixed)

Simplify logging configuration across code bases

Reported by: karsten Owned by: karsten
Priority: Medium Milestone:
Component: Metrics Version:
Severity: Normal Keywords:
Cc: metrics-team Actual Points:
Parent ID: Points:
Reviewer: irl Sponsor:


We're using different logging configurations in our code bases, some of which are broken and others are more complex than we need them to be. Some examples:

  • CollecTor has a shutdown console logger, but it's not being printed to anymore since we refactored package names.
  • CollecTor has five separate log files for the modules that existed back when we added those log files, but these are not being written to since we refactored package names. We also added more modules but did not update the logging configuration for them.
  • Onionoo and a few others have a separate error log file which logs only ERROR messages, but we're not doing anything with that file.
  • Some code bases have a separate statistics log file that we likewise do not pay any attention to.
  • All code bases rotate log files whenever they reach a size of 1 MiB, but they do not restrict the number or total size of log files being written. It would be easier to handle 1 file per day even if that grows to 100 MiB or more.
  • There could be a more reasonable default for the log directory than LOGBASE_IS_UNDEFINED.
  • It would be convenient to be able to configure the log level that doesn't require providing a full logging configuration, like -DLOGLEVEL=DEBUG.

Child Tickets

Change History (5)

comment:1 Changed 9 months ago by karsten

Status: assignedneeds_review

I have the beginning of a patch in commit fd85646 in my metrics-base branch task-33549 that I'd like to discuss here together with the general direction as laid out above.

Next steps after merging that metrics-base patch are to remove logback.xml files from other code bases, to remove logging code in all code bases to log to their class logger, and to test everything in local deployments.

comment:2 Changed 8 months ago by irl

Reviewer: irl

comment:3 Changed 8 months ago by irl

Status: needs_reviewmerge_ready

The configuration in the metrics-base branch looks good, this looks like a big win for reducing complexity, the plan looks good.

comment:4 Changed 8 months ago by karsten

Great! Pushed to metrics-base and updated all code bases except for ExoneraTor where I'd like to wait with making changes until #24542 is merged to avoid unnecessary merge conflicts.

comment:5 Changed 8 months ago by karsten

Resolution: fixed
Status: merge_readyclosed

The #24542 patch is merged, and I just made the remaining changes to ExoneraTor related to this ticket. That concludes this ticket. Closing. Thanks!

Note: See TracTickets for help on using tickets.