Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐛 BUG]: Failed logging by file_logger_options hide permission error #1708

Closed
1 task done
Kaspiman opened this issue Sep 5, 2023 · 9 comments
Closed
1 task done
Assignees
Labels
B-bug Bug: bug, exception F-need-verification

Comments

@Kaspiman
Copy link
Sponsor

Kaspiman commented Sep 5, 2023

No duplicates 🥲.

  • I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

Maybe bug, i'm not sure

Version (rr --version)

rr version 2023.2.0 (build time: 2023-07-06T19:02:11+0000, go1.20.5), OS: linux, arch: amd64

How to reproduce the issue?

Let's say we have application in Docker container with volume for dev purpose. Log file permissions can be changed due to volume synchronization: uid:guid inside and outside has different values.

docker-compose fragment:

services:
    php:
        volumes:
            - ./:/app

RR logs fragment:

logs:
    mode: raw
    level: debug
    output: stderr
    file_logger_options:
        log_output: rr.log

The file rr.log will be created, but the owner will be changed almost immediately. Nothing will be written to the file, nor will it be written to the output. Silence.

Previous logging settings are ignored if file_logger_options are specified.

Maybe there should be fallback to stdout or mixed logging with the previously specified parameters (output, level, mode): zapcore.NewMultiWriteSyncer(zapcore.AddSync(os.Stdout), w)

Relevant log output

No response

@Kaspiman
Copy link
Sponsor Author

Kaspiman commented Sep 5, 2023

There another "bug" with log. If I delete a file it won't be created automatically natefinch/lumberjack#52

@rustatian
Copy link
Member

Hey @Kaspiman 👋🏻
Thanks for the report 👍🏻

@Kaspiman
Copy link
Sponsor Author

Kaspiman commented Sep 5, 2023

@rustatian I found another problem with lamberjack library. Default file permission 600 is too strict, we can't easy read logs by filebeat. natefinch/lumberjack#168 after this MR we can change permission to 644. At this moment, the file_logger_options functionality is not ready for production use.

The package looks abandoned natefinch/lumberjack#170

@rustatian I have another question: is it possible to reliably rotate files with config:

logs:
   output: rr.log

During rotation, a file can be deleted and re-created. Will the logs disappear at this time?

@rustatian
Copy link
Member

I found another problem with lamberjack library

This package is quite small. I may fork it, add it to the RR organization, and continue to maintain it.

I have another question: is it possible to reliably rotate files with config:

Should be possible, but you have to track the file size by yourself.

@Kaspiman
Copy link
Sponsor Author

@rustatian It would be nice to maintain that project. I can share information about the new fork among users of the old library. This will help attract more attention to Roadrunner

@rustatian
Copy link
Member

@Kaspiman Done: https://github.com/roadrunner-server/filelogger
I'll spruce it up a bit, update go.mod, etc. Then it would be able to move unmerged PR's here.

@rustatian
Copy link
Member

Oh my, there are a lot of races in this package...

@Kaspiman
Copy link
Sponsor Author

A journey of a thousand miles begins with a single step

@rustatian
Copy link
Member

I've decided to move towards the official log/slog link package and deprecate/remove logger from Uber.
This would be a wholly new filelogger package. As the result, this bug is no more actual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-bug Bug: bug, exception F-need-verification
Projects
Status: ✅ Done
Development

No branches or pull requests

2 participants