From 3a46f2ac56e9aa23037f76e56a0d2858ea41f152 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 8 Sep 2021 17:15:42 -0400 Subject: [PATCH 1/5] add --lightstep flag for tracing --- api/api.go | 31 +++++++++++------ cmd/ghcs/main.go | 90 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/api/api.go b/api/api.go index 7bb43811a..28c7072ea 100644 --- a/api/api.go +++ b/api/api.go @@ -18,6 +18,8 @@ import ( "net/http" "strconv" "strings" + + "github.com/opentracing/opentracing-go" ) const githubAPI = "https://api.github.com" @@ -42,7 +44,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/user") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -87,7 +89,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/repos/*") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -146,7 +148,7 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, err } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -194,7 +196,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*/token") if err != nil { return "", fmt.Errorf("error making request: %v", err) } @@ -228,7 +230,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) } req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -262,7 +264,7 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes } req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/proxy/environments/*/start") if err != nil { return fmt.Errorf("error making request: %v", err) } @@ -299,7 +301,7 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { return "", fmt.Errorf("error creating request: %v", err) } - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, req.URL.String()) if err != nil { return "", fmt.Errorf("error making request: %v", err) } @@ -340,7 +342,7 @@ func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Rep req.URL.RawQuery = q.Encode() a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/skus") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -384,7 +386,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -414,7 +416,7 @@ func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceN } req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { return fmt.Errorf("error making request: %v", err) } @@ -446,7 +448,7 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod req.URL.RawQuery = q.Encode() a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/repos/*/contents/*") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -478,6 +480,13 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod return decoded, nil } +func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http.Response, error) { + // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. + span, ctx := opentracing.StartSpanFromContext(ctx, spanName) + defer span.Finish() + return a.client.Do(req) +} + func (a *API) setHeaders(req *http.Request) { req.Header.Set("Authorization", "Bearer "+a.token) req.Header.Set("Accept", "application/vnd.github.v3+json") diff --git a/cmd/ghcs/main.go b/cmd/ghcs/main.go index 3e861dacb..5d7d7cf23 100644 --- a/cmd/ghcs/main.go +++ b/cmd/ghcs/main.go @@ -4,8 +4,13 @@ import ( "errors" "fmt" "io" + "log" "os" + "strconv" + "strings" + "github.com/lightstep/lightstep-tracer-go" + "github.com/opentracing/opentracing-go" "github.com/spf13/cobra" ) @@ -18,22 +23,33 @@ func main() { var version = "DEV" -var rootCmd = &cobra.Command{ - Use: "ghcs", - SilenceUsage: true, // don't print usage message after each error (see #80) - SilenceErrors: false, // print errors automatically so that main need not - Long: `Unofficial CLI tool to manage GitHub Codespaces. +var rootCmd = newRootCmd() + +func newRootCmd() *cobra.Command { + var lightstep string + + root := &cobra.Command{ + Use: "ghcs", + SilenceUsage: true, // don't print usage message after each error (see #80) + SilenceErrors: false, // print errors automatically so that main need not + Long: `Unofficial CLI tool to manage GitHub Codespaces. Running commands requires the GITHUB_TOKEN environment variable to be set to a token to access the GitHub API with.`, - Version: version, + Version: version, - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - if os.Getenv("GITHUB_TOKEN") == "" { - return tokenError - } - return nil - }, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + if os.Getenv("GITHUB_TOKEN") == "" { + return tokenError + } + initLightstep(lightstep) + return nil + }, + } + + root.PersistentFlags().StringVar(&lightstep, "lightstep", "", "Lightstep tracing endpoint (service:token@host:port)") + + return root } var tokenError = errors.New("GITHUB_TOKEN is missing") @@ -45,3 +61,53 @@ func explainError(w io.Writer, err error) { return } } + +// initLightstep parses the --lightstep=service:token@host:port flag and +// enables tracing if non-empty. +func initLightstep(config string) { + if config == "" { + return + } + + cut := func(s, sep string) (pre, post string) { + if i := strings.Index(s, sep); i >= 0 { + return s[:i], s[i+len(sep):] + } + return s, "" + } + + // Parse service:password@host:port. + serviceToken, hostPort := cut(config, "@") + service, token := cut(serviceToken, ":") + host, port := cut(hostPort, ":") + portI, err := strconv.Atoi(port) + if err != nil { + log.Fatalf("invalid lightstep configuration: %s", config) + } + + // View at https://app.lightstep.com/github-prod/service-directory/ghcs/deployments + // --lightstep=ghcs:dhhPgaoavzIHz3tJMnj3Oz88Md2VC4HpwcZ8mpoWwwuOwcfU3x+K70lLhJJAXsk63T3bWfPXGgrAwTMQxLY=@lightstep-collector.service.iad.github.net:443 + // From https://app.lightstep.com/github-prod/project ghcs + + opentracing.SetGlobalTracer(lightstep.NewTracer(lightstep.Options{ + AccessToken: token, + Collector: lightstep.Endpoint{ + Host: host, + Port: portI, + Plaintext: false, + }, + Tags: opentracing.Tags{ + lightstep.ComponentNameKey: service, + }, + })) + + // Report failure to record traces. + lightstep.SetGlobalEventHandler(func(ev lightstep.Event) { + switch ev := ev.(type) { + case lightstep.EventStatusReport, lightstep.MetricEventStatusReport: + // ignore + default: + log.Printf("[trace] %s", ev) + } + }) +} From c6a991586104cc933e09fd1ba008ec98f6e32073 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 8 Sep 2021 17:15:42 -0400 Subject: [PATCH 2/5] add --lightstep flag for tracing --- api/api.go | 31 ++++++++++------- cmd/ghcs/main.go | 86 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/api/api.go b/api/api.go index 7bb43811a..28c7072ea 100644 --- a/api/api.go +++ b/api/api.go @@ -18,6 +18,8 @@ import ( "net/http" "strconv" "strings" + + "github.com/opentracing/opentracing-go" ) const githubAPI = "https://api.github.com" @@ -42,7 +44,7 @@ func (a *API) GetUser(ctx context.Context) (*User, error) { } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/user") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -87,7 +89,7 @@ func (a *API) GetRepository(ctx context.Context, nwo string) (*Repository, error } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/repos/*") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -146,7 +148,7 @@ func (a *API) ListCodespaces(ctx context.Context, user *User) ([]*Codespace, err } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -194,7 +196,7 @@ func (a *API) GetCodespaceToken(ctx context.Context, ownerLogin, codespaceName s } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*/token") if err != nil { return "", fmt.Errorf("error making request: %v", err) } @@ -228,7 +230,7 @@ func (a *API) GetCodespace(ctx context.Context, token, owner, codespace string) } req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -262,7 +264,7 @@ func (a *API) StartCodespace(ctx context.Context, token string, codespace *Codes } req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/proxy/environments/*/start") if err != nil { return fmt.Errorf("error making request: %v", err) } @@ -299,7 +301,7 @@ func (a *API) GetCodespaceRegionLocation(ctx context.Context) (string, error) { return "", fmt.Errorf("error creating request: %v", err) } - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, req.URL.String()) if err != nil { return "", fmt.Errorf("error making request: %v", err) } @@ -340,7 +342,7 @@ func (a *API) GetCodespacesSKUs(ctx context.Context, user *User, repository *Rep req.URL.RawQuery = q.Encode() a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/skus") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -384,7 +386,7 @@ func (a *API) CreateCodespace(ctx context.Context, user *User, repository *Repos } a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -414,7 +416,7 @@ func (a *API) DeleteCodespace(ctx context.Context, user *User, token, codespaceN } req.Header.Set("Authorization", "Bearer "+token) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/vscs_internal/user/*/codespaces/*") if err != nil { return fmt.Errorf("error making request: %v", err) } @@ -446,7 +448,7 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod req.URL.RawQuery = q.Encode() a.setHeaders(req) - resp, err := a.client.Do(req) + resp, err := a.do(ctx, req, "/repos/*/contents/*") if err != nil { return nil, fmt.Errorf("error making request: %v", err) } @@ -478,6 +480,13 @@ func (a *API) GetCodespaceRepositoryContents(ctx context.Context, codespace *Cod return decoded, nil } +func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http.Response, error) { + // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. + span, ctx := opentracing.StartSpanFromContext(ctx, spanName) + defer span.Finish() + return a.client.Do(req) +} + func (a *API) setHeaders(req *http.Request) { req.Header.Set("Authorization", "Bearer "+a.token) req.Header.Set("Accept", "application/vnd.github.v3+json") diff --git a/cmd/ghcs/main.go b/cmd/ghcs/main.go index 3e861dacb..b1108547e 100644 --- a/cmd/ghcs/main.go +++ b/cmd/ghcs/main.go @@ -4,8 +4,13 @@ import ( "errors" "fmt" "io" + "log" "os" + "strconv" + "strings" + "github.com/lightstep/lightstep-tracer-go" + "github.com/opentracing/opentracing-go" "github.com/spf13/cobra" ) @@ -18,22 +23,33 @@ func main() { var version = "DEV" -var rootCmd = &cobra.Command{ - Use: "ghcs", - SilenceUsage: true, // don't print usage message after each error (see #80) - SilenceErrors: false, // print errors automatically so that main need not - Long: `Unofficial CLI tool to manage GitHub Codespaces. +var rootCmd = newRootCmd() + +func newRootCmd() *cobra.Command { + var lightstep string + + root := &cobra.Command{ + Use: "ghcs", + SilenceUsage: true, // don't print usage message after each error (see #80) + SilenceErrors: false, // print errors automatically so that main need not + Long: `Unofficial CLI tool to manage GitHub Codespaces. Running commands requires the GITHUB_TOKEN environment variable to be set to a token to access the GitHub API with.`, - Version: version, + Version: version, - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - if os.Getenv("GITHUB_TOKEN") == "" { - return tokenError - } - return nil - }, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + if os.Getenv("GITHUB_TOKEN") == "" { + return tokenError + } + initLightstep(lightstep) + return nil + }, + } + + root.PersistentFlags().StringVar(&lightstep, "lightstep", "", "Lightstep tracing endpoint (service:token@host:port)") + + return root } var tokenError = errors.New("GITHUB_TOKEN is missing") @@ -45,3 +61,49 @@ func explainError(w io.Writer, err error) { return } } + +// initLightstep parses the --lightstep=service:token@host:port flag and +// enables tracing if non-empty. +func initLightstep(config string) { + if config == "" { + return + } + + cut := func(s, sep string) (pre, post string) { + if i := strings.Index(s, sep); i >= 0 { + return s[:i], s[i+len(sep):] + } + return s, "" + } + + // Parse service:password@host:port. + serviceToken, hostPort := cut(config, "@") + service, token := cut(serviceToken, ":") + host, port := cut(hostPort, ":") + portI, err := strconv.Atoi(port) + if err != nil { + log.Fatalf("invalid lightstep configuration: %s", config) + } + + opentracing.SetGlobalTracer(lightstep.NewTracer(lightstep.Options{ + AccessToken: token, + Collector: lightstep.Endpoint{ + Host: host, + Port: portI, + Plaintext: false, + }, + Tags: opentracing.Tags{ + lightstep.ComponentNameKey: service, + }, + })) + + // Report failure to record traces. + lightstep.SetGlobalEventHandler(func(ev lightstep.Event) { + switch ev := ev.(type) { + case lightstep.EventStatusReport, lightstep.MetricEventStatusReport: + // ignore + default: + log.Printf("[trace] %s", ev) + } + }) +} From cbb82535448b11b53dfd51ea8a67c37e6a9ac1f4 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 9 Sep 2021 10:28:55 -0400 Subject: [PATCH 3/5] consolidate survey functions --- cmd/ghcs/code.go | 5 +- cmd/ghcs/common.go | 101 ++++++++++++++++++++++++++++++ cmd/ghcs/create.go | 5 -- cmd/ghcs/delete.go | 3 +- cmd/ghcs/logs.go | 2 +- cmd/ghcs/ports.go | 4 +- cmd/ghcs/ssh.go | 2 +- internal/codespaces/codespaces.go | 80 ----------------------- 8 files changed, 108 insertions(+), 94 deletions(-) create mode 100644 cmd/ghcs/common.go diff --git a/cmd/ghcs/code.go b/cmd/ghcs/code.go index 5bad53648..81c2cc9a2 100644 --- a/cmd/ghcs/code.go +++ b/cmd/ghcs/code.go @@ -7,7 +7,6 @@ import ( "os" "github.com/github/ghcs/api" - "github.com/github/ghcs/internal/codespaces" "github.com/skratchdot/open-golang/open" "github.com/spf13/cobra" ) @@ -47,9 +46,9 @@ func code(codespaceName string, useInsiders bool) error { } if codespaceName == "" { - codespace, err := codespaces.ChooseCodespace(ctx, apiClient, user) + codespace, err := chooseCodespace(ctx, apiClient, user) if err != nil { - if err == codespaces.ErrNoCodespaces { + if err == errNoCodespaces { return err } return fmt.Errorf("error choosing codespace: %v", err) diff --git a/cmd/ghcs/common.go b/cmd/ghcs/common.go new file mode 100644 index 000000000..712c04daa --- /dev/null +++ b/cmd/ghcs/common.go @@ -0,0 +1,101 @@ +package main + +// This file defines functions common to the entire ghcs command set. + +import ( + "context" + "errors" + "fmt" + "sort" + + "github.com/AlecAivazis/survey/v2" + "github.com/github/ghcs/api" + "golang.org/x/term" +) + +var errNoCodespaces = errors.New("You have no codespaces.") + +func chooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (*api.Codespace, error) { + codespaces, err := apiClient.ListCodespaces(ctx, user) + if err != nil { + return nil, fmt.Errorf("error getting codespaces: %v", err) + } + + if len(codespaces) == 0 { + return nil, errNoCodespaces + } + + sort.Slice(codespaces, func(i, j int) bool { + return codespaces[i].CreatedAt > codespaces[j].CreatedAt + }) + + codespacesByName := make(map[string]*api.Codespace) + codespacesNames := make([]string, 0, len(codespaces)) + for _, codespace := range codespaces { + codespacesByName[codespace.Name] = codespace + codespacesNames = append(codespacesNames, codespace.Name) + } + + sshSurvey := []*survey.Question{ + { + Name: "codespace", + Prompt: &survey.Select{ + Message: "Choose codespace:", + Options: codespacesNames, + Default: codespacesNames[0], + }, + Validate: survey.Required, + }, + } + + var answers struct { + Codespace string + } + if err := ask(sshSurvey, &answers); err != nil { + return nil, fmt.Errorf("error getting answers: %v", err) + } + + codespace := codespacesByName[answers.Codespace] + return codespace, nil +} + +func getOrChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User, codespaceName string) (codespace *api.Codespace, token string, err error) { + if codespaceName == "" { + codespace, err = chooseCodespace(ctx, apiClient, user) + if err != nil { + if err == errNoCodespaces { + return nil, "", err + } + return nil, "", fmt.Errorf("choosing codespace: %v", err) + } + codespaceName = codespace.Name + + token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) + if err != nil { + return nil, "", fmt.Errorf("getting codespace token: %v", err) + } + } else { + token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) + if err != nil { + return nil, "", fmt.Errorf("getting codespace token for given codespace: %v", err) + } + + codespace, err = apiClient.GetCodespace(ctx, token, user.Login, codespaceName) + if err != nil { + return nil, "", fmt.Errorf("getting full codespace details: %v", err) + } + } + + return codespace, token, nil +} + +var hasTTY = term.IsTerminal(0) && term.IsTerminal(1) // is process connected to a terminal? + +// ask asks survey questions on the terminal, using standard options. +// It fails unless hasTTY, but ideally callers should avoid calling it in that case. +func ask(qs []*survey.Question, response interface{}) error { + if !hasTTY { + return fmt.Errorf("no terminal") + } + return survey.Ask(qs, response, survey.WithShowCursor(true)) +} diff --git a/cmd/ghcs/create.go b/cmd/ghcs/create.go index bd1d89e4e..093450e7d 100644 --- a/cmd/ghcs/create.go +++ b/cmd/ghcs/create.go @@ -286,8 +286,3 @@ func getMachineName(ctx context.Context, machine string, user *api.User, repo *a return machine, nil } - -// ask asks survery questions using standard options. -func ask(qs []*survey.Question, response interface{}) error { - return survey.Ask(qs, response, survey.WithShowCursor(true)) -} diff --git a/cmd/ghcs/delete.go b/cmd/ghcs/delete.go index 92c405766..75b9362bb 100644 --- a/cmd/ghcs/delete.go +++ b/cmd/ghcs/delete.go @@ -8,7 +8,6 @@ import ( "github.com/github/ghcs/api" "github.com/github/ghcs/cmd/ghcs/output" - "github.com/github/ghcs/internal/codespaces" "github.com/spf13/cobra" ) @@ -63,7 +62,7 @@ func delete_(codespaceName string) error { return fmt.Errorf("error getting user: %v", err) } - codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, codespaceName) + codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { return fmt.Errorf("get or choose codespace: %v", err) } diff --git a/cmd/ghcs/logs.go b/cmd/ghcs/logs.go index e35ecb728..9998b5f0f 100644 --- a/cmd/ghcs/logs.go +++ b/cmd/ghcs/logs.go @@ -51,7 +51,7 @@ func logs(ctx context.Context, tail bool, codespaceName string) error { return fmt.Errorf("getting user: %v", err) } - codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, codespaceName) + codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { return fmt.Errorf("get or choose codespace: %v", err) } diff --git a/cmd/ghcs/ports.go b/cmd/ghcs/ports.go index 4258991b6..c175549a0 100644 --- a/cmd/ghcs/ports.go +++ b/cmd/ghcs/ports.go @@ -67,9 +67,9 @@ func ports(opts *portsOptions) error { return fmt.Errorf("error getting user: %v", err) } - codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, opts.codespaceName) + codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, opts.codespaceName) if err != nil { - if err == codespaces.ErrNoCodespaces { + if err == errNoCodespaces { return err } return fmt.Errorf("error choosing codespace: %v", err) diff --git a/cmd/ghcs/ssh.go b/cmd/ghcs/ssh.go index e2003b347..aefb959f3 100644 --- a/cmd/ghcs/ssh.go +++ b/cmd/ghcs/ssh.go @@ -52,7 +52,7 @@ func ssh(ctx context.Context, sshProfile, codespaceName string, localSSHServerPo return fmt.Errorf("error getting user: %v", err) } - codespace, token, err := codespaces.GetOrChooseCodespace(ctx, apiClient, user, codespaceName) + codespace, token, err := getOrChooseCodespace(ctx, apiClient, user, codespaceName) if err != nil { return fmt.Errorf("get or choose codespace: %v", err) } diff --git a/internal/codespaces/codespaces.go b/internal/codespaces/codespaces.go index b5fa4a583..9aee3564c 100644 --- a/internal/codespaces/codespaces.go +++ b/internal/codespaces/codespaces.go @@ -4,62 +4,12 @@ import ( "context" "errors" "fmt" - "sort" "time" - "github.com/AlecAivazis/survey/v2" "github.com/github/ghcs/api" "github.com/github/go-liveshare" ) -var ( - ErrNoCodespaces = errors.New("You have no codespaces.") -) - -func ChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User) (*api.Codespace, error) { - codespaces, err := apiClient.ListCodespaces(ctx, user) - if err != nil { - return nil, fmt.Errorf("error getting codespaces: %v", err) - } - - if len(codespaces) == 0 { - return nil, ErrNoCodespaces - } - - sort.Slice(codespaces, func(i, j int) bool { - return codespaces[i].CreatedAt > codespaces[j].CreatedAt - }) - - codespacesByName := make(map[string]*api.Codespace) - codespacesNames := make([]string, 0, len(codespaces)) - for _, codespace := range codespaces { - codespacesByName[codespace.Name] = codespace - codespacesNames = append(codespacesNames, codespace.Name) - } - - sshSurvey := []*survey.Question{ - { - Name: "codespace", - Prompt: &survey.Select{ - Message: "Choose codespace:", - Options: codespacesNames, - Default: codespacesNames[0], - }, - Validate: survey.Required, - }, - } - - answers := struct { - Codespace string - }{} - if err := survey.Ask(sshSurvey, &answers); err != nil { - return nil, fmt.Errorf("error getting answers: %v", err) - } - - codespace := codespacesByName[answers.Codespace] - return codespace, nil -} - type logger interface { Print(v ...interface{}) (int, error) Println(v ...interface{}) (int, error) @@ -123,33 +73,3 @@ func ConnectToLiveshare(ctx context.Context, log logger, apiClient *api.API, use return lsclient.JoinWorkspace(ctx) } - -func GetOrChooseCodespace(ctx context.Context, apiClient *api.API, user *api.User, codespaceName string) (codespace *api.Codespace, token string, err error) { - if codespaceName == "" { - codespace, err = ChooseCodespace(ctx, apiClient, user) - if err != nil { - if err == ErrNoCodespaces { - return nil, "", err - } - return nil, "", fmt.Errorf("choosing codespace: %v", err) - } - codespaceName = codespace.Name - - token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) - if err != nil { - return nil, "", fmt.Errorf("getting codespace token: %v", err) - } - } else { - token, err = apiClient.GetCodespaceToken(ctx, user.Login, codespaceName) - if err != nil { - return nil, "", fmt.Errorf("getting codespace token for given codespace: %v", err) - } - - codespace, err = apiClient.GetCodespace(ctx, token, user.Login, codespaceName) - if err != nil { - return nil, "", fmt.Errorf("getting full codespace details: %v", err) - } - } - - return codespace, token, nil -} From ee44ecc944cbd6288dadec9c9b723367f81b9f8c Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 9 Sep 2021 13:42:44 -0400 Subject: [PATCH 4/5] include span context in HTTP request --- api/api.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/api.go b/api/api.go index 28c7072ea..cb9f9371a 100644 --- a/api/api.go +++ b/api/api.go @@ -484,6 +484,7 @@ func (a *API) do(ctx context.Context, req *http.Request, spanName string) (*http // TODO(adonovan): use NewRequestWithContext(ctx) and drop ctx parameter. span, ctx := opentracing.StartSpanFromContext(ctx, spanName) defer span.Finish() + req = req.WithContext(ctx) return a.client.Do(req) } From 2cbe1207742a7e5add6d0da54edda4257a98083c Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 9 Sep 2021 16:37:26 -0400 Subject: [PATCH 5/5] return err, don"t fatal --- cmd/ghcs/main.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/ghcs/main.go b/cmd/ghcs/main.go index 44db33113..3a692d443 100644 --- a/cmd/ghcs/main.go +++ b/cmd/ghcs/main.go @@ -42,8 +42,7 @@ token to access the GitHub API with.`, if os.Getenv("GITHUB_TOKEN") == "" { return tokenError } - initLightstep(lightstep) - return nil + return initLightstep(lightstep) }, } @@ -64,9 +63,9 @@ func explainError(w io.Writer, err error) { // initLightstep parses the --lightstep=service:token@host:port flag and // enables tracing if non-empty. -func initLightstep(config string) { +func initLightstep(config string) error { if config == "" { - return + return nil } cut := func(s, sep string) (pre, post string) { @@ -82,7 +81,7 @@ func initLightstep(config string) { host, port := cut(hostPort, ":") portI, err := strconv.Atoi(port) if err != nil { - log.Fatalf("invalid Lightstep configuration: %s", config) + return fmt.Errorf("invalid Lightstep configuration: %s", config) } opentracing.SetGlobalTracer(lightstep.NewTracer(lightstep.Options{ @@ -106,4 +105,6 @@ func initLightstep(config string) { log.Printf("[trace] %s", ev) } }) + + return nil }