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

Add tracing logging #887

Closed
wants to merge 1 commit into from
Closed

Add tracing logging #887

wants to merge 1 commit into from

Conversation

Miloas
Copy link
Contributor

@Miloas Miloas commented May 12, 2023

This PR tried to add a logging library for better user and developer experience.
Only transformed all our println! with logging library.

Why tracing?
Rust compiler using it (rust-lang/rust#74726).
It gives us structured logging power, then we(also other service using nixpacks) can easily collecting and parse building logs. (expecially error logs)

@coffee-cup coffee-cup added the release/minor Author minor release label May 12, 2023
@coffee-cup
Copy link
Contributor

coffee-cup commented May 12, 2023

I'm not sure I agree with this PR change. I can see using a tracing library for warnings and errors. But for main logging it really changes what the output looks like

Now

image

Previous

image

IMO the reasons for switching (Rust uses it and other services can parse build logs) are not strong enough for us to switch since it decreases the quality of logs for users using Nixpacks on Railway or with the CLI.

@coffee-cup
Copy link
Contributor

Going to close this for the reasons listed above. I'm happy to move to a better more standardized logging system. But the quality of logs needs to go up, not down. The most important logs is the build plan at the top which shows how the app will be built.

@coffee-cup coffee-cup closed this May 12, 2023
@Miloas
Copy link
Contributor Author

Miloas commented May 12, 2023

@coffee-cup sorry I am not check the normal log. 🤔️ I got your means. Let me think how to do better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/minor Author minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants