Use special `logger` handler instead of `error_logger` for `report.log`

[seriyps]

This makes it possible to run yaws when no error_logger process running, since we don't try to install custom handlers in it.

In #442 I initially made a bit more ambitious plan - to use OTP logger for access and auth logs. But it seems this won't be necessary to untie Yaws from relying on error_logger process being started and being gen_event. Seems Yaws only relied on error_logger process for report.log. I just added an (optional) logger handler to replicate the behaviour of report.log.

Yaws still starts error_logger via yaws_dynopts:start_error_logger(), but only does that when there is no error_logger already configured. So, one can pre-define different version of OTP logger's error_logger handler (eg, logger:add_handler(error_logger, logger_std_h, #{..}) before Yaws is started (eg, via sys.config). And then legacy error_logger gen_event won't be started.

Not sure if I'll do the follow-up migrating access/auth loggers to OTP logger, it might be non-optimal solution because OTP logger does O(n) lookup to choose to what logger handlers to log while current solution uses O(1) ETS lookup, so, would work better when there are many virtual hosts.

[vinoski]

This seems to have problems on OTP 21.x, as it appears that logger_std_h can't be configured the same as it can on OTP 22 and newer. It fails on 21.0 through 21.2.

If you build using make you can then run the following to see one of the failures:

make check SUITES=copy_error_log_SUITE CASES=error_log_handler

[seriyps]

Oups. I actually did run the test suite, but only did so on single OTP version. So would it be fine if we don't do the log rotation on the whole OTP-21? Looks like it won't be easy to allow it on OTP-21.2 but not OTP-21.1/21.0, because ?OTP_RELEASE is "21" for all of them.

Also, I can try to create another PR migrating from .travis.yml to GitHub CI, so it would be easier to catch such problems automatically.

[seriyps]

Alternatively I can check that application:get_key(kernel, vsn). is 6.3+ if it won't deadlock in application_controller

[vinoski]

Still looking at this branch, but wanted to answer this:

Also, I can try to create another PR migrating from .travis.yml to GitHub CI, so it would be easier to catch such problems automatically.

That would be great! I started working on this in the switch-to-gha branch months ago, but my day job takes quite a lot of my time and I just never got back to it. Feel free to use that branch as a starting point if that's helpful.

[vinoski]

Alternatively I can check that application:get_key(kernel, vsn). is 6.3+ if it won't deadlock in application_controller

I prefer to do these kinds of checks in yaws_dynopts, so in my local copy of your branch, I've made that change and it works fine.

But regarding the lack of rotation on versions 21.0 through 21.2, I'd really prefer to avoid that since it's basically a silent breakage. Would it be possible to use logger_disk_log_h for those versions?

[vinoski]

Would it be possible to use logger_disk_log_h for those versions?

This won't really work, as it seems that disk_log turns a filename like "report.log" for a wrap log into 3 files:

• report.log.N
• report.log.idx
• report.log.siz

[vinoski]

Would it be possible to use logger_disk_log_h for those versions?

This won't really work, as it seems that disk_log turns a filename like "report.log" for a wrap log into 3 files:

• report.log.N
• report.log.idx
• report.log.siz

I'm now thinking the approach should be to just raise the minimum OTP version that Yaws supports up to 21.3. Then we won't need any special handling. Anyone still needing 21.0-21.2 support can use Yaws 2.1.0, which I think is fine since 1) it's very recent and 2) anyone actually using Yaws from anything newer than 2.1.0 is likely on a newer OTP release too.

I'll make the changes in your branch for this.

[seriyps]

I'm totally fine with raising the version requirement.

Regarding GitHub CI - I'll have a look at it :+1:

[vinoski]

I've made the changes we discussed, then squashed everything and rebased to master. Thanks!

[seriyps]

Hi. Would it be possible to cut a new minor release that includes this channge? Thanks in advance!

[vinoski]

Hi. Would it be possible to cut a new minor release that includes this channge? Thanks in advance!

Yes. I'll try to get to it this week.

[seriyps]

Gently reminder :wave:

[vinoski]

I haven't forgotten, @seriyps !

I had a user contact me with a problem that might have been an SSL-related bug, so I wanted to hold off any new release just in case investigating it showed that a fix was needed. We determined last night that it was not a problem in Yaws, so now I can get back to making a new release.

[vinoski]

Yaws 2.1.1 released, thanks for your patience!

[seriyps]

Thank you!