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

Do not prefix log output #901

Closed
jpzwarte opened this issue Mar 18, 2022 · 25 comments
Closed

Do not prefix log output #901

jpzwarte opened this issue Mar 18, 2022 · 25 comments
Labels
area: ergonomics Issues and features impacting the developer experience of using turbo area: logging Improvements to logging good first issue Good for newcomers

Comments

@jpzwarte
Copy link

What version of Turborepo are you using?

1.1.7-canary.5

What package manager are you using / does the bug impact?

npm

What operating system are you using?

Mac

Describe the Bug

Screenshot 2022-03-18 at 15 37 50

All the task log lines are prefixed with the scope, causing the logs to be hard to read.

Expected Behavior

Please do not prefix every log line.

To Reproduce

turbo run build --scope=foo

@gsoltis gsoltis added good first issue Good for newcomers area: ergonomics Issues and features impacting the developer experience of using turbo labels Mar 24, 2022
@gsoltis
Copy link
Contributor

gsoltis commented Mar 24, 2022

This might be a good candidate for another option for log behavior

@Kingwl
Copy link
Contributor

Kingwl commented Apr 28, 2022

This might be a good candidate for another option for log behavior

What about UI or something like cache hit or cache miss?
And should we strip prefix if cache log in full mode but replay logs in content only (the new mode) ?

@JannesMeyer
Copy link

JannesMeyer commented Jun 1, 2022

My main use case for this would be to use it with VS Code problem matchers.

There are a bunch of built-in problem matchers (such as $tsc and $eslint-stylish) that parse compiler/linter output. None of them supports prefixed log lines.

@sprohaszka
Copy link

sprohaszka commented Jun 1, 2022

Hi!
I am willing to help you implement this feature :)

I have some difficulties to deep dive in the code base and know where we can remove the prefix. I tried to rough changing this line without any success.

Can you help me by pointing me out where to start? :)

Edit
I am not sure why but seems that this line has finally some impact 😅
So I am still digging from here

@gsoltis
Copy link
Contributor

gsoltis commented Jun 1, 2022

@sprohaszka There's going to be a little bit of complexity to this. The targetUi you've identified is one place. You'll also need to look at the LogStreamer instances just below that, as well as replaying the logs. Currently, the prefix is baked into the logs, which is not the best setup. That will either need to be stripped (easiest to do now), or logs updated to not have the prefix (larger change).

@rafaeltab
Copy link
Contributor

I can also pick this up, or help out if necessary.

@sprohaszka
Copy link

@gsoltis I tried to do something here.
I may miss some modification to improve the PR ^^

@sakekasi
Copy link

Just wanted to bump this issue. I'm using a custom pino logger that outputs JSON, and my parser gets tripped up by all the log prefixes

@sprohaszka
Copy link

If it make sense, I still work on my PR to improve it.

@sakekasi
Copy link

I'd definitely appreciate that!

@sprohaszka
Copy link

I'd definitely appreciate that!

I need to know what to improve 🙂
I did some modification last time. I don't know if it is enough 🙂

@rafaeltab
Copy link
Contributor

@sprohaszka I believe, though you have changed the code, you need to resolve the requested changes from gsoltis.
Normally there is a button somewhere to do this on the requested change.

@sakekasi
Copy link

FWIW, piping the output into this sed command:

sed -l -E 's/^@[a-zA-Z-]+\/[a-z-]+:[a-z-]+: //'

did the trick for me!

@nfantone
Copy link

nfantone commented Dec 5, 2022

What about not changing the logs at all? Would that be a possibility? Just forward the original streams to stdout.

Right now, by using turbo to run commands, we lose things like colouring, formatting and hyperlinks.

image

FYI, this is explicitly mentioned on the nx docs as one of its advantages over turbo.

Turborepo only uses piping to capture the terminal output. Piping doesn’t work well for the tasks emitting “interesting” output (cypress, webpack, etc). As a result, the terminal output with Turborepo and without it doesn’t look the same. Nx can use piping, but it also supports other strategies. As a result, Nx is able to capture the output “as is”.

@jrolfs
Copy link

jrolfs commented Jan 8, 2023

This was one of the few (only?) things I could not find an analog for in Turborepo when migrating from Lerna (which has --no-prefix). I prefer prefixed output by default for local development, but in GitHub Actions the prefix prevents annotations from working correctly.

@rafaeltab
Copy link
Contributor

rafaeltab commented Jan 8, 2023

I see this still has not been implemented, could I pick this up?

If so are the following requirements for this correct?

  • Stop adding prefixes to the stored logs.
  • Add an output-logs flag called no-prefix
  • Add prefix to replayed logs for full, hash-only, and new-only (I'll make sure to double check this list when implementing)
  • Only add the prefix when output-logs != no-prefix
  • Make sure this is also supported in the turbo.json file

An alternative would be to add a separate flag --no-prefix, this would allow the combining of settings such as new-only with no prefix.

@nfantone
Copy link

nfantone commented Jan 8, 2023

I can only speak for myself, but at least from my perspective "removing prefixes" wouldn't cut it. I'd like turbo to not modify output at all, preserving original colouring, line length, formatting, meta characters, etc. If really needed (i.e.: to detail currently running process in parallel executions), it could maybe add log lines of its own instead of hi-jacking others?

Apologies if this is what you had in mind already.

@rafaeltab
Copy link
Contributor

I personally don't know to what extent turbo removes coloring, meta characters, etc.
I know coloring can be forced with --color, other than that, I'll have to find it out during implementation.

@anthonyshew
Copy link
Contributor

I'm seeing interest for:

  • removing log prefixes
  • wanting stdout

In my estimation, making an output mode for stdout covers both concerns and with one swoop. I think that's the ticket!

@rafaeltab, your point to being able to use this new output mode along with others is great. What do we think of the new list of options being:

  • full | Displays all turbo output
  • hash-only | Show only the hashes of the tasks
  • new-only | Only show turbo output from cache misses
  • stdout-full | Displays all stdout
  • stdout-new-only | Only show stdout from cache misses
  • none

@zomars
Copy link

zomars commented Feb 20, 2023

Yes please! This is also suppressing eslint/tsc annotations when running in Github actions.

@domdomegg
Copy link

domdomegg commented Mar 17, 2023

@JannesMeyer @zomars You can use tsc-absolute and the suggested problem matcher setup in its README to get TSC annotations appearing correctly when using turborepo.

Note that because TSC does not provide absolute paths, when running in a turborepo style setup GitHub is unable to resolve the paths, so you'll need to use tsc-absolute anyways, even if the prefixes were removed from turborepo.

@gsoltis
Copy link
Contributor

gsoltis commented Mar 17, 2023

This is fixed in turbo@1.8.3, you can use --log-prefix=none

@gsoltis gsoltis closed this as completed Mar 17, 2023
@marcello3d
Copy link

did this regress? the option doesn't seem to work with turbo 2.0.10. specifically I'm running:

turbo --log-prefix=none --log-order=stream run foo

@janckerchen
Copy link

did this regress? the option doesn't seem to work with turbo 2.0.10. specifically I'm running:

turbo --log-prefix=none --log-order=stream run foo

It works when I use it like this.

turbo  run --log-prefix=none --log-order=stream foo

@anthonyshew
Copy link
Contributor

@marcello3d We currently have a known bug where putting flags before the command (e.g. run) get ignored. #8893 is meant to address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ergonomics Issues and features impacting the developer experience of using turbo area: logging Improvements to logging good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.