-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 two log output modes, stdout-full
and stdout-new-only
#3439
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Ignored Deployments
|
@rafaeltab is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @rafaeltab! The implementation and tests look great, but I don't love the user-facing API.
- "stdout" is inaccurate, as it will stream
stderr
also - Adding
stdout-*
options means that we need 2n options for each "filter view" of logs.
What do you think instead about adding a new flag --pretty
and making it on by default? Then, --output-logs=stdout-full
would be the same as --output=logs=full
, etc. Some advantages of this:
- we don't need to capture what turbo means by pretty output, we can do prefixes, colors, or something else in the future
- following
git log --pretty
, in the future we could enable something like--pretty='%some-micro-syntax-for-fine-grain-control%'
I'm not sold on this idea, since adding a new flag adds some overhead, but I hope this gives us something to talk about!
I agree, the name stdout doesn't really convey the idea that prefixes are removed either. A pretty flag could cover this, however, we do not want to make this too complex either. Perhaps some baked-in pretty configurations, with the option to go custom would be best. Some ideas that come to mind are
If we were to go for this option this would require a rewrite for a large portion of the logging system. This would also allow the current stdout behavior to be combined with errors-only, which was left out in this PR. |
I think we can start with only true/false (defaulting to true). My suggestion to expand the flag was to demonstrate how cc @gsoltis / @vercel/turbo-oss to help make a decision/bikeshed here as well. One more thing to point out is that without prefixing, logs for tasks running in parallel will be mixed up. The prefixes are the only thing that let you differentiate between them right now. That may be a reason to add |
I was absolutely not planning to rewrite logging just yet, the 3 possible options I could implement all represent either the no prefix logging or the normal logging, so the current implementation could almost completely work for them. Then when you guys decide you want to go all the way we won't have any backwards compatibility issues. If you still prefer a simple boolean, that is also still an option. Would that be |
The logs will indeed be mixed up, and |
How about a separate boolean flag like |
@gsoltis that would absolutely cover what we need. I think it's more a question of what do we want to add in the future when it comes to logging, and what do we think looks and feels better. I believe Do we want more flexibility, or de we want more clarity? |
I see. Let's go with |
Can we take over this PR? |
I have been a bit busy recently (I will be moving out of my parents' home to live with a friend of mine), I will continue development for this PR this week. |
Hey @rafaeltab thanks for your work on this. We talked internally and landed on |
Alright, it's understandable since I hadn't worked on it for a little while. The naming choice seems straightforward as well. I'll close this PR then. I'll probably end up picking up grouped output in the near future. |
Add two log output modes,
stdout-full
andstdout-new-only
The new
stdout-full
mode will not prefix the output generated by the commands run by Turborepo. It will still prefix turbos messages, such as cache hit, cache miss, and cache bypass. This allows the user to identify what was and wasn't cached while preserving the original output.The new
stdout-new-only
mode combines the behavior ofnew-only
andstdout-full
by only logging non-cached tasks, without prefixing the output.stdout-full
Command:
turbo run build --output-logs=stdout-full --filter=util --force
Output:
stdout-new-only
Command:
turbo run build --output-logs=stdout-new-only --filter=util
Output:
Command:
turbo run build --output-logs=stdout-new-only --filter=util --force
Output:
Side notes
I also added some integration tests for the
output-logs
optionCloses #901