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.

Sign up for GitHub

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

Jump to bottom

Include logs when posting task summary #4642

Merged
merged 4 commits into from Apr 20, 2023
Merged

Include logs when posting task summary #4642

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
893a92e
Send logs with task summaries
mehulkar Apr 19, 2023
c6f1522
Add a spinner after 1s while uploading
mehulkar Apr 19, 2023
7a378b6
Pass in context where its used
mehulkar Apr 20, 2023
493d269
Merge branch 'main' into mehulkar/turbo-891-include-logs-when-posting…
mehulkar Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/internal/run/run.go
View file
Open in desktop
Expand Up @@ -362,6 +362,7 @@ func (r *run) run(ctx gocontext.Context, targets []string) error {
// RunSummary contains information that is statically analyzable about
// the tasks that we expect to run based on the user command.
summary := runsummary.NewRunSummary(
ctx,
startAt,
r.base.UI,
r.base.RepoRoot,
Expand Down
22 changes: 19 additions & 3 deletions cli/internal/runsummary/run_summary.go
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@
package runsummary

import (
"context"
"encoding/json"
"fmt"
"path/filepath"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/mitchellh/cli"
"github.com/segmentio/ksuid"
"github.com/vercel/turbo/cli/internal/client"
"github.com/vercel/turbo/cli/internal/spinner"
"github.com/vercel/turbo/cli/internal/turbopath"
"github.com/vercel/turbo/cli/internal/util"
"github.com/vercel/turbo/cli/internal/workspace"
Expand Down Expand Up @@ -41,6 +43,7 @@ const (
// about the Run and references to other things that we need.
type Meta struct {
RunSummary *RunSummary
ctx context.Context
Copy link
Contributor

Choose a reason for hiding this comment

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

Go idiom: don't save Context on a struct, pass it in where you need it. In this case it should be an argument to Close. Context instances are more properly tied to an operation than a particular data structure.

ui cli.Ui
repoRoot turbopath.AbsoluteSystemPath // used to write run summary
repoPath turbopath.RelativeSystemPath
Expand All @@ -66,6 +69,7 @@ type RunSummary struct {

// NewRunSummary returns a RunSummary instance
func NewRunSummary(
ctx context.Context,
startAt time.Time,
ui cli.Ui,
repoRoot turbopath.AbsoluteSystemPath,
Expand Down Expand Up @@ -105,6 +109,7 @@ func NewRunSummary(
GlobalHashSummary: globalHashSummary,
},
ui: ui,
ctx: ctx,
runType: runType,
repoRoot: repoRoot,
singlePackage: singlePackage,
Expand Down Expand Up @@ -161,8 +166,19 @@ func (rsm *Meta) Close(exitCode int, workspaceInfos workspace.Catalog) error {
return nil
}

url, errs := rsm.record()
// Wrap the record function so we can hoist out url/errors but keep
// the function signature/type the spinner.WaitFor expects.
var url string
var errs []error
record := func() {
url, errs = rsm.record()
}

func() {
_ = spinner.WaitFor(rsm.ctx, record, rsm.ui, "...sending run summary...", 1000*time.Millisecond)
}()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'm using this correctly, but would appreciate another look


// After the spinner is done
if len(errs) > 0 {
rsm.ui.Warn("Errors recording run to Spaces")
for _, err := range errs {
Expand Down Expand Up @@ -234,7 +250,7 @@ func (rsm *Meta) record() (string, []error) {
payload := rsm.newSpacesRunCreatePayload()
if startPayload, err := json.Marshal(payload); err == nil {
if resp, err := rsm.apiClient.JSONPost(createRunEndpoint, startPayload); err != nil {
errs = append(errs, err)
errs = append(errs, fmt.Errorf("POST %s: %w", createRunEndpoint, err))
} else {
if err := json.Unmarshal(resp, response); err != nil {
errs = append(errs, fmt.Errorf("Error unmarshaling response: %w", err))
Expand All @@ -250,7 +266,7 @@ func (rsm *Meta) record() (string, []error) {
if donePayload, err := json.Marshal(newSpacesDonePayload(rsm.RunSummary)); err == nil {
patchURL := fmt.Sprintf(runsPatchEndpoint, rsm.spaceID, response.ID)
if _, err := rsm.apiClient.JSONPatch(patchURL, donePayload); err != nil {
errs = append(errs, fmt.Errorf("Error marking run as done: %w", err))
errs = append(errs, fmt.Errorf("PATCH %s: %w", patchURL, err))
}
}
}
Expand Down
34 changes: 10 additions & 24 deletions cli/internal/runsummary/spaces.go
View file
Open in desktop
Expand Up @@ -11,30 +11,14 @@ type spacesRunResponse struct {
}

type spacesRunPayload struct {
// StartTime is when this run was started
StartTime int64 `json:"startTime,omitempty"`

// EndTime is when this run ended. We will never be submitting start and endtime at the same time.
EndTime int64 `json:"endTime,omitempty"`

// Status is
Status string `json:"status,omitempty"`

// Type should be hardcoded to TURBO
Type string `json:"type,omitempty"`

// ExitCode is the exit code for the full run
ExitCode int `json:"exitCode,omitempty"`

// The command that kicked off the turbo run
Command string `json:"command,omitempty"`

// RepositoryPath is the relative directory from the turborepo root to where
// the command was invoked.
RepositoryPath string `json:"repositoryPath,omitempty"`

// Context is the host on which this Run was executed (e.g. Github Action, Vercel, etc)
Context string `json:"context,omitempty"`
StartTime int64 `json:"startTime,omitempty"` // when the run was started
EndTime int64 `json:"endTime,omitempty"` // when the run ended. we should never submit start and end at the same time.
Status string `json:"status,omitempty"` // Status is "running" or "completed"
Type string `json:"type,omitempty"` // hardcoded to "TURBO"
ExitCode int `json:"exitCode,omitempty"` // exit code for the full run
Command string `json:"command,omitempty"` // the thing that kicked off the turbo run
RepositoryPath string `json:"repositoryPath,omitempty"` // where the command was invoked from
Context string `json:"context,omitempty"` // the host on which this Run was executed (e.g. Github Action, Vercel, etc)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no changes, just tightened it up by putting comments on the same line


// TODO: we need to add these in
// originationUser string
Expand Down Expand Up @@ -64,6 +48,7 @@ type spacesTask struct {
ExitCode int `json:"exitCode,omitempty"`
Dependencies []string `json:"dependencies,omitempty"`
Dependents []string `json:"dependents,omitempty"`
Logs string `json:"log"`
}

func (rsm *Meta) newSpacesRunCreatePayload() *spacesRunPayload {
Expand Down Expand Up @@ -106,5 +91,6 @@ func newSpacesTaskPayload(taskSummary *TaskSummary) *spacesTask {
ExitCode: *taskSummary.Execution.exitCode,
Dependencies: taskSummary.Dependencies,
Dependents: taskSummary.Dependents,
Logs: string(taskSummary.GetLogs()),
}
}
11 changes: 11 additions & 0 deletions cli/internal/runsummary/task_summary.go
View file
Open in desktop
@@ -1,6 +1,8 @@
package runsummary

import (
"os"

"github.com/vercel/turbo/cli/internal/cache"
"github.com/vercel/turbo/cli/internal/fs"
"github.com/vercel/turbo/cli/internal/turbopath"
Expand Down Expand Up @@ -76,6 +78,15 @@ type TaskSummary struct {
Execution *TaskExecutionSummary `json:"execution,omitempty"` // omit when it's not set
}

// GetLogs reads the Logfile and returns the data
func (ts *TaskSummary) GetLogs() []byte {
bytes, err := os.ReadFile(ts.LogFile)
if err != nil {
return []byte{}
}
return bytes
}

// TaskEnvVarSummary contains the environment variables that impacted a task's hash
type TaskEnvVarSummary struct {
Configured []string `json:"configured"`
Expand Down