Provide rotation for log files

[denis-konovalyenko]

Currently, there is no log rotation option available and if the --verbose option is turned on, it is only possible to overview about 2 hours interval.

It would help to spot events happening in the system at a wider period of time. For instance, if I had a Ruby process core dump, saying 24 hours ago, I would see what were the preceding log entries.

[0crat]

@yegor256/z please, pay attention to this issue

[0crat]

@denis-konovalyenko/z this project will fix the problem faster if you donate a few dollars to it; just click here and pay via Stripe, it's very fast, convenient and appreciated; thanks a lot!

[yegor256]

@denis-konovalyenko make sense, thanks

[0crat]

Job #610 is now in scope, role is DEV

[0crat]

Bug was reported, see §29: +15 point(s) just awarded to @denis-konovalyenko/z

[yegor256]

@0crat assign @emilianodellacasa

[yegor256]

@emilianodellacasa let's add --rotate-logs option. When it's set, the node.rb should not delete old logs, but rename the file, according to the current time. The suffix has to be added, like -27-11-2018.log

[0crat]

@0crat assign @emilianodellacasa (here)

@yegor256 The job #610 assigned to @emilianodellacasa/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this; there will be a monetary reward for this job

[0crat]

Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @yegor256/z

[denis-konovalyenko]

@yegor256 , @emilianodellacasa , I'd rather have 20181127 suffix. That would help to find a required log file faster if they are sorted by name.

[0crat]

@emilianodellacasa/z this job was assigned to you 5days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

[emilianodellacasa]

@0crat wait - going on vacation for a few days

[0crat]

@0crat wait - going on vacation for a few days (here)

@emilianodellacasa The impediment for #610 was registered successfully by @emilianodellacasa/z

[yegor256]

@0crat assign @v-kolesnikov

[0crat]

@0crat assign @v-kolesnikov (here)

@yegor256 The job #610 assigned to @v-kolesnikov/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this; there will be a monetary reward for this job

[0crat]

Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @yegor256/z

[v-kolesnikov]

@yegor256 So, I took a look at this issue and related code. Well... I see some key points in which the changes may be made. But I would ask you before:

• Why don't just log all the things to STDOUT by default and do nothing anymore?
  In case if node owner wants to keep node logs it can be achieved by platform's capabilities on which the node is running. It is a simple, lightweight, UNIX-like and 12factor-complience way, why not? Of course, I see that there is nohup mode in which we fork the main process and manually write it's STDOUT stream to our nohup-log file. But why don't just use Kernel.spawn? It will share parent process OUT/ERR streams with its children. For example:

  # stdout.rb
  if ENV['CHILD']
    STDOUT << "Some STDOUT message\n"
    STDERR << "Some STDERR message\n"
  else
    Process.wait Kernel.spawn({ 'CHILD' => '1' }, "ruby #{File.expand_path($PROGRAM_NAME)}")
  end

  $ ruby stdout.rb
  Some STDOUT message
  Some STDERR message

  Writing logs to files is a bad practice in case of containers. If we add --rotate-log option «as is», there will be a vulnerability of disk overflow or docker IO limits. And it isn't the latest problem which manual writing log files brings, I'm just highlighting the biggest of them.

• How --rotate-log and nohup-log-truncate options should be combined? Usually it is mutual exclusive options. The standard Ruby library module Logger supports log-rotation with both options, but these are mutual exclusive as well. So in case of daily rotation the code might look like that:

      # Log facility for nohup
      class NohupLog
        def initialize(file, rotation_age = 'daily')
          @logger = Logger.new(file, rotation_age)
        end
  
        def print(data)
          @logger << data
        end
      end

[denis-konovalyenko]

@yegor256 , @v-kolesnikov , I think it would be nice to have the logging to the STDOUT.

However, if this is not the way to go, then it would probably be all right to relax the --nohup-log-truncate option default limit of 1024 * 1024 a bit (how about 1024 * 1024 * 64?) and let users configure what they want (data volumes or logging services).

[yegor256]

@denis-konovalyenko it's better to ask questions in separate tickets. If you have suggestions for design improvements, submit new tickets.

[denis-konovalyenko]

@yegor256 , do I understand you correctly that I should ignore any further questions/discussions here despite the fact that the issue was created by me?

[yegor256]

@denis-konovalyenko you should only focus on the discussion relevant to your question. Actually, you should not discuss, but protect your idea. Check this article: https://www.yegor256.com/2014/11/24/principles-of-bug-tracking.html We are not here to have a nice chat. We are here to decide who is right, you or @v-kolesnikov. Your job is to prove that you are right. Everything else, which is not relevant to your question, should be ignored here and submitted as new tickets.

[denis-konovalyenko]

@yegor256 , thank you for your reply and the reference for reading!

Actually, I do not care who is right or wrong, as the only meaningful thing for me is the user experience we are trying to improve or not here. And if someone else offers a better alternative, why should I be so stubborn and prevent that alternative from going live?

Once again, I do not quite get what was irrelevant in my message exactly? Did I not formulate it clear enough and there was some room left for taking the final non-disputable decision by the architect (sorry, that was too indirect I think)?

Moreover, since the ticket was opened a long time ago, I have realized that there is an even simpler way to address my initial concern (it is only possible to overview about 2 hours interval) by just documenting the already existing --nohup-log-truncate option, as long as logging to the STDOUT is already available out of the box, when the --nohup option is not specified.

@v-kolesnikov , could you please provide the aforementioned piece of documentation for the --nohup-log-truncate option and that would be it?

[v-kolesnikov]

I think it would be nice to have the logging to the STDOUT.

I got a point of @yegor256 and will defense "logging to STDOUT" in separated issue.

could you please provide the aforementioned piece of documentation for the --nohup-log-truncate option and that would be it?

@denis-konovalyenko What kind of documentation you mean? Option is documented in the default style (as the code around): https://github.com/zold-io/zold/pull/755/files#diff-68c77400c4ecc5cde2ae1bb977f754d1R120

Do you have any question about my current solution? If not, let's merge PR and close the issue please.

[denis-konovalyenko]

@v-kolesnikov , I meant to make a note on that somewhere in the scope of the How to start a node README section.

As for your solution, I think I am not a reviewer of your pull request (By the way, have you forgotten to mention this issue in your pull request? If you make so, your PR will be visible here then).