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

feat(logging): group logging in github by task id #5318

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cli/internal/run/real_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,10 @@ func (ec *execContext) exec(ctx gocontext.Context, packageTask *nodes.PackageTas
WarnPrefix: prettyPrefix,
}

if ec.rs.Opts.runOpts.IsGithubActions {
ui.Output(fmt.Sprintf("::group::%s", packageTask.OutputPrefix(ec.isSinglePackage)))
}

cacheStatus, err := taskCache.RestoreOutputs(ctx, prefixedUI, progressLogger)

// It's safe to set the CacheStatus even if there's an error, because if there's
Expand Down Expand Up @@ -413,6 +417,11 @@ func (ec *execContext) exec(ctx gocontext.Context, packageTask *nodes.PackageTas

closeOutputs := func() error {
var closeErrors []error
if ec.rs.Opts.runOpts.IsGithubActions {
// We don't use the prefixedUI here because the prefix in this case would include
// the ::group::<taskID>, and we explicitly want to close the github group
ui.Output("::endgroup::")
}

if err := logStreamerOut.Close(); err != nil {
closeErrors = append(closeErrors, errors.Wrap(err, "log stdout"))
Expand Down
8 changes: 8 additions & 0 deletions cli/internal/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/vercel/turbo/cli/internal/analytics"
"github.com/vercel/turbo/cli/internal/cache"
"github.com/vercel/turbo/cli/internal/ci"
"github.com/vercel/turbo/cli/internal/cmdutil"
"github.com/vercel/turbo/cli/internal/context"
"github.com/vercel/turbo/cli/internal/core"
Expand Down Expand Up @@ -128,6 +129,13 @@ func optsFromArgs(args *turbostate.ParsedArgsFromRust) (*Opts, error) {
}

func configureRun(base *cmdutil.CmdBase, opts *Opts, signalWatcher *signals.Watcher) *run {
if opts.runOpts.LogOrder == "auto" {
if name := ci.Constant(); name == "GITHUB_ACTIONS" {
opts.runOpts.LogOrder = "grouped"
opts.runOpts.IsGithubActions = true
}
}

processes := process.NewManager(base.Logger.Named("processes"))
signalWatcher.AddOnClose(processes.Close)
return &run{
Expand Down
3 changes: 3 additions & 0 deletions cli/internal/util/run_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,7 @@ type RunOpts struct {
Summarize bool

ExperimentalSpaceID string

// Whether this run is in Github Actions
IsGithubActions bool
}
13 changes: 13 additions & 0 deletions examples-tests/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,16 @@ fi
[ ! -d .git ] || rm -rf .git

"$MONOREPO_ROOT_DIR/turborepo-tests/helpers/setup_git.sh" "${TARGET_DIR}" "false"

# We set this explicitly to stream, so we can lock into to streaming logs (i.e. not "auto") behavior.
#
# We do this because when these tests are invoked in CI (through .github/actions/test.yml), they will
# inherit the GITHUB_ACTIONS=true env var, and each of the `turbo run` invocations into behavior
# we do not want. Since prysk mainly tests log output, this extra behavior will break all the tests
# and can be unpredictable over time, as we make "auto" do more magic.
#
# Note: since these tests are invoked _through_ turbo, the ideal setup would be to pass --env-mode=strict
# so we can prevent the `GITHUB_ACTIONS` env var from being passed down here from the top level turbo.
# But as of now, this breaks our tests (and I'm not sure why). If we make that work, we can remove this
# explicit locking of log order. See PR attempt here: https://github.com/vercel/turbo/pull/5324
export TURBO_LOG_ORDER=stream
13 changes: 13 additions & 0 deletions turborepo-tests/e2e/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ import { Monorepo } from "./monorepo";
import path from "path";
import * as fs from "fs";

// We set this explicitly to stream, so we can lock into to streaming logs (i.e. not "auto") behavior.
//
// We do this because when these tests are invoked in CI (through .github/actions/test.yml), they will
// inherit the GITHUB_ACTIONS=true env var, and each of the `turbo run` invocations into behavior
// we do not want. Since prysk mainly tests log output, this extra behavior will break all the tests
// and can be unpredictable over time, as we make "auto" do more magic.
//
// Note: since these tests are invoked _through_ turbo, the ideal setup would be to pass --env-mode=strict
// so we can prevent the `GITHUB_ACTIONS` env var from being passed down here from the top level turbo.
// But as of now, this breaks our tests (and I'm not sure why). If we make that work, we can remove this
// explicit locking of log order. See PR attempt here: https://github.com/vercel/turbo/pull/5324
process.env.TURBO_LOG_ORDER = "stream";

const basicPipeline = {
pipeline: {
test: {
Expand Down
13 changes: 13 additions & 0 deletions turborepo-tests/integration/test.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
#!/bin/bash

# We set this explicitly to stream, so we can lock into to streaming logs (i.e. not "auto") behavior.
#
# We do this because when these tests are invoked in CI (through .github/actions/test.yml), they will
# inherit the GITHUB_ACTIONS=true env var, and each of the `turbo run` invocations into behavior
# we do not want. Since prysk mainly tests log output, this extra behavior will break all the tests
# and can be unpredictable over time, as we make "auto" do more magic.
#
# Note: since these tests are invoked _through_ turbo, the ideal setup would be to pass --env-mode=strict
# so we can prevent the `GITHUB_ACTIONS` env var from being passed down here from the top level turbo.
# But as of now, this breaks our tests (and I'm not sure why). If we make that work, we can remove this
# explicit locking of log order. See PR attempt here: https://github.com/vercel/turbo/pull/5324
export TURBO_LOG_ORDER=stream
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes our integration tests (turbo_help.t and no_args.t, and verbosity.t), because when clap serializes the help message from the CLI, it inlines the value of this env var. So the output diff is:

-        --log-order <LOG_ORDER>          Set type of task output order. Use "stream" to show output as soon as it is available. Use "grouped" to show output when a command has finished execution. Use "auto" to let turbo decide based on its own heuristics. (default auto) [env: TURBO_LOG_ORDER=] [default: auto] [possible values: auto, stream, grouped]
+        --log-order <LOG_ORDER>          Set type of task output order. Use "stream" to show output as soon as it is available. Use "grouped" to show output when a command has finished execution. Use "auto" to let turbo decide based on its own heuristics. (default auto) [env: TURBO_LOG_ORDER=stream] [default: auto] [possible values: auto, stream, grouped]

We could solve this in a few ways:


if [ "$1" = "" ]; then
.cram_env/bin/prysk --shell="$(which bash)" tests
else
Expand Down
Loading