search

spdfg / elektron

Elektron is a lightweight, power-aware, pluggable Mesos framework that behaves as a playground to experiment with different scheduling policies to schedule ad-hoc jobs in docker containers.
GNU General Public License v3.0
4 stars 3 forks source link

WIP : Elektron Logging library #16

Closed balandi1 closed 4 years ago

pradykaushik commented 4 years ago

@ridv this PR corresponds to retrofitting code to use logrus for logging and get rid of the built-in logging library.

pradykaushik commented 4 years ago

@balandi1 please also make a PR to elektron-vendor with the updated dependencies to make sure that dependencies are kept up to date.

pradykaushik commented 4 years ago

Just checked the code and made a test run.

  1. Seems like the log prefix provided using the -logPrefix commandline option is not used when creating the log directory.
  2. Color codes should be removed from the ESC[32;1m[INFO]:ESC[0m prefix on each line in the logs.
  3. PCP log file has ESC[32;1m[INFO]:ESC[0m prefix on each line. PCP logs should not have this prefix and should just have the timestamp and the message. One way of making this generic is by supporting overriding of log config for each element in the chain. For example, PCP logs are to be persisted in a different format than the universal format specified for all others.
  4. If a log type is disabled in (specifying enabled: false in the logConfig yaml file), then the corresponding log file should not be created. This would result in no log file being empty.
ridv commented 4 years ago

Thanks @balandi1 .This was a big undertaking and I really appreciate the effort you're putting towards getting it merged. Looks good to me as soon as all of @pradykaushik 's concerns are addressed. Very excited to have you be part of the list of contributors once this is merged.

pradykaushik commented 4 years ago

Overall, LGTM. I have mentioned a few nit picks plus a formatting related fix. Once those are fixed, I think this is ready to be merged in. Thank you for undertaking a non-trivial task. This was an important addition to elektron.