From f3b153ec918644fe5feb0e0cc57d595fcd52d9aa Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 22 Aug 2023 16:28:04 -0400 Subject: [PATCH 01/45] Delete a remotely created project if runtime setup fails. --- internal/runners/initialize/init.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/runners/initialize/init.go b/internal/runners/initialize/init.go index b0182e4b76..cb8621c3f7 100644 --- a/internal/runners/initialize/init.go +++ b/internal/runners/initialize/init.go @@ -234,6 +234,11 @@ func (r *Initialize) Run(params *RunParams) (rerr error) { err = runbits.RefreshRuntime(r.auth, r.out, r.analytics, proj, commitID, true, target.TriggerInit, r.svcModel) if err != nil { + logging.Debug("Deleting remotely created project due to runtime setup error") + err2 := model.DeleteProject(namespace.Owner, namespace.Project, r.auth) + if err2 != nil { + multilog.Error("Error deleting remotely created project after runtime setup error: %v", errs.JoinMessage(err2)) + } return locale.WrapError(err, "err_init_refresh", "Could not setup runtime after init") } From bd2efc40ae3837d02f1daee3c59e699d3dbae49f Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 23 Aug 2023 16:07:17 -0700 Subject: [PATCH 02/45] Always print auth URL --- internal/locale/locales/en-us.yaml | 6 +++++- internal/runners/learn/learn.go | 2 +- pkg/cmdlets/auth/login.go | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index 5191b27bda..91889d7882 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -937,7 +937,7 @@ prompt_signup_browser_action: prompt_login_after_browser_signup: other: Please login once you've registered your account err_browser_open: - other: "Could not open your browser, please manually open the following URL in your browser: {{.V0}}" + other: "Could not open your browser, please manually open the following URL above." err_activate_auth_required: other: Activating a project requires you to be authenticated against the ActiveState Platform err_os_not_a_directory: @@ -1032,6 +1032,10 @@ auth_device_verify_security_code: Please sign into the ActiveState Platform (if you have not already done so) and click "Authorize". After that, it may take a few seconds for the authorization process here to complete. + + If the URL does not open automatically, please copy and paste it into your browser. + + [ACTIONABLE]{{.V1}}[/RESET] auth_device_timeout: other: Authorization timeout. Please try again. auth_device_success: diff --git a/internal/runners/learn/learn.go b/internal/runners/learn/learn.go index 0c5967830e..3e87504d2c 100644 --- a/internal/runners/learn/learn.go +++ b/internal/runners/learn/learn.go @@ -27,7 +27,7 @@ func (l *Learn) Run() error { err := open.Run(constants.CheatSheetURL) if err != nil { logging.Warning("Could not open browser: %v", err) - l.out.Notice(locale.Tr("err_browser_open", constants.CheatSheetURL)) + l.out.Notice(locale.Tr("err_browser_open")) } return nil diff --git a/pkg/cmdlets/auth/login.go b/pkg/cmdlets/auth/login.go index 108bcebeef..896c1d5a28 100644 --- a/pkg/cmdlets/auth/login.go +++ b/pkg/cmdlets/auth/login.go @@ -235,7 +235,7 @@ func AuthenticateWithBrowser(out output.Outputer, auth *authentication.Auth, pro if response.UserCode == nil { return errs.New("Invalid response: Missing user code.") } - out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode)) + out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, *response.VerificationURIComplete)) // Open URL in browser if response.VerificationURIComplete == nil { @@ -244,7 +244,7 @@ func AuthenticateWithBrowser(out output.Outputer, auth *authentication.Auth, pro err = OpenURI(*response.VerificationURIComplete) if err != nil { logging.Warning("Could not open browser: %v", err) - out.Notice(locale.Tr("err_browser_open", *response.VerificationURIComplete)) + out.Notice(locale.Tr("err_browser_open")) } var apiKey string From 1f2f3d739027218cc57882081554024ed140de49 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 24 Aug 2023 12:50:15 -0400 Subject: [PATCH 03/45] Notify user if deleting remotely created project fails. --- internal/runners/initialize/init.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/runners/initialize/init.go b/internal/runners/initialize/init.go index cb8621c3f7..dc5e5a3e96 100644 --- a/internal/runners/initialize/init.go +++ b/internal/runners/initialize/init.go @@ -238,6 +238,7 @@ func (r *Initialize) Run(params *RunParams) (rerr error) { err2 := model.DeleteProject(namespace.Owner, namespace.Project, r.auth) if err2 != nil { multilog.Error("Error deleting remotely created project after runtime setup error: %v", errs.JoinMessage(err2)) + return locale.WrapError(err, "err_init_refresh_delete_project", "Could not setup runtime after init, and could not delete newly created Platform project. Please delete it manually before trying again") } return locale.WrapError(err, "err_init_refresh", "Could not setup runtime after init") } From a5aa0b9c14eca153997a6098a3bac4fb6d427a3e Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 24 Aug 2023 12:39:16 -0400 Subject: [PATCH 04/45] Added analytics dimension for state tool use on state tool CI. --- .github/workflows/build.yml | 2 ++ .github/workflows/propagate.yml | 1 + .github/workflows/release.yml | 1 + .github/workflows/verify.yml | 1 + internal/analytics/client/async/client.go | 3 +++ internal/analytics/client/sync/client.go | 1 + internal/analytics/client/sync/reporters/ga-state.go | 1 + internal/analytics/dimensions/dimensions.go | 6 ++++++ internal/condition/condition.go | 4 ++++ internal/constants/constants.go | 3 +++ test/integration/analytics_int_test.go | 2 ++ 11 files changed, 25 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0e928c98a2..f991dbced9 100755 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -46,6 +46,7 @@ jobs: fail-fast: false runs-on: ${{ matrix.platform }} env: + ACTIVESTATE_CI: true ACTIVESTATE_CLI_DISABLE_RUNTIME: true SHELL: bash GITHUB_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -389,6 +390,7 @@ jobs: - os_specific runs-on: ubuntu-20.04 env: + ACTIVESTATE_CI: true ACTIVESTATE_CLI_DISABLE_RUNTIME: true SHELL: bash GITHUB_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/propagate.yml b/.github/workflows/propagate.yml index a3925a939d..b5d5048b08 100644 --- a/.github/workflows/propagate.yml +++ b/.github/workflows/propagate.yml @@ -18,6 +18,7 @@ jobs: go-version: - 1.20.x env: + ACTIVESTATE_CI: true ACTIVESTATE_CLI_DISABLE_RUNTIME: true SHELL: bash GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 96e7d573ce..188adc8487 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,6 +20,7 @@ jobs: go-version: - 1.20.x env: + ACTIVESTATE_CI: true ACTIVESTATE_CLI_DISABLE_RUNTIME: true SHELL: bash GITHUB_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 6788479d3d..78fde2fe8b 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -17,6 +17,7 @@ jobs: name: Target & Verify PR runs-on: ubuntu-20.04 env: + ACTIVESTATE_CI: true ACTIVESTATE_CLI_DISABLE_RUNTIME: true SHELL: bash GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/internal/analytics/client/async/client.go b/internal/analytics/client/async/client.go index ecd9a43441..9793a435c1 100644 --- a/internal/analytics/client/async/client.go +++ b/internal/analytics/client/async/client.go @@ -37,6 +37,7 @@ type Client struct { sequence int ci bool interactive bool + activestateCI bool } var _ analytics.Dispatcher = &Client{} @@ -55,6 +56,7 @@ func New(svcModel *model.SvcModel, cfg *config.Instance, auth *authentication.Au a.auth = auth a.ci = condition.OnCI() a.interactive = out.Config().Interactive + a.activestateCI = condition.InActiveStateCI() if condition.InUnitTest() { return a @@ -118,6 +120,7 @@ func (a *Client) sendEvent(category, action, label string, dims ...*dimensions.V a.sequence++ dim.CI = &a.ci dim.Interactive = &a.interactive + dim.ActiveStateCI = &a.activestateCI dim.Merge(dims...) dimMarshalled, err := dim.Marshal() diff --git a/internal/analytics/client/sync/client.go b/internal/analytics/client/sync/client.go index 6b769c2e0f..36d725fb14 100644 --- a/internal/analytics/client/sync/client.go +++ b/internal/analytics/client/sync/client.go @@ -115,6 +115,7 @@ func New(cfg *config.Instance, auth *authentication.Auth, out output.Outputer) * Sequence: ptr.To(0), CI: ptr.To(condition.OnCI()), Interactive: ptr.To(interactive), + ActiveStateCI: ptr.To(condition.InActiveStateCI()), } a.customDimensions = customDimensions diff --git a/internal/analytics/client/sync/reporters/ga-state.go b/internal/analytics/client/sync/reporters/ga-state.go index 40f7fe217b..623b7470d8 100644 --- a/internal/analytics/client/sync/reporters/ga-state.go +++ b/internal/analytics/client/sync/reporters/ga-state.go @@ -99,5 +99,6 @@ func legacyDimensionMap(d *dimensions.Values) map[string]string { "24": ptr.From(d.TargetVersion, ""), "25": ptr.From(d.Error, ""), "26": ptr.From(d.Message, ""), + "27": strconv.FormatBool(ptr.From(d.ActiveStateCI, false)), } } diff --git a/internal/analytics/dimensions/dimensions.go b/internal/analytics/dimensions/dimensions.go index 37c5669b63..fbcd340e34 100644 --- a/internal/analytics/dimensions/dimensions.go +++ b/internal/analytics/dimensions/dimensions.go @@ -45,6 +45,7 @@ type Values struct { Message *string CI *bool Interactive *bool + ActiveStateCI *bool preProcessor func(*Values) error } @@ -98,6 +99,7 @@ func NewDefaultDimensions(pjNamespace, sessionToken, updateTag string) *Values { ptr.To(""), ptr.To(false), ptr.To(false), + ptr.To(false), nil, } } @@ -128,6 +130,7 @@ func (v *Values) Clone() *Values { Message: ptr.Clone(v.Message), CI: ptr.Clone(v.CI), Interactive: ptr.Clone(v.Interactive), + ActiveStateCI: ptr.Clone(v.ActiveStateCI), preProcessor: v.preProcessor, } } @@ -208,6 +211,9 @@ func (m *Values) Merge(mergeWith ...*Values) { if dim.Interactive != nil { m.Interactive = dim.Interactive } + if dim.ActiveStateCI != nil { + m.ActiveStateCI = dim.ActiveStateCI + } if dim.preProcessor != nil { m.preProcessor = dim.preProcessor } diff --git a/internal/condition/condition.go b/internal/condition/condition.go index d81a68a1a3..65e5e15011 100644 --- a/internal/condition/condition.go +++ b/internal/condition/condition.go @@ -41,6 +41,10 @@ func BuiltOnDevMachine() bool { return !BuiltViaCI() } +func InActiveStateCI() bool { + return os.Getenv(constants.ActiveStateCIEnvVarName) == "true" +} + func OptInUnstable(cfg Configurable) bool { if v := os.Getenv(constants.OptinUnstableEnvVarName); v != "" { return v == "true" diff --git a/internal/constants/constants.go b/internal/constants/constants.go index d62c67ba3d..7f8121e5e3 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -504,3 +504,6 @@ const TerminalAnimationInterval = 150 * time.Millisecond // RuntimeSetupWaitEnvVarName is only used for an integration test to pause installation and wait // for Ctrl+C. const RuntimeSetupWaitEnvVarName = "ACTIVESTATE_CLI_RUNTIME_SETUP_WAIT" + +// ActiveStateCIEnvVarName is the environment variable set when running in an ActiveState CI environment. +const ActiveStateCIEnvVarName = "ACTIVESTATE_CI" diff --git a/test/integration/analytics_int_test.go b/test/integration/analytics_int_test.go index ab8e675c7b..8bd885f5c8 100644 --- a/test/integration/analytics_int_test.go +++ b/test/integration/analytics_int_test.go @@ -601,6 +601,8 @@ func (suite *AnalyticsIntegrationTestSuite) TestCIAndInteractiveDimensions() { } suite.Equal(condition.OnCI(), *e.Dimensions.CI, "analytics should report being on CI") suite.Equal(interactive, *e.Dimensions.Interactive, "analytics did not report the correct interactive value for %v", e) + suite.Equal(condition.OnCI(), // not InActiveStateCI() because if it's false, we forgot to set ACTIVESTATE_CI env var in GitHub Actions scripts + *e.Dimensions.ActiveStateCI, "analytics did not report being in ActiveState CI") processedAnEvent = true } suite.True(processedAnEvent, "did not actually test CI and Interactive dimensions") From b634975cb25924fcc817ba0b709c29ca9043149c Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Mon, 14 Aug 2023 14:54:31 -0700 Subject: [PATCH 05/45] Added v0.40.1 changelog --- changelog.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/changelog.md b/changelog.md index bfc3292299..162340cff7 100644 --- a/changelog.md +++ b/changelog.md @@ -6,6 +6,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +### 0.40.1 + +### Added + +* State tool will now warn users if its executables are deleted during + installation, indicating a false-positive action from antivirus software. + +### Fixed + +* Fixed auto updates not being run (if you are on an older version: + run `state update`). +* Fixed a rare parsing panic that would happen when running particularly complex + builds. +* Fixed race condition during artifact installation that could lead to errors + like "Could not unpack artifact .. file already exists". + ### 0.40.0 ### Added From 88cb47d948ce54f1b42af638ce2332a947f23e13 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 25 Aug 2023 12:55:55 -0700 Subject: [PATCH 06/45] Update runtime-debug error handling --- pkg/platform/api/buildplanner/model/buildplan.go | 5 +++++ pkg/platform/model/buildplanner.go | 2 +- pkg/platform/runtime/runtime.go | 7 ++++++- pkg/platform/runtime/setup/setup.go | 16 +++++++++++++--- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index 8c7f64b137..ae69db29a3 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -92,6 +92,7 @@ func (o Operation) String() string { } type BuildPlannerError struct { + Err error ValidationErrors []string IsTransient bool } @@ -111,6 +112,10 @@ func (e *BuildPlannerError) UserError() string { } func (e *BuildPlannerError) Error() string { + if e.Err != nil { + return e.Err.Error() + } + // Append last five lines to error message offset := 0 numLines := len(e.ValidationErrors) diff --git a/pkg/platform/model/buildplanner.go b/pkg/platform/model/buildplanner.go index e6d72eee51..0939e6f70b 100644 --- a/pkg/platform/model/buildplanner.go +++ b/pkg/platform/model/buildplanner.go @@ -370,7 +370,7 @@ func processBuildPlannerError(bpErr error, fallbackMessage string) error { return locale.NewInputError("err_buildplanner_deprecated", "Encountered deprecation error: {{.V0}}", graphqlErr.Message) } } - return errs.Wrap(bpErr, fallbackMessage) + return &bpModel.BuildPlannerError{Err: errs.Wrap(bpErr, fallbackMessage)} } var versionRe = regexp.MustCompile(`^\d+(\.\d+)*$`) diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index 95a2a7b813..b5eb63d17d 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -192,7 +192,7 @@ func (r *Runtime) recordCompletion(err error) { errorType = "input" case errs.Matches(err, &model.SolverError{}): errorType = "solve" - case errs.Matches(err, &setup.BuildError{}) || errs.Matches(err, &buildlog.BuildError{}): + case errs.Matches(err, &setup.BuildError{}), errs.Matches(err, &buildlog.BuildError{}): errorType = "build" case errs.Matches(err, &bpModel.BuildPlannerError{}): errorType = "buildplan" @@ -206,6 +206,9 @@ func (r *Runtime) recordCompletion(err error) { case errs.Matches(err, &setup.ArtifactInstallError{}): errorType = "install" // Note: do not break because there could be download errors, and those take precedence + case errs.Matches(err, &setup.BuildError{}), errs.Matches(err, &buildlog.BuildError{}): + errorType = "build" + break // it only takes one build failure to report the runtime failure as due to build error } } } @@ -213,6 +216,8 @@ func (r *Runtime) recordCompletion(err error) { // and those errors actually caused the failure, not these. case errs.Matches(err, &setup.ProgressReportError{}) || errs.Matches(err, &buildlog.EventHandlerError{}): errorType = "progress" + case errs.Matches(err, &setup.ExecutorSetupError{}): + errorType = "postprocess" } var message string diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 15b85b36f6..51049739a2 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -79,6 +79,10 @@ type ArtifactSetupErrors struct { errs []error } +type ExecutorSetupError struct { + *errs.WrapperError +} + func (a *ArtifactSetupErrors) Error() string { var errors []string for _, err := range a.errs { @@ -189,7 +193,7 @@ func (s *Setup) Update() (rerr error) { // Update executors if err := s.updateExecutors(artifacts); err != nil { - return errs.Wrap(err, "Failed to update executors") + return ExecutorSetupError{errs.Wrap(err, "Failed to update executors")} } // Mark installation as completed @@ -745,12 +749,16 @@ func (s *Setup) moveToInstallPath(a artifact.ArtifactID, unpackedDir string, env func (s *Setup) downloadArtifact(a artifact.ArtifactDownload, targetFile string) (rerr error) { defer func() { if rerr != nil { - rerr = &ArtifactDownloadError{errs.Wrap(rerr, "Unable to download artifact")} + if !errs.Matches(rerr, &ProgressReportError{}) { + rerr = &ArtifactDownloadError{errs.Wrap(rerr, "Unable to download artifact")} + } + if err := s.handleEvent(events.ArtifactDownloadFailure{a.ArtifactID, rerr}); err != nil { rerr = errs.Wrap(rerr, "Could not handle ArtifactDownloadFailure event") return } } + if err := s.handleEvent(events.ArtifactDownloadSuccess{a.ArtifactID}); err != nil { rerr = errs.Wrap(rerr, "Could not handle ArtifactDownloadSuccess event") return @@ -765,7 +773,7 @@ func (s *Setup) downloadArtifact(a artifact.ArtifactDownload, targetFile string) b, err := httputil.GetWithProgress(artifactURL.String(), &progress.Report{ ReportSizeCb: func(size int) error { if err := s.handleEvent(events.ArtifactDownloadStarted{a.ArtifactID, size}); err != nil { - return errs.Wrap(err, "Could not handle ArtifactDownloadStarted event") + return ProgressReportError{errs.Wrap(err, "Could not handle ArtifactDownloadStarted event")} } return nil }, @@ -779,9 +787,11 @@ func (s *Setup) downloadArtifact(a artifact.ArtifactDownload, targetFile string) if err != nil { return errs.Wrap(err, "Download %s failed", artifactURL.String()) } + if err := fileutils.WriteFile(targetFile, b); err != nil { return errs.Wrap(err, "Writing download to target file %s failed", targetFile) } + return nil } From 72aecd649e6a3e7468f43a10a88896aa5cd46c1b Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 29 Aug 2023 13:09:15 -0700 Subject: [PATCH 07/45] Make deprecation errors also buildplanner errors --- pkg/platform/model/buildplanner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/platform/model/buildplanner.go b/pkg/platform/model/buildplanner.go index 0939e6f70b..cddded07d7 100644 --- a/pkg/platform/model/buildplanner.go +++ b/pkg/platform/model/buildplanner.go @@ -367,7 +367,7 @@ func processBuildPlannerError(bpErr error, fallbackMessage string) error { if errors.As(bpErr, graphqlErr) { code, ok := graphqlErr.Extensions[codeExtensionKey].(string) if ok && code == clientDeprecationErrorKey { - return locale.NewInputError("err_buildplanner_deprecated", "Encountered deprecation error: {{.V0}}", graphqlErr.Message) + return &bpModel.BuildPlannerError{Err: locale.NewInputError("err_buildplanner_deprecated", "Encountered deprecation error: {{.V0}}", graphqlErr.Message)} } } return &bpModel.BuildPlannerError{Err: errs.Wrap(bpErr, fallbackMessage)} From 88eeab0058c2ae50d8ed03fb6582617448633818 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 30 Aug 2023 16:51:21 -0400 Subject: [PATCH 08/45] Enrich service wait up/down errors with more debug info. This should help with rollbar reports. --- internal/ipc/client.go | 4 + internal/svcctl/debugdata.go | 139 +++++++++++++++++++++++++++++++++++ internal/svcctl/svcctl.go | 41 ++++++++--- 3 files changed, 172 insertions(+), 12 deletions(-) create mode 100644 internal/svcctl/debugdata.go diff --git a/internal/ipc/client.go b/internal/ipc/client.go index dc6da797bf..d60e593ff1 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -23,6 +23,10 @@ func NewClient(n *SockPath) *Client { } } +func (c *Client) SockPath() *SockPath { + return c.sockpath +} + func (c *Client) Request(ctx context.Context, key string) (string, error) { spath := c.sockpath.String() conn, err := c.dialer.DialContext(ctx, network, spath) diff --git a/internal/svcctl/debugdata.go b/internal/svcctl/debugdata.go new file mode 100644 index 0000000000..a0c48bf814 --- /dev/null +++ b/internal/svcctl/debugdata.go @@ -0,0 +1,139 @@ +package svcctl + +import ( + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "time" + + "github.com/ActiveState/cli/internal/fileutils" + "github.com/ActiveState/cli/internal/logging" +) + +type execKind string + +const ( + startSvc execKind = "Start" + stopSvc execKind = "Stop" +) + +type debugData struct { + sockInfo *fileInfo + sockDirInfo *fileInfo + sockDirList []string + + execStart time.Time + execDur time.Duration + execKind execKind + + waitStart time.Time + waitAttempts []*waitAttempt + waitDur time.Duration +} + +func newDebugData(ipComm IPCommunicator, kind execKind) *debugData { + sock := ipComm.SockPath().String() + sockDir := filepath.Dir(sock) + + return &debugData{ + sockInfo: newFileInfo(sock), + sockDirInfo: newFileInfo(sockDir), + sockDirList: fileutils.ListDirSimple(sockDir, false), + execStart: time.Now(), + execKind: kind, + } +} + +func (d *debugData) Error() string { + var sockDirList string + for _, entry := range d.sockDirList { + sockDirList += "\n " + entry + } + if sockDirList == "" { + sockDirList = "No entries found" + } + + var attemptsMsg string + for _, wa := range d.waitAttempts { + attemptsMsg += "\n " + wa.LogString() + } + + return fmt.Sprintf(strings.TrimSpace(` +Sock Info : %s +Sock Dir Info : %s +Sock Dir List : %s +%s Start : %s +%s Duration : %s +Wait Start : %s +Wait Duration : %s +Wait Log: %s +`), + strings.ReplaceAll(d.sockInfo.LogString(), "\n", "\n "), + strings.ReplaceAll(d.sockDirInfo.LogString(), "\n", "\n "), + sockDirList, + d.execKind, d.execStart, + d.execKind, d.execDur, + d.waitStart, + d.waitDur, + attemptsMsg, + ) +} + +func (d *debugData) startWait() { + d.execDur = time.Since(d.execStart) + logging.Debug("Exec time before wait was %v", d.execDur) + d.waitStart = time.Now() +} + +func (d *debugData) addWaitAttempt(start time.Time, iter int, timeout time.Duration) { + attempt := &waitAttempt{ + waitStart: d.waitStart, + start: start, + iter: iter, + timeout: timeout, + } + d.waitAttempts = append(d.waitAttempts, attempt) + logging.Debug("%s", attempt) +} + +func (d *debugData) stopWait() { + d.waitDur = time.Since(d.waitStart) + logging.Debug("Wait duration: %s", d.waitDur) +} + +type fileInfo struct { + path string + fs.FileInfo + osFIErr error +} + +func newFileInfo(path string) *fileInfo { + fi := &fileInfo{path: path} + fi.FileInfo, fi.osFIErr = os.Stat(path) + return fi +} + +func (f *fileInfo) LogString() string { + if f.osFIErr != nil { + return fmt.Sprintf("Path: %s Error: %s", f.path, f.osFIErr) + } + return fmt.Sprintf("Path: %s Size: %d Mode: %s ModTime: %s", f.path, f.Size(), f.Mode(), f.ModTime()) +} + +type waitAttempt struct { + waitStart time.Time + start time.Time + iter int + timeout time.Duration +} + +func (a *waitAttempt) String() string { + return fmt.Sprintf("Attempt %2d at %10s with timeout %v", + a.iter, a.start.Sub(a.waitStart).Round(time.Microsecond), a.timeout) +} + +func (a *waitAttempt) LogString() string { + return fmt.Sprintf("%2d: %10s/%v", a.iter, a.start.Sub(a.waitStart).Round(time.Microsecond), a.timeout) +} diff --git a/internal/svcctl/svcctl.go b/internal/svcctl/svcctl.go index e3b11ace73..df158ebe98 100644 --- a/internal/svcctl/svcctl.go +++ b/internal/svcctl/svcctl.go @@ -40,6 +40,7 @@ type IPCommunicator interface { Requester PingServer(context.Context) (time.Duration, error) StopServer(context.Context) error + SockPath() *ipc.SockPath } func NewIPCSockPathFromGlobals() *ipc.SockPath { @@ -153,12 +154,14 @@ func startAndWait(ctx context.Context, ipComm IPCommunicator, exec, argText stri args = args[:len(args)-1] } + debugInfo := newDebugData(ipComm, startSvc) + if _, err := exeutils.ExecuteAndForget(exec, args); err != nil { return locale.WrapError(err, "svcctl_cannot_exec_and_forget", "Cannot execute service in background: {{.V0}}", err.Error()) } logging.Debug("Waiting for service") - if err := waitUp(ctx, ipComm); err != nil { + if err := waitUp(ctx, ipComm, debugInfo); err != nil { return locale.WrapError(err, "svcctl_wait_startup_failed", "Waiting for service startup confirmation failed") } @@ -169,19 +172,23 @@ var ( waitTimeoutL10nKey = "svcctl_wait_timeout" ) -func waitUp(ctx context.Context, ipComm IPCommunicator) error { +func waitUp(ctx context.Context, ipComm IPCommunicator, debugInfo *debugData) error { + debugInfo.startWait() + defer debugInfo.stopWait() + start := time.Now() for try := 1; try <= pingRetryIterations; try++ { select { case <-ctx.Done(): - return locale.WrapError(ctx.Err(), waitTimeoutL10nKey, "", time.Since(start).String(), "1", constants.ForumsURL) + err := locale.WrapError(ctx.Err(), waitTimeoutL10nKey, "", time.Since(start).String(), "1", constants.ForumsURL) + return errs.Pack(err, debugInfo) default: } tryStart := time.Now() timeout := time.Millisecond * time.Duration(try*try) - logging.Debug("Attempt: %d, timeout: %v, total: %v", try, timeout, time.Since(start)) + debugInfo.addWaitAttempt(tryStart, try, timeout) if err := ping(ctx, ipComm, timeout); err != nil { // Timeout does not reveal enough info, try again. // We don't need to sleep for this type of error because, @@ -190,7 +197,8 @@ func waitUp(ctx context.Context, ipComm IPCommunicator) error { continue } if !errors.Is(err, ctlErrNotUp) { - return locale.WrapError(err, "svcctl_ping_failed", "Ping encountered unexpected failure: {{.V0}}", err.Error()) + err := locale.WrapError(err, "svcctl_ping_failed", "Ping encountered unexpected failure: {{.V0}}", err.Error()) + return errs.Pack(err, debugInfo) } elapsed := time.Since(tryStart) time.Sleep(timeout - elapsed) @@ -199,35 +207,42 @@ func waitUp(ctx context.Context, ipComm IPCommunicator) error { return nil } - return locale.NewError(waitTimeoutL10nKey, "", time.Since(start).Round(time.Millisecond).String(), "2", constants.ForumsURL) + err := locale.NewError(waitTimeoutL10nKey, "", time.Since(start).Round(time.Millisecond).String(), "2", constants.ForumsURL) + return errs.Pack(err, debugInfo) } func stopAndWait(ctx context.Context, ipComm IPCommunicator) error { + debugInfo := newDebugData(ipComm, stopSvc) + if err := ipComm.StopServer(ctx); err != nil { return locale.WrapError(err, "svcctl_stop_req_failed", "Service stop request failed") } logging.Debug("Waiting for service to die") - if err := waitDown(ctx, ipComm); err != nil { + if err := waitDown(ctx, ipComm, debugInfo); err != nil { return locale.WrapError(err, "svcctl_wait_shutdown_failed", "Waiting for service shutdown confirmation failed") } return nil } -func waitDown(ctx context.Context, ipComm IPCommunicator) error { +func waitDown(ctx context.Context, ipComm IPCommunicator, debugInfo *debugData) error { + debugInfo.startWait() + defer debugInfo.stopWait() + start := time.Now() for try := 1; try <= pingRetryIterations; try++ { select { case <-ctx.Done(): - return locale.WrapError(ctx.Err(), waitTimeoutL10nKey, "", time.Since(start).String(), "3", constants.ForumsURL) + err := locale.WrapError(ctx.Err(), waitTimeoutL10nKey, "", time.Since(start).String(), "3", constants.ForumsURL) + return errs.Pack(err, debugInfo) default: } tryStart := time.Now() timeout := time.Millisecond * time.Duration(try*try) - logging.Debug("Attempt: %d, timeout: %v, total: %v", try, timeout, time.Since(start)) + debugInfo.addWaitAttempt(tryStart, try, timeout) if err := ping(ctx, ipComm, timeout); err != nil { // Timeout does not reveal enough info, try again. // We don't need to sleep for this type of error because, @@ -242,14 +257,16 @@ func waitDown(ctx context.Context, ipComm IPCommunicator) error { return nil } if !errors.Is(err, ctlErrTempNotUp) { - return locale.WrapError(err, "svcctl_ping_failed", "Ping encountered unexpected failure: {{.V0}}", err.Error()) + err := locale.WrapError(err, "svcctl_ping_failed", "Ping encountered unexpected failure: {{.V0}}", err.Error()) + return errs.Pack(err, debugInfo) } } elapsed := time.Since(tryStart) time.Sleep(timeout - elapsed) } - return locale.NewError(waitTimeoutL10nKey, "", time.Since(start).Round(time.Millisecond).String(), "4", constants.ForumsURL) + err := locale.NewError(waitTimeoutL10nKey, "", time.Since(start).Round(time.Millisecond).String(), "4", constants.ForumsURL) + return errs.Pack(err, debugInfo) } func ping(ctx context.Context, ipComm IPCommunicator, timeout time.Duration) error { From c807070572589986f98a71392bbe6ebbd19094ee Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 31 Aug 2023 15:04:31 -0400 Subject: [PATCH 09/45] Include info on how state-svc was executed (e.g. on startup) in debug info. --- cmd/state-svc/autostart/autostart.go | 2 +- cmd/state-svc/main.go | 13 +++++++++++-- internal/svcctl/debugdata.go | 7 ++++++- internal/svcctl/svcctl.go | 4 ++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/cmd/state-svc/autostart/autostart.go b/cmd/state-svc/autostart/autostart.go index b979d89ed9..6e79056518 100644 --- a/cmd/state-svc/autostart/autostart.go +++ b/cmd/state-svc/autostart/autostart.go @@ -13,7 +13,7 @@ import ( var Options = autostart.Options{ Name: constants.SvcAppName, LaunchFileName: constants.SvcLaunchFileName, - Args: []string{"start"}, + Args: []string{"start", "--startup"}, } func RegisterConfigListener(cfg *config.Instance) error { diff --git a/cmd/state-svc/main.go b/cmd/state-svc/main.go index 7189d0a00e..93de26d4b1 100644 --- a/cmd/state-svc/main.go +++ b/cmd/state-svc/main.go @@ -122,16 +122,25 @@ func run(cfg *config.Instance) error { ) var foregroundArgText string + var startup bool cmd.AddChildren( captain.NewCommand( cmdStart, "", "Start the ActiveState Service (Background)", - p, nil, nil, + p, + []*captain.Flag{ + {Name: "startup", Value: &startup}, // differentiate between autostart and cli invocation + }, + nil, func(ccmd *captain.Command, args []string) error { logging.Debug("Running CmdStart") - return runStart(out, "svc-start:cli") + argText := "svc-start:cli" + if startup { + argText = "svc-start:startup" + } + return runStart(out, argText) }, ), captain.NewCommand( diff --git a/internal/svcctl/debugdata.go b/internal/svcctl/debugdata.go index a0c48bf814..d981380e51 100644 --- a/internal/svcctl/debugdata.go +++ b/internal/svcctl/debugdata.go @@ -20,6 +20,8 @@ const ( ) type debugData struct { + argText string + sockInfo *fileInfo sockDirInfo *fileInfo sockDirList []string @@ -33,11 +35,12 @@ type debugData struct { waitDur time.Duration } -func newDebugData(ipComm IPCommunicator, kind execKind) *debugData { +func newDebugData(ipComm IPCommunicator, kind execKind, argText string) *debugData { sock := ipComm.SockPath().String() sockDir := filepath.Dir(sock) return &debugData{ + argText: argText, sockInfo: newFileInfo(sock), sockDirInfo: newFileInfo(sockDir), sockDirList: fileutils.ListDirSimple(sockDir, false), @@ -61,6 +64,7 @@ func (d *debugData) Error() string { } return fmt.Sprintf(strings.TrimSpace(` +Arg Text : %s Sock Info : %s Sock Dir Info : %s Sock Dir List : %s @@ -70,6 +74,7 @@ Wait Start : %s Wait Duration : %s Wait Log: %s `), + d.argText, strings.ReplaceAll(d.sockInfo.LogString(), "\n", "\n "), strings.ReplaceAll(d.sockDirInfo.LogString(), "\n", "\n "), sockDirList, diff --git a/internal/svcctl/svcctl.go b/internal/svcctl/svcctl.go index df158ebe98..73eb8fd888 100644 --- a/internal/svcctl/svcctl.go +++ b/internal/svcctl/svcctl.go @@ -154,7 +154,7 @@ func startAndWait(ctx context.Context, ipComm IPCommunicator, exec, argText stri args = args[:len(args)-1] } - debugInfo := newDebugData(ipComm, startSvc) + debugInfo := newDebugData(ipComm, startSvc, argText) if _, err := exeutils.ExecuteAndForget(exec, args); err != nil { return locale.WrapError(err, "svcctl_cannot_exec_and_forget", "Cannot execute service in background: {{.V0}}", err.Error()) @@ -212,7 +212,7 @@ func waitUp(ctx context.Context, ipComm IPCommunicator, debugInfo *debugData) er } func stopAndWait(ctx context.Context, ipComm IPCommunicator) error { - debugInfo := newDebugData(ipComm, stopSvc) + debugInfo := newDebugData(ipComm, stopSvc, "") if err := ipComm.StopServer(ctx); err != nil { return locale.WrapError(err, "svcctl_stop_req_failed", "Service stop request failed") From 6a2794ac09115f989e837ada3d3a575a9810ea42 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 31 Aug 2023 16:05:29 -0400 Subject: [PATCH 10/45] Renamed startup flag to autostart based on PR feedback. --- cmd/state-svc/autostart/autostart.go | 2 +- cmd/state-svc/main.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/state-svc/autostart/autostart.go b/cmd/state-svc/autostart/autostart.go index 6e79056518..245843201d 100644 --- a/cmd/state-svc/autostart/autostart.go +++ b/cmd/state-svc/autostart/autostart.go @@ -13,7 +13,7 @@ import ( var Options = autostart.Options{ Name: constants.SvcAppName, LaunchFileName: constants.SvcLaunchFileName, - Args: []string{"start", "--startup"}, + Args: []string{"start", "--autostart"}, } func RegisterConfigListener(cfg *config.Instance) error { diff --git a/cmd/state-svc/main.go b/cmd/state-svc/main.go index 93de26d4b1..a61f981ddb 100644 --- a/cmd/state-svc/main.go +++ b/cmd/state-svc/main.go @@ -122,7 +122,7 @@ func run(cfg *config.Instance) error { ) var foregroundArgText string - var startup bool + var autostart bool cmd.AddChildren( captain.NewCommand( @@ -131,14 +131,14 @@ func run(cfg *config.Instance) error { "Start the ActiveState Service (Background)", p, []*captain.Flag{ - {Name: "startup", Value: &startup}, // differentiate between autostart and cli invocation + {Name: "autostart", Value: &autostart, Hidden: true}, // differentiate between autostart and cli invocation }, nil, func(ccmd *captain.Command, args []string) error { logging.Debug("Running CmdStart") argText := "svc-start:cli" - if startup { - argText = "svc-start:startup" + if autostart { + argText += "-autostart" } return runStart(out, argText) }, From d31b0cec3f055575beb4558e454dce5504b53f8e Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 31 Aug 2023 17:12:03 -0400 Subject: [PATCH 11/45] Updated autostart text based on PR feedback. --- cmd/state-svc/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/state-svc/main.go b/cmd/state-svc/main.go index a61f981ddb..22faee2da8 100644 --- a/cmd/state-svc/main.go +++ b/cmd/state-svc/main.go @@ -138,7 +138,7 @@ func run(cfg *config.Instance) error { logging.Debug("Running CmdStart") argText := "svc-start:cli" if autostart { - argText += "-autostart" + argText = "svc-start:auto" } return runStart(out, argText) }, From ef5b08a4be158bd0f3998dc5665bfcaa05427d30 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 1 Sep 2023 14:31:09 -0700 Subject: [PATCH 12/45] Initial implementation of user-facing errors --- internal/errs/errs.go | 15 +++++++ internal/runbits/hello_example.go | 6 ++- internal/runners/hello/hello_example.go | 53 ++++++++++++++++++++++--- pkg/cmdlets/errors/errors.go | 8 ++-- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 252b4543b8..6212f70538 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -36,6 +36,10 @@ type TransientError interface { IsTransient() } +type UserFacingError interface { + UserFacingError() error +} + // PackedErrors represents a collection of errors that aren't necessarily related to each other // note that rtutils replicates this functionality to avoid import cycles type PackedErrors struct { @@ -258,3 +262,14 @@ func Unpack(err error) []error { } return result } + +func UserFacing(err error) (error, bool) { + errs := Unpack(err) + for _, err := range errs { + if uerr, ok := err.(UserFacingError); ok { + return uerr.UserFacingError(), true + } + } + + return err, false +} diff --git a/internal/runbits/hello_example.go b/internal/runbits/hello_example.go index 93c0c65c32..194315059c 100644 --- a/internal/runbits/hello_example.go +++ b/internal/runbits/hello_example.go @@ -5,10 +5,14 @@ import ( "github.com/ActiveState/cli/internal/output" ) +type NoNameProvidedError struct { + *locale.LocalizedError +} + func SayHello(out output.Outputer, name string) error { if name == "" { // Errors that are due to USER input should use `NewInputError` or `WrapInputError` - return locale.NewInputError("hello_err_no_name", "No name provided.") + return &NoNameProvidedError{locale.NewInputError("hello_err_no_name", "No name provided.")} } out.Print(locale.Tl("hello_message", "Hello, {{.V0}}!", name)) diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index d336a88640..88d3b270aa 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -24,6 +24,16 @@ type primeable interface { primer.Projecter } +// HelloUserFacingError is an error from this runner that is intended to be +// shown to the user. It is a wrapper around a localized error. +type HelloUserFacingError struct { + *locale.LocalizedError +} + +func (e *HelloUserFacingError) UserFacingError() error { + return e +} + // RunParams defines the parameters needed to execute a given runner. These // values are typically collected from flags and arguments entered into the // cli, but there is no reason that they couldn't be set in another manner. @@ -37,9 +47,7 @@ type RunParams struct { // can be set. If no default or construction-time values are necessary, direct // construction of RunParams is fine, and this construction func may be dropped. func NewRunParams() *RunParams { - return &RunParams{ - Name: "Friend", - } + return &RunParams{} } // Hello defines the app-level dependencies that are accessible within the Run @@ -58,8 +66,40 @@ func New(p primeable) *Hello { } } -// Run contains the scope in which the hello runner logic is executed. +// Run wraps the scope in which the hello runner logic is executed. This +// is to ensure we catch specific errors that we are looking for and wrap +// them in a user-facing error. func (h *Hello) Run(params *RunParams) error { + err := h.run(params) + if err != nil { + for _, unpackedError := range errs.Unpack(err) { + switch unpackedError.(type) { + case *runbits.NoNameProvidedError: + // Errors that we are looking for should be wrapped in a user-facing error. + // Ensure we wrap the top-level error returned from the runner and not + // the unpacked error that we are inspecting. + return &HelloUserFacingError{ + locale.WrapInputError( + err, + "hello_err_no_name", + "Cannot say hello because no name was provided.", + ), + } + + default: + // If this is not a specific error we are looking for, do nothing. + continue + } + } + + return locale.WrapError(err, "hello_cannot_say", "Cannot say hello.") + } + + return nil +} + +// Run contains the scope in which the hello runner logic is executed. +func (h *Hello) run(params *RunParams) error { h.out.Print(locale.Tl("hello_notice", "This command is for example use only")) if h.project == nil { @@ -79,7 +119,7 @@ func (h *Hello) Run(params *RunParams) error { // runners. Runners should NEVER invoke other runners. if err := runbits.SayHello(h.out, params.Name); err != nil { // Errors should nearly always be localized. - return locale.WrapError( + return locale.WrapInputError( err, "hello_cannot_say", "Cannot say hello.", ) } @@ -127,7 +167,8 @@ func currentCommitMessage(proj *project.Project) (string, error) { commit, err := model.GetCommit(proj.CommitUUID()) if err != nil { - return "", locale.NewError( + return "", locale.WrapError( + err, "hello_info_err_get_commitr", "Cannot get commit from server", ) } diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index a47b2747c0..00287bdb8b 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -93,8 +93,6 @@ func ParseUserFacing(err error) (int, error) { return 0, nil } - _, hasMarshaller := err.(output.Marshaller) - // unwrap exit code before we remove un-localized wrapped errors from err variable code := errs.ParseExitCode(err) @@ -103,8 +101,10 @@ func ParseUserFacing(err error) (int, error) { return code, nil } - if hasMarshaller { - return code, err + uerr, ok := errs.UserFacing(err) + if ok { + logging.Debug("Returning user facing error, error stack: \n%s", errs.JoinMessage(err)) + return code, uerr } return code, &OutputError{err} From 08a51666d6f58c5d319e00d55365b4240f989c80 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 7 Sep 2023 14:47:17 -0700 Subject: [PATCH 13/45] Move some error handling to errs package Adjust how we handle errors in example runner Add back marshaller handling in user facing error parsing --- internal/errs/errs.go | 32 ++++++++++++++++++------- internal/runners/hello/hello_example.go | 24 +++++-------------- pkg/cmdlets/errors/errors.go | 18 ++++++++++---- 3 files changed, 44 insertions(+), 30 deletions(-) diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 6212f70538..9c19eca9c1 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -36,8 +36,17 @@ type TransientError interface { IsTransient() } -type UserFacingError interface { - UserFacingError() error +type UserFacingError struct { + wrapped error + message string +} + +func (e UserFacingError) Error() string { + return JoinMessage(Wrap(e.wrapped, e.message)) +} + +func (e UserFacingError) UserError() string { + return e.message } // PackedErrors represents a collection of errors that aren't necessarily related to each other @@ -108,6 +117,13 @@ func Wrap(wrapTarget error, message string, args ...interface{}) *WrapperError { return newError(msg, wrapTarget) } +func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) *UserFacingError { + return &UserFacingError{ + wrapTarget, + fmt.Sprintf(message, args...), + } +} + // Pack creates a new error that packs the given errors together, allowing for multiple errors to be returned func Pack(err error, errs ...error) error { return &PackedErrors{append([]error{err}, errs...)} @@ -263,12 +279,12 @@ func Unpack(err error) []error { return result } -func UserFacing(err error) (error, bool) { - errs := Unpack(err) - for _, err := range errs { - if uerr, ok := err.(UserFacingError); ok { - return uerr.UserFacingError(), true - } +// IsUserFacing identifies whether this error was curated for end-users and +// returns that error. This is NOT the same as an error that is localized. +func IsUserFacing(err error) (error, bool) { + var userFacingError *UserFacingError + if errors.As(err, &userFacingError) { + return userFacingError, true } return err, false diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 88d3b270aa..19febb212a 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -72,24 +72,12 @@ func New(p primeable) *Hello { func (h *Hello) Run(params *RunParams) error { err := h.run(params) if err != nil { - for _, unpackedError := range errs.Unpack(err) { - switch unpackedError.(type) { - case *runbits.NoNameProvidedError: - // Errors that we are looking for should be wrapped in a user-facing error. - // Ensure we wrap the top-level error returned from the runner and not - // the unpacked error that we are inspecting. - return &HelloUserFacingError{ - locale.WrapInputError( - err, - "hello_err_no_name", - "Cannot say hello because no name was provided.", - ), - } - - default: - // If this is not a specific error we are looking for, do nothing. - continue - } + switch { + case errs.Matches(err, &runbits.NoNameProvidedError{}): + // Errors that we are looking for should be wrapped in a user-facing error. + // Ensure we wrap the top-level error returned from the runner and not + // the unpacked error that we are inspecting. + return errs.WrapUserFacingError(err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) } return locale.WrapError(err, "hello_cannot_say", "Cannot say hello.") diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index 00287bdb8b..3c74eb216d 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -93,18 +93,28 @@ func ParseUserFacing(err error) (int, error) { return 0, nil } + _, hasMarshaller := err.(output.Marshaller) + // unwrap exit code before we remove un-localized wrapped errors from err variable code := errs.ParseExitCode(err) + // If there is a user facing error in the error stack we want to ensure + // that is it forwarded to the user. + uerr, ok := errs.IsUserFacing(err) + if ok { + logging.Debug("Returning user facing error, error stack: \n%s", errs.JoinMessage(err)) + return code, &OutputError{uerr} + } + if errs.IsSilent(err) { logging.Debug("Suppressing silent failure: %v", err.Error()) return code, nil } - uerr, ok := errs.UserFacing(err) - if ok { - logging.Debug("Returning user facing error, error stack: \n%s", errs.JoinMessage(err)) - return code, uerr + // If the error already has a marshalling function we do not want to wrap + // it again in the OutputError type. + if hasMarshaller { + return code, err } return code, &OutputError{err} From 2181e6143915386ca53010628fe31dc6b02b681e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 7 Sep 2023 14:48:21 -0700 Subject: [PATCH 14/45] Change the error wrapping --- internal/runners/hello/hello_example.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 19febb212a..a27afcb7f5 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -107,7 +107,7 @@ func (h *Hello) run(params *RunParams) error { // runners. Runners should NEVER invoke other runners. if err := runbits.SayHello(h.out, params.Name); err != nil { // Errors should nearly always be localized. - return locale.WrapInputError( + return locale.WrapError( err, "hello_cannot_say", "Cannot say hello.", ) } From ad399cf44a69e7f024cc17ae34c5413c372fbdd6 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 8 Sep 2023 15:36:29 -0700 Subject: [PATCH 15/45] Update reciever to be pointer Add back interface Remove unused error type --- internal/errs/errs.go | 17 +++++++++++------ internal/runners/hello/hello_example.go | 10 ---------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 9c19eca9c1..342185540c 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -36,16 +36,21 @@ type TransientError interface { IsTransient() } -type UserFacingError struct { +type UserFacingError interface { + error + UserError() string +} + +type userFacingError struct { wrapped error message string } -func (e UserFacingError) Error() string { +func (e *userFacingError) Error() string { return JoinMessage(Wrap(e.wrapped, e.message)) } -func (e UserFacingError) UserError() string { +func (e *userFacingError) UserError() string { return e.message } @@ -117,8 +122,8 @@ func Wrap(wrapTarget error, message string, args ...interface{}) *WrapperError { return newError(msg, wrapTarget) } -func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) *UserFacingError { - return &UserFacingError{ +func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) *userFacingError { + return &userFacingError{ wrapTarget, fmt.Sprintf(message, args...), } @@ -282,7 +287,7 @@ func Unpack(err error) []error { // IsUserFacing identifies whether this error was curated for end-users and // returns that error. This is NOT the same as an error that is localized. func IsUserFacing(err error) (error, bool) { - var userFacingError *UserFacingError + var userFacingError UserFacingError if errors.As(err, &userFacingError) { return userFacingError, true } diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index a27afcb7f5..3093963af5 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -24,16 +24,6 @@ type primeable interface { primer.Projecter } -// HelloUserFacingError is an error from this runner that is intended to be -// shown to the user. It is a wrapper around a localized error. -type HelloUserFacingError struct { - *locale.LocalizedError -} - -func (e *HelloUserFacingError) UserFacingError() error { - return e -} - // RunParams defines the parameters needed to execute a given runner. These // values are typically collected from flags and arguments entered into the // cli, but there is no reason that they couldn't be set in another manner. From c92a964f59b566029a58aeda0923fecb4cb29cc5 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 13:26:12 -0400 Subject: [PATCH 16/45] Process PlanningError build plan types as errors. Not all of these types have non-nil Error fields. --- pkg/platform/api/buildplanner/model/buildplan.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index ae69db29a3..99b6c12750 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -278,10 +278,6 @@ func ProcessCommitError(commit *Commit, fallbackMessage string) error { } func ProcessBuildError(build *Build, fallbackMessage string) error { - if build.Error == nil { - return errs.New(fallbackMessage) - } - if build.Type == PlanningErrorType { var errs []string var isTransient bool @@ -304,6 +300,8 @@ func ProcessBuildError(build *Build, fallbackMessage string) error { ValidationErrors: errs, IsTransient: isTransient, } + } else if build.Error == nil { + return errs.New(fallbackMessage) } return locale.NewInputError("err_buildplanner_build", "Encountered error processing build response") From 39bf2f3a562667f3f36270d626f2f4e262779359 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 13:59:35 -0400 Subject: [PATCH 17/45] Mark `state init` as stable. --- cmd/state/internal/cmdtree/init.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/state/internal/cmdtree/init.go b/cmd/state/internal/cmdtree/init.go index 57ece7db64..08d2f33a80 100644 --- a/cmd/state/internal/cmdtree/init.go +++ b/cmd/state/internal/cmdtree/init.go @@ -48,5 +48,5 @@ func newInitCommand(prime *primer.Values) *captain.Command { func(ccmd *captain.Command, _ []string) error { return initRunner.Run(¶ms) }, - ).SetGroup(EnvironmentSetupGroup).SetUnstable(true) + ).SetGroup(EnvironmentSetupGroup) } From 1670009b5109627eea741895c419cfd1ac1437e5 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 11 Sep 2023 11:09:33 -0700 Subject: [PATCH 18/45] Only localize user-facing errors --- internal/errs/centralized.go | 5 ++++ internal/errs/errs.go | 18 ++++++++++++++ internal/runners/hello/hello_example.go | 33 +++++++++++-------------- 3 files changed, 38 insertions(+), 18 deletions(-) create mode 100644 internal/errs/centralized.go diff --git a/internal/errs/centralized.go b/internal/errs/centralized.go new file mode 100644 index 0000000000..a86557d417 --- /dev/null +++ b/internal/errs/centralized.go @@ -0,0 +1,5 @@ +package errs + +type ErrNoProject struct { + *WrapperError +} diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 342185540c..0f0e9bf976 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -44,6 +44,7 @@ type UserFacingError interface { type userFacingError struct { wrapped error message string + tips []string } func (e *userFacingError) Error() string { @@ -122,13 +123,30 @@ func Wrap(wrapTarget error, message string, args ...interface{}) *WrapperError { return newError(msg, wrapTarget) } +func NewUserFacingError(message string, args ...interface{}) *userFacingError { + return &userFacingError{ + nil, + fmt.Sprintf(message, args...), + nil, + } +} + func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) *userFacingError { return &userFacingError{ wrapTarget, fmt.Sprintf(message, args...), + nil, } } +func (e *userFacingError) AddTips(tips ...string) { + e.tips = append(e.tips, tips...) +} + +func (e *userFacingError) ErrorTips() []string { + return e.tips +} + // Pack creates a new error that packs the given errors together, allowing for multiple errors to be returned func Pack(err error, errs ...error) error { return &PackedErrors{append([]error{err}, errs...)} diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 3093963af5..997de2ad52 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -68,9 +68,17 @@ func (h *Hello) Run(params *RunParams) error { // Ensure we wrap the top-level error returned from the runner and not // the unpacked error that we are inspecting. return errs.WrapUserFacingError(err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) + case errs.Matches(err, &errs.ErrNoProject{}): + err := errs.WrapUserFacingError(err, locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory.")) + + // It's useful to offer users reasonable tips on recourses. + return errs.AddTips(err, locale.Tl( + "hello_suggest_checkout", + "Try using [ACTIONABLE]`state checkout`[/RESET] first.", + )) } - return locale.WrapError(err, "hello_cannot_say", "Cannot say hello.") + return errs.Wrap(err, "Cannot say hello") } return nil @@ -81,15 +89,7 @@ func (h *Hello) run(params *RunParams) error { h.out.Print(locale.Tl("hello_notice", "This command is for example use only")) if h.project == nil { - err := locale.NewInputError( - "hello_info_err_no_project", "Not in a project directory.", - ) - - // It's useful to offer users reasonable tips on recourses. - return errs.AddTips(err, locale.Tl( - "hello_suggest_checkout", - "Try using [ACTIONABLE]`state checkout`[/RESET] first.", - )) + return &errs.ErrNoProject{errs.New("Not in a project directory")} } // Reusable runner logic is contained within the runbits package. @@ -97,8 +97,8 @@ func (h *Hello) run(params *RunParams) error { // runners. Runners should NEVER invoke other runners. if err := runbits.SayHello(h.out, params.Name); err != nil { // Errors should nearly always be localized. - return locale.WrapError( - err, "hello_cannot_say", "Cannot say hello.", + return errs.Wrap( + err, "Cannot say hello.", ) } @@ -116,8 +116,8 @@ func (h *Hello) run(params *RunParams) error { // Grab data from the platform. commitMsg, err := currentCommitMessage(h.project) if err != nil { - err = locale.WrapError( - err, "hello_info_err_get_commit_msg", " Cannot get commit message", + err = errs.Wrap( + err, "Cannot get commit message", ) return errs.AddTips( err, @@ -145,10 +145,7 @@ func currentCommitMessage(proj *project.Project) (string, error) { commit, err := model.GetCommit(proj.CommitUUID()) if err != nil { - return "", locale.WrapError( - err, - "hello_info_err_get_commitr", "Cannot get commit from server", - ) + return "", errs.Wrap(err, "Cannot get commit from server") } commitMsg := locale.Tl("hello_info_warn_no_commit", "Commit description not provided.") From 345879a59cb1db0a4218ac0d4bff7a967ed1ccdd Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 14:17:07 -0400 Subject: [PATCH 19/45] Ask Rollbar to log user IPs for us. --- internal/rollbar/rollbar.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/rollbar/rollbar.go b/internal/rollbar/rollbar.go index 7b7e5f88d7..688033e3bf 100644 --- a/internal/rollbar/rollbar.go +++ b/internal/rollbar/rollbar.go @@ -94,10 +94,7 @@ func SetupRollbar(token string) { data["request"] = map[string]string{} } if request, ok := data["request"].(map[string]string); ok { - // Rollbar specially interprets the body["request"]["user_ip"] key, but it does not have to - // actually be an IP address. Use device_id for now. We may want to use real IP later if it - // is easily accessible (Rollbar does not provide it for us with non-http.Request errors). - request["user_ip"] = uniqid.Text() + request["user_ip"] = "$remote_ip" // ask Rollbar to log the user's IP } }) From 70fa6422365c660aecb524a0f818a6a5fbca2954 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 17:13:35 -0400 Subject: [PATCH 20/45] Add authentication tips to build planner project not found errors. Also cleaned up related tips in revert errors. --- internal/locale/locales/en-us.yaml | 2 ++ internal/runners/revert/revert.go | 7 +++++-- pkg/platform/api/buildplanner/model/buildplan.go | 9 ++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index 91889d7882..3bdcb21aad 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -1754,6 +1754,8 @@ err_user_network_solution: other: | Please ensure your device has access to internet during installation. Make sure software like a VPN, Firewall or Anti-Virus are not blocking your connectivity. If your issue persists consider reporting it on our forums at {{.V0}}. +err_revert_get_commit: + other: "Could not fetch commit details for commit with ID: {{.V0}}" err_revert_refresh: other: Could not get revert commit to check if changes were indeed made err_revert_commit: diff --git a/internal/runners/revert/revert.go b/internal/runners/revert/revert.go index 4239973b97..0f1c22b156 100644 --- a/internal/runners/revert/revert.go +++ b/internal/runners/revert/revert.go @@ -76,7 +76,7 @@ func (r *Revert) Run(params *Params) error { priorCommits, err := model.CommitHistoryPaged(commitID, 0, 2) if err != nil { return errs.AddTips( - locale.WrapError(err, "err_revert_get_commit", "Could not fetch commit details for commit with ID: {{.V0}}", params.CommitID), + locale.WrapError(err, "err_revert_get_commit", "", params.CommitID), locale.T("tip_private_project_auth"), ) } @@ -90,7 +90,10 @@ func (r *Revert) Run(params *Params) error { var err error targetCommit, err = model.GetCommitWithinCommitHistory(latestCommit, commitID) if err != nil { - return locale.WrapError(err, "err_revert_to_get_commit", "Could not fetch commit details for commit with ID: {{.V0}}", params.CommitID) + return errs.AddTips( + locale.WrapError(err, "err_revert_get_commit", "", params.CommitID), + locale.T("tip_private_project_auth"), + ) } fromCommit = latestCommit toCommit = targetCommit.CommitID diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index ae69db29a3..479ec1e604 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -310,12 +310,11 @@ func ProcessBuildError(build *Build, fallbackMessage string) error { } func ProcessProjectError(project *Project, fallbackMessage string) error { - if project.Error == nil { - return errs.New(fallbackMessage) - } - if project.Type == NotFoundErrorType { - return locale.NewInputError("err_buildplanner_project_not_found", "Unable to find project, recieved message: {{.V0}}", project.Message) + return errs.AddTips( + locale.NewInputError("err_buildplanner_project_not_found", "Unable to find project, received message: {{.V0}}", project.Message), + locale.T("tip_private_project_auth"), + ) } return errs.New(fallbackMessage) From f855977929a540e319b729ab68ebba8d96753909 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 11 Sep 2023 14:19:47 -0700 Subject: [PATCH 21/45] Use defer func to process errors --- internal/errs/errs.go | 16 ++++++++-------- internal/runbits/hello_example.go | 5 +++-- internal/runners/hello/hello_example.go | 21 ++++++++++----------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 0f0e9bf976..2c45abf6f6 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -55,6 +55,14 @@ func (e *userFacingError) UserError() string { return e.message } +func (e *userFacingError) AddTips(tips ...string) { + e.tips = append(e.tips, tips...) +} + +func (e *userFacingError) ErrorTips() []string { + return e.tips +} + // PackedErrors represents a collection of errors that aren't necessarily related to each other // note that rtutils replicates this functionality to avoid import cycles type PackedErrors struct { @@ -139,14 +147,6 @@ func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) } } -func (e *userFacingError) AddTips(tips ...string) { - e.tips = append(e.tips, tips...) -} - -func (e *userFacingError) ErrorTips() []string { - return e.tips -} - // Pack creates a new error that packs the given errors together, allowing for multiple errors to be returned func Pack(err error, errs ...error) error { return &PackedErrors{append([]error{err}, errs...)} diff --git a/internal/runbits/hello_example.go b/internal/runbits/hello_example.go index 194315059c..53e852d31f 100644 --- a/internal/runbits/hello_example.go +++ b/internal/runbits/hello_example.go @@ -1,18 +1,19 @@ package runbits import ( + "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/output" ) type NoNameProvidedError struct { - *locale.LocalizedError + *errs.WrapperError } func SayHello(out output.Outputer, name string) error { if name == "" { // Errors that are due to USER input should use `NewInputError` or `WrapInputError` - return &NoNameProvidedError{locale.NewInputError("hello_err_no_name", "No name provided.")} + return &NoNameProvidedError{errs.New("No name provided.")} } out.Print(locale.Tl("hello_message", "Hello, {{.V0}}!", name)) diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 997de2ad52..756ead946d 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -59,33 +59,32 @@ func New(p primeable) *Hello { // Run wraps the scope in which the hello runner logic is executed. This // is to ensure we catch specific errors that we are looking for and wrap // them in a user-facing error. -func (h *Hello) Run(params *RunParams) error { - err := h.run(params) +func processError(err *error) { if err != nil { switch { - case errs.Matches(err, &runbits.NoNameProvidedError{}): + case errs.Matches(*err, &runbits.NoNameProvidedError{}): // Errors that we are looking for should be wrapped in a user-facing error. // Ensure we wrap the top-level error returned from the runner and not // the unpacked error that we are inspecting. - return errs.WrapUserFacingError(err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) - case errs.Matches(err, &errs.ErrNoProject{}): - err := errs.WrapUserFacingError(err, locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory.")) + *err = errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) + case errs.Matches(*err, &errs.ErrNoProject{}): + werr := errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory.")) // It's useful to offer users reasonable tips on recourses. - return errs.AddTips(err, locale.Tl( + *err = errs.AddTips(werr, locale.Tl( "hello_suggest_checkout", "Try using [ACTIONABLE]`state checkout`[/RESET] first.", )) } - return errs.Wrap(err, "Cannot say hello") + *err = errs.Wrap(*err, "Cannot say hello") } - - return nil } // Run contains the scope in which the hello runner logic is executed. -func (h *Hello) run(params *RunParams) error { +func (h *Hello) Run(params *RunParams) (rerr error) { + defer processError(&rerr) + h.out.Print(locale.Tl("hello_notice", "This command is for example use only")) if h.project == nil { From 3acc2d55e2297c5cc15695eec0e3896c188ed025 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 17:44:21 -0400 Subject: [PATCH 22/45] Update failure analytics events should set target version, not version. --- cmd/state/autoupdate.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/state/autoupdate.go b/cmd/state/autoupdate.go index 1b38e3a1e9..02ad0f8b56 100644 --- a/cmd/state/autoupdate.go +++ b/cmd/state/autoupdate.go @@ -78,21 +78,21 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou if err != nil { if os.IsPermission(err) { an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ - Version: ptr.To(up.Version), - Error: ptr.To("Could not update the state tool due to insufficient permissions."), + TargetVersion: ptr.To(up.Version), + Error: ptr.To("Could not update the state tool due to insufficient permissions."), }) return false, locale.WrapInputError(err, locale.Tl("auto_update_permission_err", "", constants.DocumentationURL, errs.JoinMessage(err))) } if errs.Matches(err, &updater.ErrorInProgress{}) { an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ - Version: ptr.To(up.Version), - Error: ptr.To(anaConst.UpdateErrorInProgress), + TargetVersion: ptr.To(up.Version), + Error: ptr.To(anaConst.UpdateErrorInProgress), }) return false, nil } an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ - Version: ptr.To(up.Version), - Error: ptr.To(anaConst.UpdateErrorInstallFailed), + TargetVersion: ptr.To(up.Version), + Error: ptr.To(anaConst.UpdateErrorInstallFailed), }) return false, locale.WrapError(err, locale.T("auto_update_failed")) } From dd732b95c9212c0cac321e8c82de5ed2072c86bf Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 17:48:30 -0400 Subject: [PATCH 23/45] Missed a couple of instances. --- cmd/state/autoupdate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/state/autoupdate.go b/cmd/state/autoupdate.go index 02ad0f8b56..54ffc6c74e 100644 --- a/cmd/state/autoupdate.go +++ b/cmd/state/autoupdate.go @@ -109,14 +109,14 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou msg = anaConst.UpdateErrorRelaunch } an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.UpdateLabelFailed, &dimensions.Values{ - Version: ptr.To(up.Version), - Error: ptr.To(msg), + TargetVersion: ptr.To(up.Version), + Error: ptr.To(msg), }) return true, errs.Silence(errs.WrapExitCode(err, code)) } an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.UpdateLabelSuccess, &dimensions.Values{ - Version: ptr.To(up.Version), + TargetVersion: ptr.To(up.Version), }) return true, nil } From 59fd76b70d1892bf369d08d7656e175499a8a4c8 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 11 Sep 2023 15:09:01 -0700 Subject: [PATCH 24/45] Update comment --- internal/runners/hello/hello_example.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 756ead946d..b125532799 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -56,9 +56,9 @@ func New(p primeable) *Hello { } } -// Run wraps the scope in which the hello runner logic is executed. This -// is to ensure we catch specific errors that we are looking for and wrap -// them in a user-facing error. +// processError contains the scope in which errors are processed. This is +// useful for ensuring that errors are wrapped in a user-facing error and +// localized. func processError(err *error) { if err != nil { switch { From 0af6020aee408b28548b453ce32f1de7fa7488bf Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 11 Sep 2023 16:07:04 -0700 Subject: [PATCH 25/45] Use float for number value --- pkg/platform/api/buildplanner/model/buildplan.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index a00cc17220..e24117a660 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -486,7 +486,7 @@ type Artifact struct { // Error fields Errors []string `json:"errors"` - Attempts string `json:"attempts"` + Attempts float64 `json:"attempts"` NextAttempt string `json:"nextAttempt"` } From ba706191125bf0fbddd56e533c4f7815c60934f0 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 11 Sep 2023 19:10:12 -0400 Subject: [PATCH 26/45] `state auth signup` sends the user to the signup page instead of the login page. --- internal/runners/auth/signup.go | 2 +- pkg/cmdlets/auth/login.go | 37 ++++++++++++++++++++++++++------- pkg/cmdlets/auth/signup.go | 16 +++++++++++++- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/internal/runners/auth/signup.go b/internal/runners/auth/signup.go index 9d9adad467..1b7e8bb656 100644 --- a/internal/runners/auth/signup.go +++ b/internal/runners/auth/signup.go @@ -30,7 +30,7 @@ func (s *Signup) Run(params *SignupParams) error { } if !params.Prompt { - return authlet.AuthenticateWithBrowser(s.Outputer, s.Auth, s.Prompter) // user can sign up from this page too + return authlet.SignupWithBrowser(s.Outputer, s.Auth, s.Prompter) } return authlet.Signup(s.Configurable, s.Outputer, s.Prompter, s.Auth) } diff --git a/pkg/cmdlets/auth/login.go b/pkg/cmdlets/auth/login.go index 896c1d5a28..9c32358882 100644 --- a/pkg/cmdlets/auth/login.go +++ b/pkg/cmdlets/auth/login.go @@ -1,8 +1,12 @@ package auth import ( + "fmt" + "net/url" + "strings" "time" + "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/keypairs" "github.com/ActiveState/cli/internal/locale" @@ -121,7 +125,7 @@ func RequireAuthentication(message string, cfg keypairs.Configurable, out output return errs.Wrap(err, "Authenticate failed") } case locale.T("prompt_signup_browser_action"): - if err := AuthenticateWithBrowser(out, auth, prompt); err != nil { // user can sign up from this page too + if err := SignupWithBrowser(out, auth, prompt); err != nil { return errs.Wrap(err, "Signup failed") } case locale.T("prompt_signup_action"): @@ -226,22 +230,41 @@ func promptToken(credentials *mono_models.Credentials, out output.Outputer, prom func AuthenticateWithBrowser(out output.Outputer, auth *authentication.Auth, prompt prompt.Prompter) error { logging.Debug("Authenticating with browser") + err := authenticateWithBrowser(out, auth, prompt, false) + if err != nil { + return errs.Wrap(err, "Error authenticating with browser") + } + + out.Notice(locale.T("auth_device_success")) + + return nil +} + +// authenticateWithBrowser authenticates after signup if applicable. +func authenticateWithBrowser(out output.Outputer, auth *authentication.Auth, prompt prompt.Prompter, signup bool) error { response, err := model.RequestDeviceAuthorization() if err != nil { return locale.WrapError(err, "err_auth_device") } + if response.VerificationURIComplete == nil { + return errs.New("Invalid response: Missing verification URL.") + } + + verificationUrl := *response.VerificationURIComplete + if signup { + baseUrl := strings.TrimPrefix(*response.VerificationURIComplete, "https://"+constants.PlatformURL) + verificationUrl = fmt.Sprintf("%s?nextRoute=%s", constants.PlatformSignupURL, url.QueryEscape(baseUrl)) + } + // Print code to user if response.UserCode == nil { return errs.New("Invalid response: Missing user code.") } - out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, *response.VerificationURIComplete)) + out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, verificationUrl)) // Open URL in browser - if response.VerificationURIComplete == nil { - return errs.New("Invalid response: Missing verification URL.") - } - err = OpenURI(*response.VerificationURIComplete) + err = OpenURI(verificationUrl) if err != nil { logging.Warning("Could not open browser: %v", err) out.Notice(locale.Tr("err_browser_open")) @@ -275,7 +298,5 @@ func AuthenticateWithBrowser(out output.Outputer, auth *authentication.Auth, pro return locale.WrapError(err, "err_auth_token", "Failed to create token after authenticating with browser.") } - out.Notice(locale.T("auth_device_success")) - return nil } diff --git a/pkg/cmdlets/auth/signup.go b/pkg/cmdlets/auth/signup.go index fd25b08838..1ebdf43408 100644 --- a/pkg/cmdlets/auth/signup.go +++ b/pkg/cmdlets/auth/signup.go @@ -1,12 +1,13 @@ package auth import ( + "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/keypairs" "github.com/ActiveState/cli/internal/locale" + "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/multilog" "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/prompt" - "github.com/ActiveState/cli/pkg/cmdlets/legalprompt" "github.com/ActiveState/cli/pkg/platform/api" "github.com/ActiveState/cli/pkg/platform/api/mono" @@ -165,3 +166,16 @@ func UsernameValidator(val interface{}) error { } return nil } + +func SignupWithBrowser(out output.Outputer, auth *authentication.Auth, prompt prompt.Prompter) error { + logging.Debug("Signing up with browser") + + err := authenticateWithBrowser(out, auth, prompt, true) + if err != nil { + return errs.Wrap(err, "Error signing up with browser") + } + + out.Notice(locale.Tl("auth_signup_success", "Successfully signed up and authorized this device")) + + return nil +} From 44abffacc0b50f42366968758ab94571d1f55cc2 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 12 Sep 2023 13:38:24 -0400 Subject: [PATCH 27/45] Add source to analytics events. This allows for differentiation between state tool, offline installer, executors, etc. --- cmd/state-installer/cmd.go | 42 +++++++++---------- cmd/state-offline-installer/install.go | 8 ++-- cmd/state-offline-uninstaller/uninstall.go | 10 ++--- cmd/state-svc/internal/resolver/resolver.go | 12 +++--- cmd/state-svc/internal/rtwatcher/watcher.go | 4 +- .../internal/server/generated/generated.go | 31 +++++++++----- cmd/state-svc/internal/server/server.go | 6 +-- cmd/state-svc/schema/schema.graphqls | 2 +- cmd/state/autoupdate.go | 14 +++---- cmd/state/internal/cmdtree/activate.go | 8 ++-- cmd/state/internal/cmdtree/tutorial.go | 4 +- go.sum | 4 -- internal/analytics/analytics.go | 4 +- internal/analytics/client/async/client.go | 12 +++--- internal/analytics/client/blackhole/client.go | 4 +- internal/analytics/client/sync/client.go | 14 +++---- .../client/sync/reporters/ga-state.go | 2 +- .../analytics/client/sync/reporters/pixel.go | 3 +- .../analytics/client/sync/reporters/test.go | 5 ++- internal/analytics/constants/constants.go | 15 +++++++ internal/captain/command.go | 10 ++--- internal/graph/generated.go | 9 ---- internal/prompt/prompt.go | 8 ++-- internal/runbits/activation/activation.go | 2 +- internal/runbits/requirements/requirements.go | 2 +- internal/runners/activate/activate.go | 2 +- internal/runners/config/set.go | 2 +- .../runners/deploy/uninstall/uninstall.go | 2 +- internal/runners/ppm/convert.go | 16 +++---- internal/runners/ppm/shim.go | 6 +-- internal/runners/tutorial/tutorial.go | 16 +++---- internal/svcctl/comm.go | 4 +- internal/updater/checker.go | 4 +- internal/updater/fetcher.go | 2 +- internal/updater/updater.go | 2 +- pkg/cmdlets/errors/errors.go | 2 +- pkg/cmdlets/git/git.go | 2 +- pkg/platform/api/svc/request/analytics.go | 35 +++++++++------- pkg/platform/model/svc.go | 4 +- pkg/platform/runtime/runtime.go | 16 +++++-- pkg/platform/runtime/setup/setup.go | 9 +++- test/integration/analytics_int_test.go | 2 +- test/integration/performance_svc_int_test.go | 2 +- 43 files changed, 195 insertions(+), 168 deletions(-) diff --git a/cmd/state-installer/cmd.go b/cmd/state-installer/cmd.go index a69b8b564b..b5b1faa54b 100644 --- a/cmd/state-installer/cmd.go +++ b/cmd/state-installer/cmd.go @@ -12,6 +12,7 @@ import ( "github.com/ActiveState/cli/internal/analytics" "github.com/ActiveState/cli/internal/analytics/client/sync" + anaConst "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/config" "github.com/ActiveState/cli/internal/constants" @@ -37,9 +38,6 @@ import ( "golang.org/x/crypto/ssh/terminal" ) -const AnalyticsCat = "installer" -const AnalyticsFunnelCat = "installer-funnel" - type Params struct { sourceInstaller string path string @@ -126,7 +124,7 @@ func main() { logging.Debug("Processed Args: %v", processedArgs) an = sync.New(cfg, nil, out) - an.Event(AnalyticsFunnelCat, "start") + an.Event(anaConst.CatInstallerFunnel, "start", anaConst.SrcStateTool) params := newParams() cmd := captain.NewCommand( @@ -193,30 +191,30 @@ func main() { }, ) - an.Event(AnalyticsFunnelCat, "pre-exec") + an.Event(anaConst.CatInstallerFunnel, "pre-exec", anaConst.SrcStateTool) err = cmd.Execute(processedArgs[1:]) if err != nil { errors.ReportError(err, cmd, an) if locale.IsInputError(err) { - an.EventWithLabel(AnalyticsCat, "input-error", errs.JoinMessage(err)) + an.EventWithLabel(anaConst.CatInstaller, "input-error", anaConst.SrcStateTool, errs.JoinMessage(err)) logging.Debug("Installer input error: " + errs.JoinMessage(err)) } else { - an.EventWithLabel(AnalyticsCat, "error", errs.JoinMessage(err)) + an.EventWithLabel(anaConst.CatInstaller, "error", anaConst.SrcStateTool, errs.JoinMessage(err)) multilog.Critical("Installer error: " + errs.JoinMessage(err)) } - an.EventWithLabel(AnalyticsFunnelCat, "fail", errs.JoinMessage(err)) + an.EventWithLabel(anaConst.CatInstallerFunnel, "fail", anaConst.SrcStateTool, errs.JoinMessage(err)) exitCode, err = errors.ParseUserFacing(err) if err != nil { out.Error(err) } } else { - an.Event(AnalyticsFunnelCat, "success") + an.Event(anaConst.CatInstallerFunnel, "success", anaConst.SrcStateTool) } } func execute(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, args []string, params *Params) error { - an.Event(AnalyticsFunnelCat, "exec") + an.Event(anaConst.CatInstallerFunnel, "exec", anaConst.SrcStateTool) if params.path == "" { var err error @@ -274,13 +272,13 @@ func execute(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, if params.isUpdate { route = "update" } - an.Event(AnalyticsFunnelCat, route) + an.Event(anaConst.CatInstallerFunnel, route, anaConst.SrcStateTool) // Check if state tool already installed if !params.isUpdate && !params.force && stateToolInstalled && !targetingSameBranch { logging.Debug("Cancelling out because State Tool is already installed") out.Print(fmt.Sprintf("State Tool Package Manager is already installed at [NOTICE]%s[/RESET]. To reinstall use the [ACTIONABLE]--force[/RESET] flag.", installPath)) - an.Event(AnalyticsFunnelCat, "already-installed") + an.Event(anaConst.CatInstallerFunnel, "already-installed", anaConst.SrcStateTool) params.isUpdate = true return postInstallEvents(out, cfg, an, params) } @@ -295,7 +293,7 @@ func execute(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, // installOrUpdateFromLocalSource is invoked when we're performing an installation where the payload is already provided func installOrUpdateFromLocalSource(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, payloadPath string, params *Params) error { logging.Debug("Install from local source") - an.Event(AnalyticsFunnelCat, "local-source") + an.Event(anaConst.CatInstallerFunnel, "local-source", anaConst.SrcStateTool) if !params.isUpdate { // install.sh or install.ps1 downloaded this installer and is running it. out.Print(output.Title("Installing State Tool Package Manager")) @@ -324,12 +322,12 @@ func installOrUpdateFromLocalSource(out output.Outputer, cfg *config.Instance, a } // Run installer - an.Event(AnalyticsFunnelCat, "pre-installer") + an.Event(anaConst.CatInstallerFunnel, "pre-installer", anaConst.SrcStateTool) if err := installer.Install(); err != nil { out.Print("[ERROR]x Failed[/RESET]") return err } - an.Event(AnalyticsFunnelCat, "post-installer") + an.Event(anaConst.CatInstallerFunnel, "post-installer", anaConst.SrcStateTool) out.Print("[SUCCESS]✔ Done[/RESET]") if !params.isUpdate { @@ -342,7 +340,7 @@ func installOrUpdateFromLocalSource(out output.Outputer, cfg *config.Instance, a } func postInstallEvents(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, params *Params) error { - an.Event(AnalyticsFunnelCat, "post-install-events") + an.Event(anaConst.CatInstallerFunnel, "post-install-events", anaConst.SrcStateTool) installPath, err := resolveInstallPath(params.path) if err != nil { @@ -368,30 +366,30 @@ func postInstallEvents(out output.Outputer, cfg *config.Instance, an analytics.D switch { // Execute provided --command case params.command != "": - an.Event(AnalyticsFunnelCat, "forward-command") + an.Event(anaConst.CatInstallerFunnel, "forward-command", anaConst.SrcStateTool) out.Print(fmt.Sprintf("\nRunning `[ACTIONABLE]%s[/RESET]`\n", params.command)) cmd, args := exeutils.DecodeCmd(params.command) if _, _, err := exeutils.ExecuteAndPipeStd(cmd, args, envSlice(binPath)); err != nil { - an.EventWithLabel(AnalyticsFunnelCat, "forward-command-err", err.Error()) + an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-command-err", anaConst.SrcStateTool, err.Error()) return errs.Silence(errs.Wrap(err, "Running provided command failed, error returned: %s", errs.JoinMessage(err))) } // Activate provided --activate Namespace case params.activate.IsValid(): - an.Event(AnalyticsFunnelCat, "forward-activate") + an.Event(anaConst.CatInstallerFunnel, "forward-activate", anaConst.SrcStateTool) out.Print(fmt.Sprintf("\nRunning `[ACTIONABLE]state activate %s[/RESET]`\n", params.activate.String())) if _, _, err := exeutils.ExecuteAndPipeStd(stateExe, []string{"activate", params.activate.String()}, envSlice(binPath)); err != nil { - an.EventWithLabel(AnalyticsFunnelCat, "forward-activate-err", err.Error()) + an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-activate-err", anaConst.SrcStateTool, err.Error()) return errs.Silence(errs.Wrap(err, "Could not activate %s, error returned: %s", params.activate.String(), errs.JoinMessage(err))) } // Activate provided --activate-default Namespace case params.activateDefault.IsValid(): - an.Event(AnalyticsFunnelCat, "forward-activate-default") + an.Event(anaConst.CatInstallerFunnel, "forward-activate-default", anaConst.SrcStateTool) out.Print(fmt.Sprintf("\nRunning `[ACTIONABLE]state activate --default %s[/RESET]`\n", params.activateDefault.String())) if _, _, err := exeutils.ExecuteAndPipeStd(stateExe, []string{"activate", params.activateDefault.String(), "--default"}, envSlice(binPath)); err != nil { - an.EventWithLabel(AnalyticsFunnelCat, "forward-activate-default-err", err.Error()) + an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-activate-default-err", anaConst.SrcStateTool, err.Error()) return errs.Silence(errs.Wrap(err, "Could not activate %s, error returned: %s", params.activateDefault.String(), errs.JoinMessage(err))) } case !params.isUpdate && terminal.IsTerminal(int(os.Stdin.Fd())) && os.Getenv(constants.InstallerNoSubshell) != "true" && os.Getenv("TERM") != "dumb": diff --git a/cmd/state-offline-installer/install.go b/cmd/state-offline-installer/install.go index 42d7d0da76..cd46faa035 100644 --- a/cmd/state-offline-installer/install.go +++ b/cmd/state-offline-installer/install.go @@ -92,9 +92,9 @@ func (r *runner) Run(params *Params) (rerr error) { return } if locale.IsInputError(rerr) { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) } else { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) } }() @@ -122,7 +122,7 @@ func (r *runner) Run(params *Params) (rerr error) { CommitID: &r.icfg.CommitID, Trigger: ptr.To(target.TriggerOfflineInstaller.String()), } - r.analytics.Event(ac.CatOfflineInstaller, "start", installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, "start", ac.SrcOfflineInstaller, installerDimensions) // Detect target path targetPath, err := r.getTargetPath(params.path) @@ -248,7 +248,7 @@ func (r *runner) Run(params *Params) (rerr error) { return errs.Wrap(err, "Could not configure environment") } - r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, ac.SrcOfflineInstaller, installerDimensions) r.out.Print(fmt.Sprintf(`Installation complete. Your language runtime has been installed in [ACTIONABLE]%s[/RESET].`, targetPath)) diff --git a/cmd/state-offline-uninstaller/uninstall.go b/cmd/state-offline-uninstaller/uninstall.go index 08a86065fc..209891f9ff 100644 --- a/cmd/state-offline-uninstaller/uninstall.go +++ b/cmd/state-offline-uninstaller/uninstall.go @@ -79,9 +79,9 @@ func (r *runner) Run(params *Params) (rerr error) { return } if locale.IsInputError(rerr) { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) } else { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) } }() @@ -116,7 +116,7 @@ func (r *runner) Run(params *Params) (rerr error) { CommitID: &r.icfg.CommitID, Trigger: ptr.To(target.TriggerOfflineUninstaller.String()), } - r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerStart, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerStart, ac.SrcOfflineInstaller, installerDimensions) r.out.Print("Removing environment configuration") err = r.removeEnvPaths(namespace) @@ -130,8 +130,8 @@ func (r *runner) Run(params *Params) (rerr error) { return errs.Wrap(err, "Error removing installation directory") } - r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, installerDimensions) - r.analytics.Event(ac.CatRuntimeUsage, ac.ActRuntimeDelete, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, ac.SrcOfflineInstaller, installerDimensions) + r.analytics.Event(ac.CatRuntimeUsage, ac.ActRuntimeDelete, ac.SrcOfflineInstaller, installerDimensions) r.out.Print("Uninstall Complete") diff --git a/cmd/state-svc/internal/resolver/resolver.go b/cmd/state-svc/internal/resolver/resolver.go index b644a7a008..f027de602f 100644 --- a/cmd/state-svc/internal/resolver/resolver.go +++ b/cmd/state-svc/internal/resolver/resolver.go @@ -102,7 +102,7 @@ func (r *Resolver) Query() genserver.QueryResolver { return r } func (r *Resolver) Version(ctx context.Context) (*graph.Version, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", "Version") + r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", anaConsts.SrcStateTool, "Version") logging.Debug("Version resolver") return &graph.Version{ State: &graph.StateVersion{ @@ -118,7 +118,7 @@ func (r *Resolver) Version(ctx context.Context) (*graph.Version, error) { func (r *Resolver) AvailableUpdate(ctx context.Context) (*graph.AvailableUpdate, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", "AvailableUpdate") + r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", anaConsts.SrcStateTool, "AvailableUpdate") logging.Debug("AvailableUpdate resolver") defer logging.Debug("AvailableUpdate done") @@ -142,7 +142,7 @@ func (r *Resolver) AvailableUpdate(ctx context.Context) (*graph.AvailableUpdate, func (r *Resolver) Projects(ctx context.Context) ([]*graph.Project, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", "Projects") + r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", anaConsts.SrcStateTool, "Projects") logging.Debug("Projects resolver") var projects []*graph.Project localConfigProjects := projectfile.GetProjectMapping(r.cfg) @@ -159,10 +159,10 @@ func (r *Resolver) Projects(ctx context.Context) ([]*graph.Project, error) { return projects, nil } -func (r *Resolver) AnalyticsEvent(_ context.Context, category, action string, _label *string, dimensionsJson string) (*graph.AnalyticsEventResponse, error) { +func (r *Resolver) AnalyticsEvent(_ context.Context, category, action, source string, _label *string, dimensionsJson string) (*graph.AnalyticsEventResponse, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - logging.Debug("Analytics event resolver: %s - %s", category, action) + logging.Debug("Analytics event resolver: %s - %s (%s)", category, action, source) label := "" if _label != nil { @@ -188,7 +188,7 @@ func (r *Resolver) AnalyticsEvent(_ context.Context, category, action string, _l return nil }) - r.anForClient.EventWithLabel(category, action, label, dims) + r.anForClient.EventWithLabel(category, action, source, label, dims) return &graph.AnalyticsEventResponse{Sent: true}, nil } diff --git a/cmd/state-svc/internal/rtwatcher/watcher.go b/cmd/state-svc/internal/rtwatcher/watcher.go index 60634c7040..fa22aaeaff 100644 --- a/cmd/state-svc/internal/rtwatcher/watcher.go +++ b/cmd/state-svc/internal/rtwatcher/watcher.go @@ -30,7 +30,7 @@ type Watcher struct { } type analytics interface { - Event(category string, action string, dim ...*dimensions.Values) + Event(category, action, source string, dim ...*dimensions.Values) } func New(cfg *config.Instance, an analytics) *Watcher { @@ -97,7 +97,7 @@ func (w *Watcher) check() { func (w *Watcher) RecordUsage(e entry) { logging.Debug("Recording usage of %s (%d)", e.Exec, e.PID) - w.an.Event(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, e.Dims) + w.an.Event(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcStateTool, e.Dims) } func (w *Watcher) Close() error { diff --git a/cmd/state-svc/internal/server/generated/generated.go b/cmd/state-svc/internal/server/generated/generated.go index c08a7f2084..104c805ca9 100644 --- a/cmd/state-svc/internal/server/generated/generated.go +++ b/cmd/state-svc/internal/server/generated/generated.go @@ -78,7 +78,7 @@ type ComplexityRoot struct { } Query struct { - AnalyticsEvent func(childComplexity int, category string, action string, label *string, dimensionsJSON string) int + AnalyticsEvent func(childComplexity int, category string, action string, source string, label *string, dimensionsJSON string) int AvailableUpdate func(childComplexity int) int CheckMessages func(childComplexity int, command string, flags []string) int CheckRuntimeUsage func(childComplexity int, organizationName string) int @@ -110,7 +110,7 @@ type QueryResolver interface { Version(ctx context.Context) (*graph.Version, error) AvailableUpdate(ctx context.Context) (*graph.AvailableUpdate, error) Projects(ctx context.Context) ([]*graph.Project, error) - AnalyticsEvent(ctx context.Context, category string, action string, label *string, dimensionsJSON string) (*graph.AnalyticsEventResponse, error) + AnalyticsEvent(ctx context.Context, category string, action string, source string, label *string, dimensionsJSON string) (*graph.AnalyticsEventResponse, error) ReportRuntimeUsage(ctx context.Context, pid int, exec string, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) CheckRuntimeUsage(ctx context.Context, organizationName string) (*graph.CheckRuntimeUsageResponse, error) CheckMessages(ctx context.Context, command string, flags []string) ([]*graph.MessageInfo, error) @@ -262,7 +262,7 @@ func (e *executableSchema) Complexity(typeName, field string, childComplexity in return 0, false } - return e.complexity.Query.AnalyticsEvent(childComplexity, args["category"].(string), args["action"].(string), args["label"].(*string), args["dimensionsJson"].(string)), true + return e.complexity.Query.AnalyticsEvent(childComplexity, args["category"].(string), args["action"].(string), args["source"].(string), args["label"].(*string), args["dimensionsJson"].(string)), true case "Query.availableUpdate": if e.complexity.Query.AvailableUpdate == nil { @@ -512,7 +512,7 @@ type Query { version: Version availableUpdate: AvailableUpdate projects: [Project]! - analyticsEvent(category: String!, action: String!, label: String, dimensionsJson: String!): AnalyticsEventResponse + analyticsEvent(category: String!, action: String!, source: String!, label: String, dimensionsJson: String!): AnalyticsEventResponse reportRuntimeUsage(pid: Int!, exec: String!, dimensionsJson: String!): ReportRuntimeUsageResponse checkRuntimeUsage(organizationName: String!): CheckRuntimeUsageResponse checkMessages(command: String!, flags: [String!]!): [MessageInfo!]! @@ -567,24 +567,33 @@ func (ec *executionContext) field_Query_analyticsEvent_args(ctx context.Context, } } args["action"] = arg1 - var arg2 *string + var arg2 string + if tmp, ok := rawArgs["source"]; ok { + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("source")) + arg2, err = ec.unmarshalNString2string(ctx, tmp) + if err != nil { + return nil, err + } + } + args["source"] = arg2 + var arg3 *string if tmp, ok := rawArgs["label"]; ok { ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("label")) - arg2, err = ec.unmarshalOString2áš–string(ctx, tmp) + arg3, err = ec.unmarshalOString2áš–string(ctx, tmp) if err != nil { return nil, err } } - args["label"] = arg2 - var arg3 string + args["label"] = arg3 + var arg4 string if tmp, ok := rawArgs["dimensionsJson"]; ok { ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("dimensionsJson")) - arg3, err = ec.unmarshalNString2string(ctx, tmp) + arg4, err = ec.unmarshalNString2string(ctx, tmp) if err != nil { return nil, err } } - args["dimensionsJson"] = arg3 + args["dimensionsJson"] = arg4 return args, nil } @@ -1620,7 +1629,7 @@ func (ec *executionContext) _Query_analyticsEvent(ctx context.Context, field gra }() resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (interface{}, error) { ctx = rctx // use context from middleware stack in children - return ec.resolvers.Query().AnalyticsEvent(rctx, fc.Args["category"].(string), fc.Args["action"].(string), fc.Args["label"].(*string), fc.Args["dimensionsJson"].(string)) + return ec.resolvers.Query().AnalyticsEvent(rctx, fc.Args["category"].(string), fc.Args["action"].(string), fc.Args["source"].(string), fc.Args["label"].(*string), fc.Args["dimensionsJson"].(string)) }) if err != nil { ec.Error(ctx, err) diff --git a/cmd/state-svc/internal/server/server.go b/cmd/state-svc/internal/server/server.go index 9750b77711..f1048e1730 100644 --- a/cmd/state-svc/internal/server/server.go +++ b/cmd/state-svc/internal/server/server.go @@ -75,16 +75,16 @@ func (s *Server) Resolver() *resolver.Resolver { } func (s *Server) Start() error { - s.analytics.Event(constants.CatStateSvc, "start") + s.analytics.Event(constants.CatStateSvc, "start", constants.SrcStateTool) err := s.httpServer.Start(s.listener.Addr().String()) if err != nil { - s.analytics.Event(constants.CatStateSvc, "start-failure") + s.analytics.Event(constants.CatStateSvc, "start-failure", constants.SrcStateTool) } return err } func (s *Server) Shutdown() error { - s.analytics.Event(constants.CatStateSvc, "shutdown") + s.analytics.Event(constants.CatStateSvc, "shutdown", constants.SrcStateTool) logging.Debug("shutting down server") ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() diff --git a/cmd/state-svc/schema/schema.graphqls b/cmd/state-svc/schema/schema.graphqls index 0ab058c449..6af4224bbd 100644 --- a/cmd/state-svc/schema/schema.graphqls +++ b/cmd/state-svc/schema/schema.graphqls @@ -69,7 +69,7 @@ type Query { version: Version availableUpdate: AvailableUpdate projects: [Project]! - analyticsEvent(category: String!, action: String!, label: String, dimensionsJson: String!): AnalyticsEventResponse + analyticsEvent(category: String!, action: String!, source: String!, label: String, dimensionsJson: String!): AnalyticsEventResponse reportRuntimeUsage(pid: Int!, exec: String!, dimensionsJson: String!): ReportRuntimeUsageResponse checkRuntimeUsage(organizationName: String!): CheckRuntimeUsageResponse checkMessages(command: String!, flags: [String!]!): [MessageInfo!]! diff --git a/cmd/state/autoupdate.go b/cmd/state/autoupdate.go index 54ffc6c74e..0992bff9be 100644 --- a/cmd/state/autoupdate.go +++ b/cmd/state/autoupdate.go @@ -63,7 +63,7 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou if !isEnabled(cfg) { logging.Debug("Not performing autoupdates because user turned off autoupdates.") - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, anaConst.UpdateLabelDisabledConfig) + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, anaConst.SrcStateTool, anaConst.UpdateLabelDisabledConfig) out.Notice(output.Title(locale.Tl("update_available_header", "Auto Update"))) out.Notice(locale.Tr("update_available", constants.Version, up.Version)) return false, nil @@ -77,20 +77,20 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou err = up.InstallBlocking("") if err != nil { if os.IsPermission(err) { - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To("Could not update the state tool due to insufficient permissions."), }) return false, locale.WrapInputError(err, locale.Tl("auto_update_permission_err", "", constants.DocumentationURL, errs.JoinMessage(err))) } if errs.Matches(err, &updater.ErrorInProgress{}) { - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To(anaConst.UpdateErrorInProgress), }) return false, nil } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To(anaConst.UpdateErrorInstallFailed), }) @@ -108,14 +108,14 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou } else if errs.Matches(err, &ErrExecuteRelaunch{}) { msg = anaConst.UpdateErrorRelaunch } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To(msg), }) return true, errs.Silence(errs.WrapExitCode(err, code)) } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.UpdateLabelSuccess, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.SrcStateTool, anaConst.UpdateLabelSuccess, &dimensions.Values{ TargetVersion: ptr.To(up.Version), }) return true, nil @@ -187,7 +187,7 @@ func shouldRunAutoUpdate(args []string, cfg *config.Instance, an analytics.Dispa label = anaConst.UpdateLabelLocked } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, label) + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, anaConst.SrcStateTool, label) return shouldUpdate } diff --git a/cmd/state/internal/cmdtree/activate.go b/cmd/state/internal/cmdtree/activate.go index d675e300b4..0202124e30 100644 --- a/cmd/state/internal/cmdtree/activate.go +++ b/cmd/state/internal/cmdtree/activate.go @@ -66,19 +66,19 @@ func newActivateCommand(prime *primer.Values) *captain.Command { an := prime.Analytics() var serr interface{ Signal() os.Signal } if errors.As(err, &serr) { - an.Event(constants.CatActivationFlow, "user-interrupt-error") + an.Event(constants.CatActivationFlow, "user-interrupt-error", constants.SrcStateTool) } if locale.IsInputError(err) { // Failed due to user input - an.Event(constants.CatActivationFlow, "user-input-error") + an.Event(constants.CatActivationFlow, "user-input-error", constants.SrcStateTool) } else { var exitErr = &exec.ExitError{} if !errors.As(err, &exitErr) { // Failed due to an error we might need to address - an.Event(constants.CatActivationFlow, "error") + an.Event(constants.CatActivationFlow, "error", constants.SrcStateTool) } else { // Failed due to user subshell actions / events - an.Event(constants.CatActivationFlow, "user-exit-error") + an.Event(constants.CatActivationFlow, "user-exit-error", constants.SrcStateTool) } } } diff --git a/cmd/state/internal/cmdtree/tutorial.go b/cmd/state/internal/cmdtree/tutorial.go index af69f9a785..8c669867d9 100644 --- a/cmd/state/internal/cmdtree/tutorial.go +++ b/cmd/state/internal/cmdtree/tutorial.go @@ -53,9 +53,9 @@ func newTutorialProjectCommand(prime *primer.Values) *captain.Command { func(ccmd *captain.Command, args []string) error { err := runner.RunNewProject(params) if err != nil { - prime.Analytics().EventWithLabel(constants.CatTutorial, "error", errs.JoinMessage(err)) + prime.Analytics().EventWithLabel(constants.CatTutorial, "error", constants.SrcStateTool, errs.JoinMessage(err)) } else { - prime.Analytics().Event(constants.CatTutorial, "completed") + prime.Analytics().Event(constants.CatTutorial, "completed", constants.SrcStateTool) } return err }, diff --git a/go.sum b/go.sum index dcf45175a8..14d761555c 100644 --- a/go.sum +++ b/go.sum @@ -341,10 +341,6 @@ github.com/99designs/gqlgen v0.17.19 h1:lO3PBSOv5Mw8RPt0Nwg7ovJ9tNfjGDEnU48AYqLz github.com/99designs/gqlgen v0.17.19/go.mod h1:tXjo2/o1L/9A8S/4nLK7Arx/677H8hxlD/iSTRx0BtE= github.com/ActiveState/go-ogle-analytics v0.0.0-20170510030904-9b3f14901527 h1:lW+qgVXf/iAnSs8SgagWxh8d6nsbpmwyhmeg9/fp0Os= github.com/ActiveState/go-ogle-analytics v0.0.0-20170510030904-9b3f14901527/go.mod h1:/9SyzKLlJSuIa7WAsLUUhHqTK9+PtZD8cKF8G4SLpa0= -github.com/ActiveState/graphql v0.0.0-20180524141115-05b17f315749 h1:6Uj/oFUEJ7UcQ721u2Smti9IIy9lPHEho50buO4ZYXE= -github.com/ActiveState/graphql v0.0.0-20180524141115-05b17f315749/go.mod h1:NhUbNQ8UpfnC6nZvZ8oThqYSCE/G8FQp9JUrK9jXJs0= -github.com/ActiveState/graphql v0.0.0-20230718220857-da7d147af6b4 h1:f2n4heMWzHlHUqQZuV2hTPO5bU95WEQrT76qyfS6qtE= -github.com/ActiveState/graphql v0.0.0-20230718220857-da7d147af6b4/go.mod h1:NhUbNQ8UpfnC6nZvZ8oThqYSCE/G8FQp9JUrK9jXJs0= github.com/ActiveState/graphql v0.0.0-20230719154233-6949037a6e48 h1:UCx/ObpVRgC4fp2OlJM2iNdMMu+K87/aPxKrQ1WRLj4= github.com/ActiveState/graphql v0.0.0-20230719154233-6949037a6e48/go.mod h1:NhUbNQ8UpfnC6nZvZ8oThqYSCE/G8FQp9JUrK9jXJs0= github.com/ActiveState/termtest v0.7.2 h1:vNPMpI2AyZnLZzZn/CRpMZzIuVL0XhaTLA4qyghIGF8= diff --git a/internal/analytics/analytics.go b/internal/analytics/analytics.go index 4edc267590..6f2aaa3aca 100644 --- a/internal/analytics/analytics.go +++ b/internal/analytics/analytics.go @@ -8,8 +8,8 @@ import ( // Dispatcher describes a struct that can send analytics event in the background type Dispatcher interface { - Event(category string, action string, dim ...*dimensions.Values) - EventWithLabel(category string, action string, label string, dim ...*dimensions.Values) + Event(category, action, source string, dim ...*dimensions.Values) + EventWithLabel(category, action, source, label string, dim ...*dimensions.Values) Wait() Close() } diff --git a/internal/analytics/client/async/client.go b/internal/analytics/client/async/client.go index 9793a435c1..c590453c11 100644 --- a/internal/analytics/client/async/client.go +++ b/internal/analytics/client/async/client.go @@ -75,13 +75,13 @@ func New(svcModel *model.SvcModel, cfg *config.Instance, auth *authentication.Au } // Event logs an event to google analytics -func (a *Client) Event(category string, action string, dims ...*dimensions.Values) { - a.EventWithLabel(category, action, "", dims...) +func (a *Client) Event(category, action, source string, dims ...*dimensions.Values) { + a.EventWithLabel(category, action, source, "", dims...) } // EventWithLabel logs an event with a label to google analytics -func (a *Client) EventWithLabel(category string, action string, label string, dims ...*dimensions.Values) { - err := a.sendEvent(category, action, label, dims...) +func (a *Client) EventWithLabel(category, action, source, label string, dims ...*dimensions.Values) { + err := a.sendEvent(category, action, source, label, dims...) if err != nil { multilog.Error("Error during analytics.sendEvent: %v", errs.JoinMessage(err)) } @@ -98,7 +98,7 @@ func (a *Client) Wait() { a.eventWaitGroup.Wait() } -func (a *Client) sendEvent(category, action, label string, dims ...*dimensions.Values) error { +func (a *Client) sendEvent(category, action, source, label string, dims ...*dimensions.Values) error { if a.svcModel == nil { // this is only true on CI return nil } @@ -133,7 +133,7 @@ func (a *Client) sendEvent(category, action, label string, dims ...*dimensions.V defer func() { handlePanics(recover(), debug.Stack()) }() defer a.eventWaitGroup.Done() - if err := a.svcModel.AnalyticsEvent(context.Background(), category, action, label, string(dimMarshalled)); err != nil { + if err := a.svcModel.AnalyticsEvent(context.Background(), category, action, source, label, string(dimMarshalled)); err != nil { logging.Debug("Failed to report analytics event via state-svc: %s", errs.JoinMessage(err)) } }() diff --git a/internal/analytics/client/blackhole/client.go b/internal/analytics/client/blackhole/client.go index c36cf79d1b..a7ac5d4431 100644 --- a/internal/analytics/client/blackhole/client.go +++ b/internal/analytics/client/blackhole/client.go @@ -13,10 +13,10 @@ func New() *Client { return &Client{} } -func (c Client) Event(category string, action string, dim ...*dimensions.Values) { +func (c Client) Event(category, action, source string, dim ...*dimensions.Values) { } -func (c Client) EventWithLabel(category string, action string, label string, dim ...*dimensions.Values) { +func (c Client) EventWithLabel(category, action, source, label string, dim ...*dimensions.Values) { } func (c Client) Wait() { diff --git a/internal/analytics/client/sync/client.go b/internal/analytics/client/sync/client.go index 36d725fb14..6ffee1f480 100644 --- a/internal/analytics/client/sync/client.go +++ b/internal/analytics/client/sync/client.go @@ -32,7 +32,7 @@ import ( type Reporter interface { ID() string - Event(category, action, label string, dimensions *dimensions.Values) error + Event(category, action, source, label string, dimensions *dimensions.Values) error } // Client instances send analytics events to GA and S3 endpoints without delay. It is only supposed to be used inside the `state-svc`. All other processes should use the DefaultClient. @@ -150,13 +150,13 @@ func (a *Client) Wait() { } // Events returns a channel to feed eventData directly to the report loop -func (a *Client) report(category, action, label string, dimensions *dimensions.Values) { +func (a *Client) report(category, action, source, label string, dimensions *dimensions.Values) { if !a.sendReports { return } for _, reporter := range a.reporters { - if err := reporter.Event(category, action, label, dimensions); err != nil { + if err := reporter.Event(category, action, source, label, dimensions); err != nil { logging.Debug( "Reporter failed: %s, category: %s, action: %s, error: %s", reporter.ID(), category, action, errs.JoinMessage(err), @@ -165,8 +165,8 @@ func (a *Client) report(category, action, label string, dimensions *dimensions.V } } -func (a *Client) Event(category string, action string, dims ...*dimensions.Values) { - a.EventWithLabel(category, action, "", dims...) +func (a *Client) Event(category, action, source string, dims ...*dimensions.Values) { + a.EventWithLabel(category, action, source, "", dims...) } func mergeDimensions(target *dimensions.Values, dims ...*dimensions.Values) *dimensions.Values { @@ -180,7 +180,7 @@ func mergeDimensions(target *dimensions.Values, dims ...*dimensions.Values) *dim return actualDims } -func (a *Client) EventWithLabel(category string, action, label string, dims ...*dimensions.Values) { +func (a *Client) EventWithLabel(category, action, source, label string, dims ...*dimensions.Values) { if a.customDimensions == nil { if condition.InUnitTest() { return @@ -210,7 +210,7 @@ func (a *Client) EventWithLabel(category string, action, label string, dims ...* go func() { defer a.eventWaitGroup.Done() defer func() { handlePanics(recover(), debug.Stack()) }() - a.report(category, action, label, actualDims) + a.report(category, action, source, label, actualDims) }() } diff --git a/internal/analytics/client/sync/reporters/ga-state.go b/internal/analytics/client/sync/reporters/ga-state.go index 623b7470d8..afdec89b64 100644 --- a/internal/analytics/client/sync/reporters/ga-state.go +++ b/internal/analytics/client/sync/reporters/ga-state.go @@ -44,7 +44,7 @@ func (r *GaCLIReporter) AddOmitCategory(category string) { r.omit[category] = struct{}{} } -func (r *GaCLIReporter) Event(category, action, label string, d *dimensions.Values) error { +func (r *GaCLIReporter) Event(category, action, source, label string, d *dimensions.Values) error { if _, ok := r.omit[category]; ok { logging.Debug("Not sending event with category: %s to Google Analytics", category) return nil diff --git a/internal/analytics/client/sync/reporters/pixel.go b/internal/analytics/client/sync/reporters/pixel.go index 8a9fde09d7..d42588862b 100644 --- a/internal/analytics/client/sync/reporters/pixel.go +++ b/internal/analytics/client/sync/reporters/pixel.go @@ -29,7 +29,7 @@ func (r *PixelReporter) ID() string { return "PixelReporter" } -func (r *PixelReporter) Event(category, action, label string, d *dimensions.Values) error { +func (r *PixelReporter) Event(category, action, source, label string, d *dimensions.Values) error { pixelURL, err := url.Parse(r.url) if err != nil { return errs.Wrap(err, "Invalid pixel URL: %s", r.url) @@ -38,6 +38,7 @@ func (r *PixelReporter) Event(category, action, label string, d *dimensions.Valu query := &url.Values{} query.Add("x-category", category) query.Add("x-action", action) + query.Add("x-source", source) query.Add("x-label", label) for num, value := range legacyDimensionMap(d) { diff --git a/internal/analytics/client/sync/reporters/test.go b/internal/analytics/client/sync/reporters/test.go index b8c60dfe08..95ff0c426a 100644 --- a/internal/analytics/client/sync/reporters/test.go +++ b/internal/analytics/client/sync/reporters/test.go @@ -36,12 +36,13 @@ func (r *TestReporter) ID() string { type TestLogEntry struct { Category string Action string + Source string Label string Dimensions *dimensions.Values } -func (r *TestReporter) Event(category, action, label string, d *dimensions.Values) error { - b, err := json.Marshal(TestLogEntry{category, action, label, d}) +func (r *TestReporter) Event(category, action, source, label string, d *dimensions.Values) error { + b, err := json.Marshal(TestLogEntry{category, action, source, label, d}) if err != nil { return errs.Wrap(err, "Could not marshal test log entry") } diff --git a/internal/analytics/constants/constants.go b/internal/analytics/constants/constants.go index b2bf3dfa4f..60202bf803 100644 --- a/internal/analytics/constants/constants.go +++ b/internal/analytics/constants/constants.go @@ -25,6 +25,21 @@ const CatConfig = "config" // CatUpdate is the event category used for all update events const CatUpdates = "updates" +// CatInstaller is the event category used for installer events. +const CatInstaller = "installer" + +// CatInstallerFunnel is the event category used for installer funnel events. +const CatInstallerFunnel = "installer-funnel" + +// SrcStateTool is the event source for State Tool. +const SrcStateTool = "State Tool" + +// SrcOfflineInstaller is the event source for offline installers. +const SrcOfflineInstaller = "Offline Installer" + +// SrcExecutor is the event source for executors. +const SrcExecutor = "Executor" + // ActRuntimeHeartbeat is the event action sent when a runtime is in use const ActRuntimeHeartbeat = "heartbeat" diff --git a/internal/captain/command.go b/internal/captain/command.go index 37e1cd4b21..9708b9da3e 100644 --- a/internal/captain/command.go +++ b/internal/captain/command.go @@ -634,10 +634,10 @@ func (c *Command) cobraExecHandler(cobraCmd *cobra.Command, args []string) error label = append(label, name) }) - c.analytics.EventWithLabel(anaConsts.CatRunCmd, appEventPrefix+subCommandString, strings.Join(label, " ")) + c.analytics.EventWithLabel(anaConsts.CatRunCmd, appEventPrefix+subCommandString, anaConsts.SrcStateTool, strings.Join(label, " ")) if shim, got := os.LookupEnv(constants.ShimEnvVarName); got { - c.analytics.Event(anaConsts.CatShim, shim) + c.analytics.Event(anaConsts.CatShim, shim, anaConsts.SrcStateTool) } } @@ -708,16 +708,16 @@ func (c *Command) cobraExecHandler(cobraCmd *cobra.Command, args []string) error var serr interface{ Signal() os.Signal } if errors.As(err, &serr) { if c.analytics != nil { - c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, "interrupt") + c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, anaConsts.SrcStateTool, "interrupt") } err = locale.WrapInputError(err, "user_interrupt", "User interrupted the State Tool process.") } else { if c.analytics != nil { if err != nil && (subCommandString == "install" || subCommandString == "activate") { // This is a temporary hack; proper implementation: https://activestatef.atlassian.net/browse/DX-495 - c.analytics.EventWithLabel(anaConsts.CatCommandError, appEventPrefix+subCommandString, errs.JoinMessage(err)) + c.analytics.EventWithLabel(anaConsts.CatCommandError, appEventPrefix+subCommandString, anaConsts.SrcStateTool, errs.JoinMessage(err)) } - c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, strconv.Itoa(exitCode)) + c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, anaConsts.SrcStateTool, strconv.Itoa(exitCode)) } } diff --git a/internal/graph/generated.go b/internal/graph/generated.go index b7102e0eff..62610a04aa 100644 --- a/internal/graph/generated.go +++ b/internal/graph/generated.go @@ -38,15 +38,6 @@ type MessageInfo struct { Placement MessagePlacementType `json:"placement"` } -/* -[ - { - "ID": "simple", - "Message": "This is a [NOTICE]simple[/RESET] message\nwith a line break", - } -] -*/ - type Project struct { Namespace string `json:"namespace"` Locations []string `json:"locations"` diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index ad5044acc8..9e3759dc79 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -15,7 +15,7 @@ import ( ) type EventDispatcher interface { - EventWithLabel(category string, action string, label string, dim ...*dimensions.Values) + EventWithLabel(category, action, source string, label string, dim ...*dimensions.Values) } // Prompter is the interface used to run our prompt from, useful for mocking in tests @@ -169,7 +169,7 @@ func (p *Prompt) Confirm(title, message string, defaultChoice *bool) (bool, erro p.out.Notice(output.Emphasize(title)) } - p.analytics.EventWithLabel(constants.CatPrompt, title, "present") + p.analytics.EventWithLabel(constants.CatPrompt, title, constants.SrcStateTool, "present") var defChoice bool if defaultChoice != nil { @@ -183,11 +183,11 @@ func (p *Prompt) Confirm(title, message string, defaultChoice *bool) (bool, erro }}, &resp, nil) if err != nil { if err == terminal.InterruptErr { - p.analytics.EventWithLabel(constants.CatPrompt, title, "interrupt") + p.analytics.EventWithLabel(constants.CatPrompt, title, constants.SrcStateTool, "interrupt") } return false, locale.NewInputError(err.Error()) } - p.analytics.EventWithLabel(constants.CatPrompt, title, translateConfirm(resp)) + p.analytics.EventWithLabel(constants.CatPrompt, title, constants.SrcStateTool, translateConfirm(resp)) return resp, nil } diff --git a/internal/runbits/activation/activation.go b/internal/runbits/activation/activation.go index d6d23740d8..da23f7b4d7 100644 --- a/internal/runbits/activation/activation.go +++ b/internal/runbits/activation/activation.go @@ -80,7 +80,7 @@ func ActivateAndWait( } defer fe.Close() - an.Event(anaConst.CatActivationFlow, "before-subshell") + an.Event(anaConst.CatActivationFlow, "before-subshell", anaConst.SrcStateTool) err = <-ss.Errors() if err != nil { diff --git a/internal/runbits/requirements/requirements.go b/internal/runbits/requirements/requirements.go index c3d7d81ac7..2a0f4b82f3 100644 --- a/internal/runbits/requirements/requirements.go +++ b/internal/runbits/requirements/requirements.go @@ -204,7 +204,7 @@ func (r *RequirementOperation) ExecuteRequirementOperation(requirementName, requ } r.Analytics.EventWithLabel( - anaConsts.CatPackageOp, fmt.Sprintf("%s-%s", operation, langName), requirementName, + anaConsts.CatPackageOp, fmt.Sprintf("%s-%s", operation, langName), anaConsts.SrcStateTool, requirementName, ) if !hasParentCommit { diff --git a/internal/runners/activate/activate.go b/internal/runners/activate/activate.go index 14fd6ba218..7366816080 100644 --- a/internal/runners/activate/activate.go +++ b/internal/runners/activate/activate.go @@ -151,7 +151,7 @@ func (r *Activate) Run(params *ActivateParams) error { } // Have to call this once the project has been set - r.analytics.Event(anaConsts.CatActivationFlow, "start") + r.analytics.Event(anaConsts.CatActivationFlow, "start", anaConsts.SrcStateTool) proj.Source().Persist() diff --git a/internal/runners/config/set.go b/internal/runners/config/set.go index dca1705333..8b78072531 100644 --- a/internal/runners/config/set.go +++ b/internal/runners/config/set.go @@ -106,5 +106,5 @@ func (s *Set) sendEvent(key string, value string, option configMediator.Option) } } - s.analytics.EventWithLabel(constants.CatConfig, action, key) + s.analytics.EventWithLabel(constants.CatConfig, action, constants.SrcStateTool, key) } diff --git a/internal/runners/deploy/uninstall/uninstall.go b/internal/runners/deploy/uninstall/uninstall.go index 2ffde0e656..56f18b693b 100644 --- a/internal/runners/deploy/uninstall/uninstall.go +++ b/internal/runners/deploy/uninstall/uninstall.go @@ -99,7 +99,7 @@ func (u *Uninstall) Run(params *Params) error { return locale.WrapError(err, "err_deploy_uninstall", "Unable to remove deployed runtime at '{{.V0}}'", path) } - u.analytics.Event(constants.CatRuntimeUsage, constants.ActRuntimeDelete, &dimensions.Values{ + u.analytics.Event(constants.CatRuntimeUsage, constants.ActRuntimeDelete, constants.SrcStateTool, &dimensions.Values{ Trigger: ptr.To(target.TriggerDeploy.String()), CommitID: ptr.To(commitID), ProjectNameSpace: ptr.To(namespace), diff --git a/internal/runners/ppm/convert.go b/internal/runners/ppm/convert.go index 561130d135..1220dda201 100644 --- a/internal/runners/ppm/convert.go +++ b/internal/runners/ppm/convert.go @@ -53,24 +53,24 @@ func (cf *ConversionFlow) StartIfNecessary() (bool, error) { return false, nil } - cf.analytics.Event(anaConsts.CatPpmConversion, "run") + cf.analytics.Event(anaConsts.CatPpmConversion, "run", anaConsts.SrcStateTool) r, err := cf.runSurvey() if err != nil { - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", errs.JoinMessage(err)) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", anaConsts.SrcStateTool, errs.JoinMessage(err)) return true, locale.WrapError(err, "ppm_conversion_survey_error", "Conversion flow failed.") } if r != accepted { - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", r.String()) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", anaConsts.SrcStateTool, r.String()) return true, locale.NewInputError("ppm_conversion_rejected", "Virtual environment creation cancelled.") } err = cf.createVirtualEnv() if err != nil { - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", errs.JoinMessage(err)) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", anaConsts.SrcStateTool, errs.JoinMessage(err)) return true, locale.WrapError(err, "ppm_conversion_venv_error", "Failed to create a project.") } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", r.String()) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", anaConsts.SrcStateTool, r.String()) return true, nil } @@ -103,7 +103,7 @@ func (cf *ConversionFlow) runSurvey() (conversionResult, error) { choices[0]: "create-virtual-env-1", choices[1]: "asked-why", } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", eventChoices[choice]) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", anaConsts.SrcStateTool, eventChoices[choice]) if choice == choices[0] { return accepted, nil @@ -141,7 +141,7 @@ func (cf *ConversionFlow) explainVirtualEnv() (conversionResult, error) { convertAnswerCreate: "create-virtual-env-2", no: "still-wants-ppm", } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", eventChoices[choice]) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", anaConsts.SrcStateTool, eventChoices[choice]) switch choice { case convertAnswerCreate: @@ -179,7 +179,7 @@ func (cf *ConversionFlow) explainAskFeedback() (conversionResult, error) { ok: "create-virtual-env-3", exit: "exit", } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", eventChoices[choice]) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", anaConsts.SrcStateTool, eventChoices[choice]) if choice == ok { return accepted, nil diff --git a/internal/runners/ppm/shim.go b/internal/runners/ppm/shim.go index 6d375cbcce..8164220868 100644 --- a/internal/runners/ppm/shim.go +++ b/internal/runners/ppm/shim.go @@ -81,9 +81,9 @@ func (s *Shim) RunList(converted bool, args ...string) error { func (s *Shim) shim(intercepted, replaced string, args ...string) error { err := s.executeShim(intercepted, replaced, args...) if err != nil { - s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, fmt.Sprintf("error: %v", errs.JoinMessage(err))) + s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, constants.SrcStateTool, fmt.Sprintf("error: %v", errs.JoinMessage(err))) } else { - s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, "success") + s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, constants.SrcStateTool, "success") } return err } @@ -91,7 +91,7 @@ func (s *Shim) shim(intercepted, replaced string, args ...string) error { func (s *Shim) executeShim(intercepted, replaced string, args ...string) error { if s.project == nil { // TODO: Replace this function call when conversion flow is complete - s.analytics.Event(constants.CatPPMShimCmd, "tutorial") + s.analytics.Event(constants.CatPPMShimCmd, "tutorial", constants.SrcStateTool) return tutorial() } diff --git a/internal/runners/tutorial/tutorial.go b/internal/runners/tutorial/tutorial.go index 2bc961ed35..867d273f79 100644 --- a/internal/runners/tutorial/tutorial.go +++ b/internal/runners/tutorial/tutorial.go @@ -12,8 +12,8 @@ import ( "github.com/ActiveState/cli/internal/fileutils" "github.com/ActiveState/cli/internal/language" "github.com/ActiveState/cli/internal/locale" - "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/osutils/user" + "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/primer" "github.com/ActiveState/cli/internal/prompt" "github.com/ActiveState/cli/internal/runbits" @@ -45,7 +45,7 @@ type NewProjectParams struct { } func (t *Tutorial) RunNewProject(params NewProjectParams) error { - t.analytics.EventWithLabel(anaConsts.CatTutorial, "run", fmt.Sprintf("skipIntro=%v,language=%v", params.SkipIntro, params.Language.String())) + t.analytics.EventWithLabel(anaConsts.CatTutorial, "run", anaConsts.SrcStateTool, fmt.Sprintf("skipIntro=%v,language=%v", params.SkipIntro, params.Language.String())) // Print intro if !params.SkipIntro { @@ -75,7 +75,7 @@ func (t *Tutorial) RunNewProject(params NewProjectParams) error { if lang == language.Unknown || lang == language.Unset { return locale.NewError("err_tutorial_language_unknown", "Invalid language selected: {{.V0}}.", choice) } - t.analytics.EventWithLabel(anaConsts.CatTutorial, "choose-language", lang.String()) + t.analytics.EventWithLabel(anaConsts.CatTutorial, "choose-language", anaConsts.SrcStateTool, lang.String()) } // Prompt for project name @@ -125,7 +125,7 @@ func (t *Tutorial) RunNewProject(params NewProjectParams) error { // authFlow is invoked when the user is not authenticated, it will prompt for sign in or sign up func (t *Tutorial) authFlow() error { - t.analytics.Event(anaConsts.CatTutorial, "authentication-flow") + t.analytics.Event(anaConsts.CatTutorial, "authentication-flow", anaConsts.SrcStateTool) // Sign in / Sign up choices signIn := locale.Tl("tutorial_signin", "Sign In") @@ -147,17 +147,17 @@ func (t *Tutorial) authFlow() error { // Evaluate user selection switch choice { case signIn: - t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", "sign-in") + t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", anaConsts.SrcStateTool, "sign-in") if err := runbits.Invoke(t.outputer, "auth"); err != nil { return locale.WrapInputError(err, "err_tutorial_signin", "Sign in failed. You could try manually signing in by running `state auth`.") } case signUpCLI: - t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", "sign-up") + t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", anaConsts.SrcStateTool, "sign-up") if err := runbits.Invoke(t.outputer, "auth", "signup"); err != nil { return locale.WrapInputError(err, "err_tutorial_signup", "Sign up failed. You could try manually signing up by running `state auth signup`.") } case signUpBrowser: - t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", "sign-up-browser") + t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", anaConsts.SrcStateTool, "sign-up-browser") err := open.Run(constants.PlatformSignupURL) if err != nil { return locale.WrapInputError(err, "err_tutorial_browser", "Could not open browser, please manually navigate to {{.V0}}.", constants.PlatformSignupURL) @@ -172,7 +172,7 @@ func (t *Tutorial) authFlow() error { return locale.WrapError(err, "err_tutorial_auth", "Could not authenticate after invoking `state auth ..`.") } - t.analytics.Event(anaConsts.CatTutorial, "authentication-flow-complete") + t.analytics.Event(anaConsts.CatTutorial, "authentication-flow-complete", anaConsts.SrcStateTool) return nil } diff --git a/internal/svcctl/comm.go b/internal/svcctl/comm.go index eda636e675..0582c2dfae 100644 --- a/internal/svcctl/comm.go +++ b/internal/svcctl/comm.go @@ -78,7 +78,7 @@ type Resolver interface { } type AnalyticsReporter interface { - Event(category string, action string, dims ...*dimensions.Values) + Event(category, action, source string, dims ...*dimensions.Values) } func HeartbeatHandler(cfg *config.Instance, resolver Resolver, analyticsReporter AnalyticsReporter) ipc.RequestHandler { @@ -138,7 +138,7 @@ func HeartbeatHandler(cfg *config.Instance, resolver Resolver, analyticsReporter } logging.Debug("Firing runtime usage events for %s", metaData.Namespace) - analyticsReporter.Event(constants.CatRuntimeUsage, constants.ActRuntimeAttempt, dims) + analyticsReporter.Event(constants.CatRuntimeUsage, constants.ActRuntimeAttempt, constants.SrcExecutor, dims) _, err = resolver.ReportRuntimeUsage(context.Background(), pidNum, hb.ExecPath, dimsJSON) if err != nil { multilog.Critical("Heartbeat Failure: Failed to report runtime usage in heartbeat handler: %s", errs.JoinMessage(err)) diff --git a/internal/updater/checker.go b/internal/updater/checker.go index 1e2e8de265..b6c3031659 100644 --- a/internal/updater/checker.go +++ b/internal/updater/checker.go @@ -143,7 +143,7 @@ func (u *Checker) GetUpdateInfo(desiredChannel, desiredVersion string) (*Availab err = errs.Wrap(err, "Could not fetch update info from %s", infoURL) } - u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, label, &dimensions.Values{ + u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, anaConst.SrcStateTool, label, &dimensions.Values{ Version: ptr.To(desiredVersion), Error: ptr.To(msg), }) @@ -159,6 +159,6 @@ func (u *Checker) GetUpdateInfo(desiredChannel, desiredVersion string) (*Availab info.url = u.fileURL + "/" + info.Path - u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, anaConst.UpdateLabelAvailable, &dimensions.Values{Version: ptr.To(info.Version)}) + u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, anaConst.SrcStateTool, anaConst.UpdateLabelAvailable, &dimensions.Values{Version: ptr.To(info.Version)}) return info, nil } diff --git a/internal/updater/fetcher.go b/internal/updater/fetcher.go index 747d75d976..af8d5c41d6 100644 --- a/internal/updater/fetcher.go +++ b/internal/updater/fetcher.go @@ -70,7 +70,7 @@ func (f *Fetcher) Fetch(update *AvailableUpdate, targetDir string) error { } func (f *Fetcher) analyticsEvent(version, msg string) { - f.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, anaConst.UpdateLabelFailed, &dimensions.Values{ + f.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(version), Error: ptr.To(msg), }) diff --git a/internal/updater/updater.go b/internal/updater/updater.go index d72bcb7881..d656dd1ebc 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -207,5 +207,5 @@ func (u *AvailableUpdate) analyticsEvent(action, label, version, msg string) { dims.Error = ptr.To(msg) } - u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, label, dims) + u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, anaConst.SrcStateTool, label, dims) } diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index a47b2747c0..0c0af27b5f 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -153,7 +153,7 @@ func ReportError(err error, cmd *captain.Command, an analytics.Dispatcher) { } } - an.EventWithLabel(anaConst.CatDebug, action, strings.Join(label, " "), &dimensions.Values{ + an.EventWithLabel(anaConst.CatDebug, action, anaConst.SrcStateTool, strings.Join(label, " "), &dimensions.Values{ Error: ptr.To(errorMsg), }) diff --git a/pkg/cmdlets/git/git.go b/pkg/cmdlets/git/git.go index f0e496685a..70c7e0e75b 100644 --- a/pkg/cmdlets/git/git.go +++ b/pkg/cmdlets/git/git.go @@ -122,7 +122,7 @@ func EnsureCorrectProject(owner, name, projectFilePath, repoURL string, out outp if err != nil { return locale.WrapError(err, "err_git_update_mismatch", "Could not update projectfile namespace") } - an.Event(anaConsts.CatMisc, "git-project-mismatch") + an.Event(anaConsts.CatMisc, "git-project-mismatch", anaConsts.SrcStateTool) } return nil diff --git a/pkg/platform/api/svc/request/analytics.go b/pkg/platform/api/svc/request/analytics.go index 386319d12a..713ebd2b87 100644 --- a/pkg/platform/api/svc/request/analytics.go +++ b/pkg/platform/api/svc/request/analytics.go @@ -1,26 +1,28 @@ package request type AnalyticsEvent struct { - category string - action string - label string - dimensionsJson string - outputType string - userID string + category string + action string + source string + label string + dimensionsJson string + outputType string + userID string } -func NewAnalyticsEvent(category, action, label, dimensionsJson string) *AnalyticsEvent { +func NewAnalyticsEvent(category, action, source, label, dimensionsJson string) *AnalyticsEvent { return &AnalyticsEvent{ - category: category, - action: action, - label: label, - dimensionsJson: dimensionsJson, + category: category, + action: action, + source: source, + label: label, + dimensionsJson: dimensionsJson, } } func (e *AnalyticsEvent) Query() string { - return `query($category: String!, $action: String!, $label: String, $dimensionsJson: String!) { - analyticsEvent(category: $category, action: $action, label: $label, dimensionsJson: $dimensionsJson) { + return `query($category: String!, $action: String!, $source: String!, $label: String, $dimensionsJson: String!) { + analyticsEvent(category: $category, action: $action, source: $source, label: $label, dimensionsJson: $dimensionsJson) { sent } }` @@ -28,9 +30,10 @@ func (e *AnalyticsEvent) Query() string { func (e *AnalyticsEvent) Vars() map[string]interface{} { return map[string]interface{}{ - "category": e.category, - "action": e.action, - "label": e.label, + "category": e.category, + "action": e.action, + "source": e.source, + "label": e.label, "dimensionsJson": e.dimensionsJson, } } diff --git a/pkg/platform/model/svc.go b/pkg/platform/model/svc.go index b1de885514..4a0f3b2bfe 100644 --- a/pkg/platform/model/svc.go +++ b/pkg/platform/model/svc.go @@ -97,10 +97,10 @@ func (m *SvcModel) Ping() error { return err } -func (m *SvcModel) AnalyticsEvent(ctx context.Context, category, action, label string, dimJson string) error { +func (m *SvcModel) AnalyticsEvent(ctx context.Context, category, action, source, label string, dimJson string) error { defer profile.Measure("svc:analyticsEvent", time.Now()) - r := request.NewAnalyticsEvent(category, action, label, dimJson) + r := request.NewAnalyticsEvent(category, action, source, label, dimJson) u := graph.AnalyticsEventResponse{} if err := m.request(ctx, r, &u); err != nil { return errs.Wrap(err, "Error sending analytics event via state-svc") diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index b5eb63d17d..6b3ee7ee29 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -50,6 +50,14 @@ func IsNeedsUpdateError(err error) bool { return errs.Matches(err, &NeedsUpdateError{}) } +var analyticsSource string = anaConsts.SrcStateTool + +func init() { + if osutils.Executable() == constants.StateExecutorCmd { + analyticsSource = anaConsts.SrcExecutor + } +} + func newRuntime(target setup.Targeter, an analytics.Dispatcher, svcModel *model.SvcModel) (*Runtime, error) { rt := &Runtime{ target: target, @@ -76,7 +84,7 @@ func New(target setup.Targeter, an analytics.Dispatcher, svcm *model.SvcModel) ( return &Runtime{disabled: true, target: target}, nil } recordAttempt(an, target) - an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeStart, &dimensions.Values{ + an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeStart, analyticsSource, &dimensions.Values{ Trigger: ptr.To(target.Trigger().String()), Headless: ptr.To(strconv.FormatBool(target.Headless())), CommitID: ptr.To(target.CommitUUID().String()), @@ -86,7 +94,7 @@ func New(target setup.Targeter, an analytics.Dispatcher, svcm *model.SvcModel) ( r, err := newRuntime(target, an, svcm) if err == nil { - an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeCache, &dimensions.Values{ + an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeCache, analyticsSource, &dimensions.Values{ CommitID: ptr.To(target.CommitUUID().String()), }) } @@ -225,7 +233,7 @@ func (r *Runtime) recordCompletion(err error) { message = errs.JoinMessage(err) } - r.analytics.Event(anaConsts.CatRuntime, action, &dimensions.Values{ + r.analytics.Event(anaConsts.CatRuntime, action, analyticsSource, &dimensions.Values{ CommitID: ptr.To(r.target.CommitUUID().String()), // Note: ProjectID is set by state-svc since ProjectNameSpace is specified. ProjectNameSpace: ptr.To(ns.String()), @@ -257,7 +265,7 @@ func recordAttempt(an analytics.Dispatcher, target setup.Targeter) { return } - an.Event(anaConsts.CatRuntimeUsage, anaConsts.ActRuntimeAttempt, usageDims(target)) + an.Event(anaConsts.CatRuntimeUsage, anaConsts.ActRuntimeAttempt, analyticsSource, usageDims(target)) } func usageDims(target setup.Targeter) *dimensions.Values { diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 51049739a2..4441bdc5b3 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -26,6 +26,7 @@ import ( "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/multilog" + "github.com/ActiveState/cli/internal/osutils" "github.com/ActiveState/cli/internal/proxyreader" "github.com/ActiveState/cli/internal/rollbar" "github.com/ActiveState/cli/internal/rtutils/ptr" @@ -432,8 +433,12 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } // send analytics build event, if a new runtime has to be built in the cloud + analyticsSource := anaConsts.SrcStateTool + if osutils.ExecutableName() == constants.StateExecutorCmd { + analyticsSource = anaConsts.SrcExecutor + } if buildResult.BuildStatus == bpModel.Started { - s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeBuild, dimensions) + s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeBuild, analyticsSource, dimensions) } changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false) @@ -542,7 +547,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // only send the download analytics event, if we have to install artifacts that are not yet installed if len(artifactsToInstall) > 0 { // if we get here, we dowload artifacts - s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeDownload, dimensions) + s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeDownload, analyticsSource, dimensions) } err = s.installArtifactsFromBuild(buildResult, runtimeArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, installFunc, logFilePath) diff --git a/test/integration/analytics_int_test.go b/test/integration/analytics_int_test.go index 8bd885f5c8..a6fd606b9c 100644 --- a/test/integration/analytics_int_test.go +++ b/test/integration/analytics_int_test.go @@ -179,7 +179,7 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { func countEvents(events []reporters.TestLogEntry, category, action string) int { filteredEvents := funk.Filter(events, func(e reporters.TestLogEntry) bool { - return e.Category == category && e.Action == action + return e.Category == category && e.Action == action && e.Source != "" }).([]reporters.TestLogEntry) return len(filteredEvents) } diff --git a/test/integration/performance_svc_int_test.go b/test/integration/performance_svc_int_test.go index 71b3d18df4..1ca2a467ff 100644 --- a/test/integration/performance_svc_int_test.go +++ b/test/integration/performance_svc_int_test.go @@ -75,7 +75,7 @@ func (suite *PerformanceIntegrationTestSuite) TestSvcPerformance() { suite.Run("Query Analytics", func() { t := time.Now() - err := svcmodel.AnalyticsEvent(context.Background(), "performance-test", "performance-test", "performance-test", "{}") + err := svcmodel.AnalyticsEvent(context.Background(), "performance-test", "performance-test", "performance-test", "performance-test", "{}") suite.Require().NoError(err, ts.DebugMessage(fmt.Sprintf("Error: %s\nLog Tail:\n%s", errs.JoinMessage(err), logging.ReadTail()))) duration := time.Since(t) From 394b9adf91cd975caeb7ae64363371cb681f8683 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Tue, 12 Sep 2023 15:32:00 -0700 Subject: [PATCH 28/45] Recognize docker/installer changes --- internal/locale/locales/en-us.yaml | 2 ++ pkg/cmdlets/commit/commit.go | 7 +++++++ pkg/platform/model/vcs.go | 3 +++ 3 files changed, 12 insertions(+) diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index 3bdcb21aad..c289b7eff8 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -1344,6 +1344,8 @@ namespace_label_platform: other: "Platform" namespace_label_preplatform: other: "Bits" +namespace_label_packager: + other: "Docker/Installer packagers" print_commit: other: "[NOTICE]commit {{.V0}}[/RESET]" print_commit_author: diff --git a/pkg/cmdlets/commit/commit.go b/pkg/cmdlets/commit/commit.go index 6512a0ca5d..8e5d3676ee 100644 --- a/pkg/cmdlets/commit/commit.go +++ b/pkg/cmdlets/commit/commit.go @@ -105,6 +105,13 @@ func FormatChanges(commit *mono_models.Commit) ([]string, []*requirementChange) versionConstraints = "" } + // This is a temporary fix until we start getting history in the form of build expressions + if model.NamespaceMatch(change.Namespace, model.NamespaceBuildFlagsMatch) && + (strings.Contains(change.Requirement, "docker") || strings.Contains(change.Requirement, "installer")) { + requirement = locale.T("namespace_label_packager") + versionConstraints = "" + } + var result, oldConstraints, newConstraints string switch change.Operation { case string(model.OperationAdded): diff --git a/pkg/platform/model/vcs.go b/pkg/platform/model/vcs.go index c82b7e160f..f42acd88de 100644 --- a/pkg/platform/model/vcs.go +++ b/pkg/platform/model/vcs.go @@ -85,6 +85,9 @@ const ( // NamespaceSharedMatch is the namespace used for shared requirements (usually runtime libraries) NamespaceSharedMatch = `^shared$` + + // NamespaceBuildFlagsMatch is the namespace used for passing build flags + NamespaceBuildFlagsMatch = `^build-flags$` ) type TrackingType string From 1a6ad983ae393b417acff38b253df1d5fe89cf24 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 13 Sep 2023 10:37:09 -0400 Subject: [PATCH 29/45] Move analytics source into clients. Also improve forwarding events on behalf of executors. --- cmd/state-installer/cmd.go | 40 +++++++++---------- cmd/state-offline-installer/install.go | 8 ++-- cmd/state-offline-installer/main.go | 3 +- cmd/state-offline-uninstaller/main.go | 3 +- cmd/state-offline-uninstaller/uninstall.go | 10 ++--- cmd/state-remote-installer/main.go | 3 +- cmd/state-svc/internal/resolver/resolver.go | 12 +++--- cmd/state-svc/internal/rtwatcher/watcher.go | 4 +- cmd/state-svc/internal/server/server.go | 6 +-- cmd/state-svc/main.go | 3 +- cmd/state/autoupdate.go | 14 +++---- cmd/state/internal/cmdtree/activate.go | 8 ++-- cmd/state/internal/cmdtree/tutorial.go | 4 +- cmd/state/main.go | 8 ++-- internal/analytics/analytics.go | 5 ++- internal/analytics/client/async/client.go | 21 ++++++++-- internal/analytics/client/blackhole/client.go | 7 +++- internal/analytics/client/sync/client.go | 23 +++++++++-- internal/analytics/constants/constants.go | 15 +++++-- internal/captain/command.go | 10 ++--- internal/prompt/prompt.go | 8 ++-- internal/runbits/activation/activation.go | 2 +- internal/runbits/requirements/requirements.go | 2 +- internal/runners/activate/activate.go | 2 +- internal/runners/config/set.go | 2 +- .../runners/deploy/uninstall/uninstall.go | 2 +- internal/runners/ppm/convert.go | 16 ++++---- internal/runners/ppm/shim.go | 6 +-- internal/runners/tutorial/tutorial.go | 14 +++---- internal/svcctl/comm.go | 4 +- internal/updater/checker.go | 4 +- internal/updater/fetcher.go | 2 +- internal/updater/updater.go | 2 +- pkg/cmdlets/errors/errors.go | 2 +- pkg/cmdlets/git/git.go | 2 +- pkg/platform/runtime/analytics/analytics.go | 25 ++++++++++++ pkg/platform/runtime/runtime.go | 17 +++----- pkg/platform/runtime/setup/setup.go | 17 +++----- 38 files changed, 198 insertions(+), 138 deletions(-) create mode 100644 pkg/platform/runtime/analytics/analytics.go diff --git a/cmd/state-installer/cmd.go b/cmd/state-installer/cmd.go index b5b1faa54b..462444eff5 100644 --- a/cmd/state-installer/cmd.go +++ b/cmd/state-installer/cmd.go @@ -123,8 +123,8 @@ func main() { logging.Debug("Original Args: %v", os.Args) logging.Debug("Processed Args: %v", processedArgs) - an = sync.New(cfg, nil, out) - an.Event(anaConst.CatInstallerFunnel, "start", anaConst.SrcStateTool) + an = sync.New(anaConst.SrcStateInstaller, cfg, nil, out) + an.Event(anaConst.CatInstallerFunnel, "start") params := newParams() cmd := captain.NewCommand( @@ -191,30 +191,30 @@ func main() { }, ) - an.Event(anaConst.CatInstallerFunnel, "pre-exec", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "pre-exec") err = cmd.Execute(processedArgs[1:]) if err != nil { errors.ReportError(err, cmd, an) if locale.IsInputError(err) { - an.EventWithLabel(anaConst.CatInstaller, "input-error", anaConst.SrcStateTool, errs.JoinMessage(err)) + an.EventWithLabel(anaConst.CatInstaller, "input-error", errs.JoinMessage(err)) logging.Debug("Installer input error: " + errs.JoinMessage(err)) } else { - an.EventWithLabel(anaConst.CatInstaller, "error", anaConst.SrcStateTool, errs.JoinMessage(err)) + an.EventWithLabel(anaConst.CatInstaller, "error", errs.JoinMessage(err)) multilog.Critical("Installer error: " + errs.JoinMessage(err)) } - an.EventWithLabel(anaConst.CatInstallerFunnel, "fail", anaConst.SrcStateTool, errs.JoinMessage(err)) + an.EventWithLabel(anaConst.CatInstallerFunnel, "fail", errs.JoinMessage(err)) exitCode, err = errors.ParseUserFacing(err) if err != nil { out.Error(err) } } else { - an.Event(anaConst.CatInstallerFunnel, "success", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "success") } } func execute(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, args []string, params *Params) error { - an.Event(anaConst.CatInstallerFunnel, "exec", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "exec") if params.path == "" { var err error @@ -272,13 +272,13 @@ func execute(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, if params.isUpdate { route = "update" } - an.Event(anaConst.CatInstallerFunnel, route, anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, route) // Check if state tool already installed if !params.isUpdate && !params.force && stateToolInstalled && !targetingSameBranch { logging.Debug("Cancelling out because State Tool is already installed") out.Print(fmt.Sprintf("State Tool Package Manager is already installed at [NOTICE]%s[/RESET]. To reinstall use the [ACTIONABLE]--force[/RESET] flag.", installPath)) - an.Event(anaConst.CatInstallerFunnel, "already-installed", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "already-installed") params.isUpdate = true return postInstallEvents(out, cfg, an, params) } @@ -293,7 +293,7 @@ func execute(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, // installOrUpdateFromLocalSource is invoked when we're performing an installation where the payload is already provided func installOrUpdateFromLocalSource(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, payloadPath string, params *Params) error { logging.Debug("Install from local source") - an.Event(anaConst.CatInstallerFunnel, "local-source", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "local-source") if !params.isUpdate { // install.sh or install.ps1 downloaded this installer and is running it. out.Print(output.Title("Installing State Tool Package Manager")) @@ -322,12 +322,12 @@ func installOrUpdateFromLocalSource(out output.Outputer, cfg *config.Instance, a } // Run installer - an.Event(anaConst.CatInstallerFunnel, "pre-installer", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "pre-installer") if err := installer.Install(); err != nil { out.Print("[ERROR]x Failed[/RESET]") return err } - an.Event(anaConst.CatInstallerFunnel, "post-installer", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "post-installer") out.Print("[SUCCESS]✔ Done[/RESET]") if !params.isUpdate { @@ -340,7 +340,7 @@ func installOrUpdateFromLocalSource(out output.Outputer, cfg *config.Instance, a } func postInstallEvents(out output.Outputer, cfg *config.Instance, an analytics.Dispatcher, params *Params) error { - an.Event(anaConst.CatInstallerFunnel, "post-install-events", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "post-install-events") installPath, err := resolveInstallPath(params.path) if err != nil { @@ -366,30 +366,30 @@ func postInstallEvents(out output.Outputer, cfg *config.Instance, an analytics.D switch { // Execute provided --command case params.command != "": - an.Event(anaConst.CatInstallerFunnel, "forward-command", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "forward-command") out.Print(fmt.Sprintf("\nRunning `[ACTIONABLE]%s[/RESET]`\n", params.command)) cmd, args := exeutils.DecodeCmd(params.command) if _, _, err := exeutils.ExecuteAndPipeStd(cmd, args, envSlice(binPath)); err != nil { - an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-command-err", anaConst.SrcStateTool, err.Error()) + an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-command-err", err.Error()) return errs.Silence(errs.Wrap(err, "Running provided command failed, error returned: %s", errs.JoinMessage(err))) } // Activate provided --activate Namespace case params.activate.IsValid(): - an.Event(anaConst.CatInstallerFunnel, "forward-activate", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "forward-activate") out.Print(fmt.Sprintf("\nRunning `[ACTIONABLE]state activate %s[/RESET]`\n", params.activate.String())) if _, _, err := exeutils.ExecuteAndPipeStd(stateExe, []string{"activate", params.activate.String()}, envSlice(binPath)); err != nil { - an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-activate-err", anaConst.SrcStateTool, err.Error()) + an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-activate-err", err.Error()) return errs.Silence(errs.Wrap(err, "Could not activate %s, error returned: %s", params.activate.String(), errs.JoinMessage(err))) } // Activate provided --activate-default Namespace case params.activateDefault.IsValid(): - an.Event(anaConst.CatInstallerFunnel, "forward-activate-default", anaConst.SrcStateTool) + an.Event(anaConst.CatInstallerFunnel, "forward-activate-default") out.Print(fmt.Sprintf("\nRunning `[ACTIONABLE]state activate --default %s[/RESET]`\n", params.activateDefault.String())) if _, _, err := exeutils.ExecuteAndPipeStd(stateExe, []string{"activate", params.activateDefault.String(), "--default"}, envSlice(binPath)); err != nil { - an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-activate-default-err", anaConst.SrcStateTool, err.Error()) + an.EventWithLabel(anaConst.CatInstallerFunnel, "forward-activate-default-err", err.Error()) return errs.Silence(errs.Wrap(err, "Could not activate %s, error returned: %s", params.activateDefault.String(), errs.JoinMessage(err))) } case !params.isUpdate && terminal.IsTerminal(int(os.Stdin.Fd())) && os.Getenv(constants.InstallerNoSubshell) != "true" && os.Getenv("TERM") != "dumb": diff --git a/cmd/state-offline-installer/install.go b/cmd/state-offline-installer/install.go index cd46faa035..42d7d0da76 100644 --- a/cmd/state-offline-installer/install.go +++ b/cmd/state-offline-installer/install.go @@ -92,9 +92,9 @@ func (r *runner) Run(params *Params) (rerr error) { return } if locale.IsInputError(rerr) { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, errs.JoinMessage(rerr), installerDimensions) } else { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, errs.JoinMessage(rerr), installerDimensions) } }() @@ -122,7 +122,7 @@ func (r *runner) Run(params *Params) (rerr error) { CommitID: &r.icfg.CommitID, Trigger: ptr.To(target.TriggerOfflineInstaller.String()), } - r.analytics.Event(ac.CatOfflineInstaller, "start", ac.SrcOfflineInstaller, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, "start", installerDimensions) // Detect target path targetPath, err := r.getTargetPath(params.path) @@ -248,7 +248,7 @@ func (r *runner) Run(params *Params) (rerr error) { return errs.Wrap(err, "Could not configure environment") } - r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, ac.SrcOfflineInstaller, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, installerDimensions) r.out.Print(fmt.Sprintf(`Installation complete. Your language runtime has been installed in [ACTIONABLE]%s[/RESET].`, targetPath)) diff --git a/cmd/state-offline-installer/main.go b/cmd/state-offline-installer/main.go index 89dc821608..82b8cd86e7 100644 --- a/cmd/state-offline-installer/main.go +++ b/cmd/state-offline-installer/main.go @@ -8,6 +8,7 @@ import ( "github.com/ActiveState/cli/internal/analytics" "github.com/ActiveState/cli/internal/analytics/client/sync" + anaConst "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/config" "github.com/ActiveState/cli/internal/constants" @@ -78,7 +79,7 @@ func main() { return } - an = sync.New(cfg, nil, out) + an = sync.New(anaConst.SrcOfflineInstaller, cfg, nil, out) prime := primer.New( nil, out, nil, diff --git a/cmd/state-offline-uninstaller/main.go b/cmd/state-offline-uninstaller/main.go index 822e831dfb..0ba17527c5 100644 --- a/cmd/state-offline-uninstaller/main.go +++ b/cmd/state-offline-uninstaller/main.go @@ -8,6 +8,7 @@ import ( "github.com/ActiveState/cli/internal/analytics" "github.com/ActiveState/cli/internal/analytics/client/sync" + anaConst "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/config" "github.com/ActiveState/cli/internal/constants" @@ -78,7 +79,7 @@ func main() { return } - an = sync.New(cfg, nil, out) + an = sync.New(anaConst.SrcOfflineInstaller, cfg, nil, out) prime := primer.New( nil, out, nil, diff --git a/cmd/state-offline-uninstaller/uninstall.go b/cmd/state-offline-uninstaller/uninstall.go index 209891f9ff..08a86065fc 100644 --- a/cmd/state-offline-uninstaller/uninstall.go +++ b/cmd/state-offline-uninstaller/uninstall.go @@ -79,9 +79,9 @@ func (r *runner) Run(params *Params) (rerr error) { return } if locale.IsInputError(rerr) { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerAbort, errs.JoinMessage(rerr), installerDimensions) } else { - r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, ac.SrcOfflineInstaller, errs.JoinMessage(rerr), installerDimensions) + r.analytics.EventWithLabel(ac.CatOfflineInstaller, ac.ActOfflineInstallerFailure, errs.JoinMessage(rerr), installerDimensions) } }() @@ -116,7 +116,7 @@ func (r *runner) Run(params *Params) (rerr error) { CommitID: &r.icfg.CommitID, Trigger: ptr.To(target.TriggerOfflineUninstaller.String()), } - r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerStart, ac.SrcOfflineInstaller, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerStart, installerDimensions) r.out.Print("Removing environment configuration") err = r.removeEnvPaths(namespace) @@ -130,8 +130,8 @@ func (r *runner) Run(params *Params) (rerr error) { return errs.Wrap(err, "Error removing installation directory") } - r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, ac.SrcOfflineInstaller, installerDimensions) - r.analytics.Event(ac.CatRuntimeUsage, ac.ActRuntimeDelete, ac.SrcOfflineInstaller, installerDimensions) + r.analytics.Event(ac.CatOfflineInstaller, ac.ActOfflineInstallerSuccess, installerDimensions) + r.analytics.Event(ac.CatRuntimeUsage, ac.ActRuntimeDelete, installerDimensions) r.out.Print("Uninstall Complete") diff --git a/cmd/state-remote-installer/main.go b/cmd/state-remote-installer/main.go index 6320c67d0f..73ce0feaf2 100644 --- a/cmd/state-remote-installer/main.go +++ b/cmd/state-remote-installer/main.go @@ -10,6 +10,7 @@ import ( "github.com/ActiveState/cli/internal/analytics" "github.com/ActiveState/cli/internal/analytics/client/sync" + anaConst "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/config" "github.com/ActiveState/cli/internal/constants" @@ -94,7 +95,7 @@ func main() { return } - an = sync.New(cfg, nil, out) + an = sync.New(anaConst.SrcStateRemoteInstaller, cfg, nil, out) // Set up prompter prompter := prompt.New(true, an) diff --git a/cmd/state-svc/internal/resolver/resolver.go b/cmd/state-svc/internal/resolver/resolver.go index f027de602f..4f23ea3754 100644 --- a/cmd/state-svc/internal/resolver/resolver.go +++ b/cmd/state-svc/internal/resolver/resolver.go @@ -73,7 +73,9 @@ func New(cfg *config.Instance, an *sync.Client, auth *authentication.Auth) (*Res usageChecker := rtusage.NewChecker(cfg, auth) - anForClient := sync.New(cfg, auth, nil) + // Note: source does not matter here, as analytics sent via the resolver have a source + // (e.g. State Tool or Executor), and that source will be used. + anForClient := sync.New(anaConsts.SrcStateTool, cfg, auth, nil) return &Resolver{ cfg, msg, @@ -102,7 +104,7 @@ func (r *Resolver) Query() genserver.QueryResolver { return r } func (r *Resolver) Version(ctx context.Context) (*graph.Version, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", anaConsts.SrcStateTool, "Version") + r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", "Version") logging.Debug("Version resolver") return &graph.Version{ State: &graph.StateVersion{ @@ -118,7 +120,7 @@ func (r *Resolver) Version(ctx context.Context) (*graph.Version, error) { func (r *Resolver) AvailableUpdate(ctx context.Context) (*graph.AvailableUpdate, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", anaConsts.SrcStateTool, "AvailableUpdate") + r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", "AvailableUpdate") logging.Debug("AvailableUpdate resolver") defer logging.Debug("AvailableUpdate done") @@ -142,7 +144,7 @@ func (r *Resolver) AvailableUpdate(ctx context.Context) (*graph.AvailableUpdate, func (r *Resolver) Projects(ctx context.Context) ([]*graph.Project, error) { defer func() { handlePanics(recover(), debug.Stack()) }() - r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", anaConsts.SrcStateTool, "Projects") + r.an.EventWithLabel(anaConsts.CatStateSvc, "endpoint", "Projects") logging.Debug("Projects resolver") var projects []*graph.Project localConfigProjects := projectfile.GetProjectMapping(r.cfg) @@ -188,7 +190,7 @@ func (r *Resolver) AnalyticsEvent(_ context.Context, category, action, source st return nil }) - r.anForClient.EventWithLabel(category, action, source, label, dims) + r.anForClient.EventWithSourceAndLabel(category, action, source, label, dims) return &graph.AnalyticsEventResponse{Sent: true}, nil } diff --git a/cmd/state-svc/internal/rtwatcher/watcher.go b/cmd/state-svc/internal/rtwatcher/watcher.go index fa22aaeaff..0b68b11895 100644 --- a/cmd/state-svc/internal/rtwatcher/watcher.go +++ b/cmd/state-svc/internal/rtwatcher/watcher.go @@ -30,7 +30,7 @@ type Watcher struct { } type analytics interface { - Event(category, action, source string, dim ...*dimensions.Values) + EventWithSource(category, action, source string, dim ...*dimensions.Values) } func New(cfg *config.Instance, an analytics) *Watcher { @@ -97,7 +97,7 @@ func (w *Watcher) check() { func (w *Watcher) RecordUsage(e entry) { logging.Debug("Recording usage of %s (%d)", e.Exec, e.PID) - w.an.Event(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcStateTool, e.Dims) + w.an.EventWithSource(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor, e.Dims) } func (w *Watcher) Close() error { diff --git a/cmd/state-svc/internal/server/server.go b/cmd/state-svc/internal/server/server.go index f1048e1730..9750b77711 100644 --- a/cmd/state-svc/internal/server/server.go +++ b/cmd/state-svc/internal/server/server.go @@ -75,16 +75,16 @@ func (s *Server) Resolver() *resolver.Resolver { } func (s *Server) Start() error { - s.analytics.Event(constants.CatStateSvc, "start", constants.SrcStateTool) + s.analytics.Event(constants.CatStateSvc, "start") err := s.httpServer.Start(s.listener.Addr().String()) if err != nil { - s.analytics.Event(constants.CatStateSvc, "start-failure", constants.SrcStateTool) + s.analytics.Event(constants.CatStateSvc, "start-failure") } return err } func (s *Server) Shutdown() error { - s.analytics.Event(constants.CatStateSvc, "shutdown", constants.SrcStateTool) + s.analytics.Event(constants.CatStateSvc, "shutdown") logging.Debug("shutting down server") ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() diff --git a/cmd/state-svc/main.go b/cmd/state-svc/main.go index 22faee2da8..0234ad634b 100644 --- a/cmd/state-svc/main.go +++ b/cmd/state-svc/main.go @@ -13,6 +13,7 @@ import ( "github.com/ActiveState/cli/cmd/state-svc/autostart" anaSync "github.com/ActiveState/cli/internal/analytics/client/sync" + anaConst "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/config" "github.com/ActiveState/cli/internal/constants" @@ -98,7 +99,7 @@ func run(cfg *config.Instance) error { } auth := authentication.New(cfg) - an := anaSync.New(cfg, auth, out) + an := anaSync.New(anaConst.SrcStateService, cfg, auth, out) defer an.Wait() if err := autostart.RegisterConfigListener(cfg); err != nil { diff --git a/cmd/state/autoupdate.go b/cmd/state/autoupdate.go index 0992bff9be..54ffc6c74e 100644 --- a/cmd/state/autoupdate.go +++ b/cmd/state/autoupdate.go @@ -63,7 +63,7 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou if !isEnabled(cfg) { logging.Debug("Not performing autoupdates because user turned off autoupdates.") - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, anaConst.SrcStateTool, anaConst.UpdateLabelDisabledConfig) + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, anaConst.UpdateLabelDisabledConfig) out.Notice(output.Title(locale.Tl("update_available_header", "Auto Update"))) out.Notice(locale.Tr("update_available", constants.Version, up.Version)) return false, nil @@ -77,20 +77,20 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou err = up.InstallBlocking("") if err != nil { if os.IsPermission(err) { - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To("Could not update the state tool due to insufficient permissions."), }) return false, locale.WrapInputError(err, locale.Tl("auto_update_permission_err", "", constants.DocumentationURL, errs.JoinMessage(err))) } if errs.Matches(err, &updater.ErrorInProgress{}) { - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To(anaConst.UpdateErrorInProgress), }) return false, nil } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateInstall, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To(anaConst.UpdateErrorInstallFailed), }) @@ -108,14 +108,14 @@ func autoUpdate(args []string, cfg *config.Instance, an analytics.Dispatcher, ou } else if errs.Matches(err, &ErrExecuteRelaunch{}) { msg = anaConst.UpdateErrorRelaunch } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(up.Version), Error: ptr.To(msg), }) return true, errs.Silence(errs.WrapExitCode(err, code)) } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.SrcStateTool, anaConst.UpdateLabelSuccess, &dimensions.Values{ + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateRelaunch, anaConst.UpdateLabelSuccess, &dimensions.Values{ TargetVersion: ptr.To(up.Version), }) return true, nil @@ -187,7 +187,7 @@ func shouldRunAutoUpdate(args []string, cfg *config.Instance, an analytics.Dispa label = anaConst.UpdateLabelLocked } - an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, anaConst.SrcStateTool, label) + an.EventWithLabel(anaConst.CatUpdates, anaConst.ActShouldUpdate, label) return shouldUpdate } diff --git a/cmd/state/internal/cmdtree/activate.go b/cmd/state/internal/cmdtree/activate.go index 0202124e30..d675e300b4 100644 --- a/cmd/state/internal/cmdtree/activate.go +++ b/cmd/state/internal/cmdtree/activate.go @@ -66,19 +66,19 @@ func newActivateCommand(prime *primer.Values) *captain.Command { an := prime.Analytics() var serr interface{ Signal() os.Signal } if errors.As(err, &serr) { - an.Event(constants.CatActivationFlow, "user-interrupt-error", constants.SrcStateTool) + an.Event(constants.CatActivationFlow, "user-interrupt-error") } if locale.IsInputError(err) { // Failed due to user input - an.Event(constants.CatActivationFlow, "user-input-error", constants.SrcStateTool) + an.Event(constants.CatActivationFlow, "user-input-error") } else { var exitErr = &exec.ExitError{} if !errors.As(err, &exitErr) { // Failed due to an error we might need to address - an.Event(constants.CatActivationFlow, "error", constants.SrcStateTool) + an.Event(constants.CatActivationFlow, "error") } else { // Failed due to user subshell actions / events - an.Event(constants.CatActivationFlow, "user-exit-error", constants.SrcStateTool) + an.Event(constants.CatActivationFlow, "user-exit-error") } } } diff --git a/cmd/state/internal/cmdtree/tutorial.go b/cmd/state/internal/cmdtree/tutorial.go index 8c669867d9..af69f9a785 100644 --- a/cmd/state/internal/cmdtree/tutorial.go +++ b/cmd/state/internal/cmdtree/tutorial.go @@ -53,9 +53,9 @@ func newTutorialProjectCommand(prime *primer.Values) *captain.Command { func(ccmd *captain.Command, args []string) error { err := runner.RunNewProject(params) if err != nil { - prime.Analytics().EventWithLabel(constants.CatTutorial, "error", constants.SrcStateTool, errs.JoinMessage(err)) + prime.Analytics().EventWithLabel(constants.CatTutorial, "error", errs.JoinMessage(err)) } else { - prime.Analytics().Event(constants.CatTutorial, "completed", constants.SrcStateTool) + prime.Analytics().Event(constants.CatTutorial, "completed") } return err }, diff --git a/cmd/state/main.go b/cmd/state/main.go index c90ac8ad50..696e5b2df8 100644 --- a/cmd/state/main.go +++ b/cmd/state/main.go @@ -11,11 +11,11 @@ import ( "strings" "time" - "github.com/ActiveState/cli/cmd/state/internal/cmdtree/exechandlers/messenger" - "github.com/ActiveState/cli/internal/captain" - "github.com/ActiveState/cli/cmd/state/internal/cmdtree" + "github.com/ActiveState/cli/cmd/state/internal/cmdtree/exechandlers/messenger" anAsync "github.com/ActiveState/cli/internal/analytics/client/async" + anaConst "github.com/ActiveState/cli/internal/analytics/constants" + "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/config" "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/constraints" @@ -201,7 +201,7 @@ func run(args []string, isInteractive bool, cfg *config.Instance, out output.Out logging.Warning("Could not sync authenticated state: %s", errs.JoinMessage(err)) } - an := anAsync.New(svcmodel, cfg, auth, out, pjNamespace) + an := anAsync.New(anaConst.SrcStateTool, svcmodel, cfg, auth, out, pjNamespace) defer func() { if err := events.WaitForEvents(time.Second, an.Wait); err != nil { logging.Warning("Failed waiting for events: %v", err) diff --git a/internal/analytics/analytics.go b/internal/analytics/analytics.go index 6f2aaa3aca..9292b954e9 100644 --- a/internal/analytics/analytics.go +++ b/internal/analytics/analytics.go @@ -8,8 +8,9 @@ import ( // Dispatcher describes a struct that can send analytics event in the background type Dispatcher interface { - Event(category, action, source string, dim ...*dimensions.Values) - EventWithLabel(category, action, source, label string, dim ...*dimensions.Values) + Event(category, action string, dim ...*dimensions.Values) + EventWithLabel(category, action, label string, dim ...*dimensions.Values) + EventWithSource(category, action, source string, dim ...*dimensions.Values) Wait() Close() } diff --git a/internal/analytics/client/async/client.go b/internal/analytics/client/async/client.go index c590453c11..4aa4fa2452 100644 --- a/internal/analytics/client/async/client.go +++ b/internal/analytics/client/async/client.go @@ -38,13 +38,15 @@ type Client struct { ci bool interactive bool activestateCI bool + source string } var _ analytics.Dispatcher = &Client{} -func New(svcModel *model.SvcModel, cfg *config.Instance, auth *authentication.Auth, out output.Outputer, projectNameSpace string) *Client { +func New(source string, svcModel *model.SvcModel, cfg *config.Instance, auth *authentication.Auth, out output.Outputer, projectNameSpace string) *Client { a := &Client{ eventWaitGroup: &sync.WaitGroup{}, + source: source, } o := string(output.PlainFormatName) @@ -75,12 +77,23 @@ func New(svcModel *model.SvcModel, cfg *config.Instance, auth *authentication.Au } // Event logs an event to google analytics -func (a *Client) Event(category, action, source string, dims ...*dimensions.Values) { - a.EventWithLabel(category, action, source, "", dims...) +func (a *Client) Event(category, action string, dims ...*dimensions.Values) { + a.EventWithLabel(category, action, "", dims...) } // EventWithLabel logs an event with a label to google analytics -func (a *Client) EventWithLabel(category, action, source, label string, dims ...*dimensions.Values) { +func (a *Client) EventWithLabel(category, action, label string, dims ...*dimensions.Values) { + a.eventWithSourceAndLabel(category, action, a.source, label, dims...) +} + +// EventWithSource logs an event with another source to google analytics. +// For example, log runtime events triggered by executors as coming from an executor instead of from +// State Tool. +func (a *Client) EventWithSource(category, action, source string, dims ...*dimensions.Values) { + a.eventWithSourceAndLabel(category, action, source, "", dims...) +} + +func (a *Client) eventWithSourceAndLabel(category, action, source, label string, dims ...*dimensions.Values) { err := a.sendEvent(category, action, source, label, dims...) if err != nil { multilog.Error("Error during analytics.sendEvent: %v", errs.JoinMessage(err)) diff --git a/internal/analytics/client/blackhole/client.go b/internal/analytics/client/blackhole/client.go index a7ac5d4431..e533ac7203 100644 --- a/internal/analytics/client/blackhole/client.go +++ b/internal/analytics/client/blackhole/client.go @@ -13,10 +13,13 @@ func New() *Client { return &Client{} } -func (c Client) Event(category, action, source string, dim ...*dimensions.Values) { +func (c Client) Event(category, action string, dim ...*dimensions.Values) { } -func (c Client) EventWithLabel(category, action, source, label string, dim ...*dimensions.Values) { +func (c Client) EventWithLabel(category, action, label string, dim ...*dimensions.Values) { +} + +func (c Client) EventWithSource(category, action, source string, dim ...*dimensions.Values) { } func (c Client) Wait() { diff --git a/internal/analytics/client/sync/client.go b/internal/analytics/client/sync/client.go index 6ffee1f480..751c377149 100644 --- a/internal/analytics/client/sync/client.go +++ b/internal/analytics/client/sync/client.go @@ -45,16 +45,18 @@ type Client struct { reporters []Reporter sequence int auth *authentication.Auth + source string } var _ analytics.Dispatcher = &Client{} // New initializes the analytics instance with all custom dimensions known at this time -func New(cfg *config.Instance, auth *authentication.Auth, out output.Outputer) *Client { +func New(source string, cfg *config.Instance, auth *authentication.Auth, out output.Outputer) *Client { a := &Client{ eventWaitGroup: &sync.WaitGroup{}, sendReports: true, auth: auth, + source: source, } installSource, err := storage.InstallSource() @@ -165,8 +167,8 @@ func (a *Client) report(category, action, source, label string, dimensions *dime } } -func (a *Client) Event(category, action, source string, dims ...*dimensions.Values) { - a.EventWithLabel(category, action, source, "", dims...) +func (a *Client) Event(category, action string, dims ...*dimensions.Values) { + a.EventWithLabel(category, action, "", dims...) } func mergeDimensions(target *dimensions.Values, dims ...*dimensions.Values) *dimensions.Values { @@ -180,7 +182,20 @@ func mergeDimensions(target *dimensions.Values, dims ...*dimensions.Values) *dim return actualDims } -func (a *Client) EventWithLabel(category, action, source, label string, dims ...*dimensions.Values) { +func (a *Client) EventWithLabel(category, action, label string, dims ...*dimensions.Values) { + a.EventWithSourceAndLabel(category, action, a.source, label, dims...) +} + +// EventWithSource should only be used by clients forwarding events on behalf of another source. +// Otherwise, use Event(). +func (a *Client) EventWithSource(category, action, source string, dims ...*dimensions.Values) { + a.EventWithSourceAndLabel(category, action, source, "", dims...) +} + +// EventWithSourceAndLabel should only be used by clients forwarding events on behalf of another +// source (for example, state-svc forwarding events on behalf of State Tool or an executor). +// Otherwise, use EventWithLabel(). +func (a *Client) EventWithSourceAndLabel(category, action, source, label string, dims ...*dimensions.Values) { if a.customDimensions == nil { if condition.InUnitTest() { return diff --git a/internal/analytics/constants/constants.go b/internal/analytics/constants/constants.go index 60202bf803..268000d438 100644 --- a/internal/analytics/constants/constants.go +++ b/internal/analytics/constants/constants.go @@ -31,13 +31,22 @@ const CatInstaller = "installer" // CatInstallerFunnel is the event category used for installer funnel events. const CatInstallerFunnel = "installer-funnel" -// SrcStateTool is the event source for State Tool. +// SrcStateTool is the event source for events sent by state. const SrcStateTool = "State Tool" -// SrcOfflineInstaller is the event source for offline installers. +// SrcStateService is the event source for events sent by state-svc. +const SrcStateService = "State Service" + +// SrcStateInstaller is the event source for events sent by state-installer. +const SrcStateInstaller = "State Installer" + +// SrcStateRemoteInstaller is the event source for events sent by state-remote-installer. +const SrcStateRemoteInstaller = "State Remote Installer" + +// SrcOfflineInstaller is the event source for events sent by offline installers. const SrcOfflineInstaller = "Offline Installer" -// SrcExecutor is the event source for executors. +// SrcExecutor is the event source for events sent by executors. const SrcExecutor = "Executor" // ActRuntimeHeartbeat is the event action sent when a runtime is in use diff --git a/internal/captain/command.go b/internal/captain/command.go index 9708b9da3e..37e1cd4b21 100644 --- a/internal/captain/command.go +++ b/internal/captain/command.go @@ -634,10 +634,10 @@ func (c *Command) cobraExecHandler(cobraCmd *cobra.Command, args []string) error label = append(label, name) }) - c.analytics.EventWithLabel(anaConsts.CatRunCmd, appEventPrefix+subCommandString, anaConsts.SrcStateTool, strings.Join(label, " ")) + c.analytics.EventWithLabel(anaConsts.CatRunCmd, appEventPrefix+subCommandString, strings.Join(label, " ")) if shim, got := os.LookupEnv(constants.ShimEnvVarName); got { - c.analytics.Event(anaConsts.CatShim, shim, anaConsts.SrcStateTool) + c.analytics.Event(anaConsts.CatShim, shim) } } @@ -708,16 +708,16 @@ func (c *Command) cobraExecHandler(cobraCmd *cobra.Command, args []string) error var serr interface{ Signal() os.Signal } if errors.As(err, &serr) { if c.analytics != nil { - c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, anaConsts.SrcStateTool, "interrupt") + c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, "interrupt") } err = locale.WrapInputError(err, "user_interrupt", "User interrupted the State Tool process.") } else { if c.analytics != nil { if err != nil && (subCommandString == "install" || subCommandString == "activate") { // This is a temporary hack; proper implementation: https://activestatef.atlassian.net/browse/DX-495 - c.analytics.EventWithLabel(anaConsts.CatCommandError, appEventPrefix+subCommandString, anaConsts.SrcStateTool, errs.JoinMessage(err)) + c.analytics.EventWithLabel(anaConsts.CatCommandError, appEventPrefix+subCommandString, errs.JoinMessage(err)) } - c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, anaConsts.SrcStateTool, strconv.Itoa(exitCode)) + c.analytics.EventWithLabel(anaConsts.CatCommandExit, appEventPrefix+subCommandString, strconv.Itoa(exitCode)) } } diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index 9e3759dc79..6da18f055c 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -15,7 +15,7 @@ import ( ) type EventDispatcher interface { - EventWithLabel(category, action, source string, label string, dim ...*dimensions.Values) + EventWithLabel(category, action string, label string, dim ...*dimensions.Values) } // Prompter is the interface used to run our prompt from, useful for mocking in tests @@ -169,7 +169,7 @@ func (p *Prompt) Confirm(title, message string, defaultChoice *bool) (bool, erro p.out.Notice(output.Emphasize(title)) } - p.analytics.EventWithLabel(constants.CatPrompt, title, constants.SrcStateTool, "present") + p.analytics.EventWithLabel(constants.CatPrompt, title, "present") var defChoice bool if defaultChoice != nil { @@ -183,11 +183,11 @@ func (p *Prompt) Confirm(title, message string, defaultChoice *bool) (bool, erro }}, &resp, nil) if err != nil { if err == terminal.InterruptErr { - p.analytics.EventWithLabel(constants.CatPrompt, title, constants.SrcStateTool, "interrupt") + p.analytics.EventWithLabel(constants.CatPrompt, title, "interrupt") } return false, locale.NewInputError(err.Error()) } - p.analytics.EventWithLabel(constants.CatPrompt, title, constants.SrcStateTool, translateConfirm(resp)) + p.analytics.EventWithLabel(constants.CatPrompt, title, translateConfirm(resp)) return resp, nil } diff --git a/internal/runbits/activation/activation.go b/internal/runbits/activation/activation.go index da23f7b4d7..d6d23740d8 100644 --- a/internal/runbits/activation/activation.go +++ b/internal/runbits/activation/activation.go @@ -80,7 +80,7 @@ func ActivateAndWait( } defer fe.Close() - an.Event(anaConst.CatActivationFlow, "before-subshell", anaConst.SrcStateTool) + an.Event(anaConst.CatActivationFlow, "before-subshell") err = <-ss.Errors() if err != nil { diff --git a/internal/runbits/requirements/requirements.go b/internal/runbits/requirements/requirements.go index 2a0f4b82f3..c3d7d81ac7 100644 --- a/internal/runbits/requirements/requirements.go +++ b/internal/runbits/requirements/requirements.go @@ -204,7 +204,7 @@ func (r *RequirementOperation) ExecuteRequirementOperation(requirementName, requ } r.Analytics.EventWithLabel( - anaConsts.CatPackageOp, fmt.Sprintf("%s-%s", operation, langName), anaConsts.SrcStateTool, requirementName, + anaConsts.CatPackageOp, fmt.Sprintf("%s-%s", operation, langName), requirementName, ) if !hasParentCommit { diff --git a/internal/runners/activate/activate.go b/internal/runners/activate/activate.go index 7366816080..14fd6ba218 100644 --- a/internal/runners/activate/activate.go +++ b/internal/runners/activate/activate.go @@ -151,7 +151,7 @@ func (r *Activate) Run(params *ActivateParams) error { } // Have to call this once the project has been set - r.analytics.Event(anaConsts.CatActivationFlow, "start", anaConsts.SrcStateTool) + r.analytics.Event(anaConsts.CatActivationFlow, "start") proj.Source().Persist() diff --git a/internal/runners/config/set.go b/internal/runners/config/set.go index 8b78072531..dca1705333 100644 --- a/internal/runners/config/set.go +++ b/internal/runners/config/set.go @@ -106,5 +106,5 @@ func (s *Set) sendEvent(key string, value string, option configMediator.Option) } } - s.analytics.EventWithLabel(constants.CatConfig, action, constants.SrcStateTool, key) + s.analytics.EventWithLabel(constants.CatConfig, action, key) } diff --git a/internal/runners/deploy/uninstall/uninstall.go b/internal/runners/deploy/uninstall/uninstall.go index 56f18b693b..2ffde0e656 100644 --- a/internal/runners/deploy/uninstall/uninstall.go +++ b/internal/runners/deploy/uninstall/uninstall.go @@ -99,7 +99,7 @@ func (u *Uninstall) Run(params *Params) error { return locale.WrapError(err, "err_deploy_uninstall", "Unable to remove deployed runtime at '{{.V0}}'", path) } - u.analytics.Event(constants.CatRuntimeUsage, constants.ActRuntimeDelete, constants.SrcStateTool, &dimensions.Values{ + u.analytics.Event(constants.CatRuntimeUsage, constants.ActRuntimeDelete, &dimensions.Values{ Trigger: ptr.To(target.TriggerDeploy.String()), CommitID: ptr.To(commitID), ProjectNameSpace: ptr.To(namespace), diff --git a/internal/runners/ppm/convert.go b/internal/runners/ppm/convert.go index 1220dda201..561130d135 100644 --- a/internal/runners/ppm/convert.go +++ b/internal/runners/ppm/convert.go @@ -53,24 +53,24 @@ func (cf *ConversionFlow) StartIfNecessary() (bool, error) { return false, nil } - cf.analytics.Event(anaConsts.CatPpmConversion, "run", anaConsts.SrcStateTool) + cf.analytics.Event(anaConsts.CatPpmConversion, "run") r, err := cf.runSurvey() if err != nil { - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", anaConsts.SrcStateTool, errs.JoinMessage(err)) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", errs.JoinMessage(err)) return true, locale.WrapError(err, "ppm_conversion_survey_error", "Conversion flow failed.") } if r != accepted { - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", anaConsts.SrcStateTool, r.String()) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", r.String()) return true, locale.NewInputError("ppm_conversion_rejected", "Virtual environment creation cancelled.") } err = cf.createVirtualEnv() if err != nil { - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", anaConsts.SrcStateTool, errs.JoinMessage(err)) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "error", errs.JoinMessage(err)) return true, locale.WrapError(err, "ppm_conversion_venv_error", "Failed to create a project.") } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", anaConsts.SrcStateTool, r.String()) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "completed", r.String()) return true, nil } @@ -103,7 +103,7 @@ func (cf *ConversionFlow) runSurvey() (conversionResult, error) { choices[0]: "create-virtual-env-1", choices[1]: "asked-why", } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", anaConsts.SrcStateTool, eventChoices[choice]) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", eventChoices[choice]) if choice == choices[0] { return accepted, nil @@ -141,7 +141,7 @@ func (cf *ConversionFlow) explainVirtualEnv() (conversionResult, error) { convertAnswerCreate: "create-virtual-env-2", no: "still-wants-ppm", } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", anaConsts.SrcStateTool, eventChoices[choice]) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", eventChoices[choice]) switch choice { case convertAnswerCreate: @@ -179,7 +179,7 @@ func (cf *ConversionFlow) explainAskFeedback() (conversionResult, error) { ok: "create-virtual-env-3", exit: "exit", } - cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", anaConsts.SrcStateTool, eventChoices[choice]) + cf.analytics.EventWithLabel(anaConsts.CatPpmConversion, "selection", eventChoices[choice]) if choice == ok { return accepted, nil diff --git a/internal/runners/ppm/shim.go b/internal/runners/ppm/shim.go index 8164220868..6d375cbcce 100644 --- a/internal/runners/ppm/shim.go +++ b/internal/runners/ppm/shim.go @@ -81,9 +81,9 @@ func (s *Shim) RunList(converted bool, args ...string) error { func (s *Shim) shim(intercepted, replaced string, args ...string) error { err := s.executeShim(intercepted, replaced, args...) if err != nil { - s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, constants.SrcStateTool, fmt.Sprintf("error: %v", errs.JoinMessage(err))) + s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, fmt.Sprintf("error: %v", errs.JoinMessage(err))) } else { - s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, constants.SrcStateTool, "success") + s.analytics.EventWithLabel(constants.CatPPMShimCmd, intercepted, "success") } return err } @@ -91,7 +91,7 @@ func (s *Shim) shim(intercepted, replaced string, args ...string) error { func (s *Shim) executeShim(intercepted, replaced string, args ...string) error { if s.project == nil { // TODO: Replace this function call when conversion flow is complete - s.analytics.Event(constants.CatPPMShimCmd, "tutorial", constants.SrcStateTool) + s.analytics.Event(constants.CatPPMShimCmd, "tutorial") return tutorial() } diff --git a/internal/runners/tutorial/tutorial.go b/internal/runners/tutorial/tutorial.go index 867d273f79..c1f9fb00dc 100644 --- a/internal/runners/tutorial/tutorial.go +++ b/internal/runners/tutorial/tutorial.go @@ -45,7 +45,7 @@ type NewProjectParams struct { } func (t *Tutorial) RunNewProject(params NewProjectParams) error { - t.analytics.EventWithLabel(anaConsts.CatTutorial, "run", anaConsts.SrcStateTool, fmt.Sprintf("skipIntro=%v,language=%v", params.SkipIntro, params.Language.String())) + t.analytics.EventWithLabel(anaConsts.CatTutorial, "run", fmt.Sprintf("skipIntro=%v,language=%v", params.SkipIntro, params.Language.String())) // Print intro if !params.SkipIntro { @@ -75,7 +75,7 @@ func (t *Tutorial) RunNewProject(params NewProjectParams) error { if lang == language.Unknown || lang == language.Unset { return locale.NewError("err_tutorial_language_unknown", "Invalid language selected: {{.V0}}.", choice) } - t.analytics.EventWithLabel(anaConsts.CatTutorial, "choose-language", anaConsts.SrcStateTool, lang.String()) + t.analytics.EventWithLabel(anaConsts.CatTutorial, "choose-language", lang.String()) } // Prompt for project name @@ -125,7 +125,7 @@ func (t *Tutorial) RunNewProject(params NewProjectParams) error { // authFlow is invoked when the user is not authenticated, it will prompt for sign in or sign up func (t *Tutorial) authFlow() error { - t.analytics.Event(anaConsts.CatTutorial, "authentication-flow", anaConsts.SrcStateTool) + t.analytics.Event(anaConsts.CatTutorial, "authentication-flow") // Sign in / Sign up choices signIn := locale.Tl("tutorial_signin", "Sign In") @@ -147,17 +147,17 @@ func (t *Tutorial) authFlow() error { // Evaluate user selection switch choice { case signIn: - t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", anaConsts.SrcStateTool, "sign-in") + t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", "sign-in") if err := runbits.Invoke(t.outputer, "auth"); err != nil { return locale.WrapInputError(err, "err_tutorial_signin", "Sign in failed. You could try manually signing in by running `state auth`.") } case signUpCLI: - t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", anaConsts.SrcStateTool, "sign-up") + t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", "sign-up") if err := runbits.Invoke(t.outputer, "auth", "signup"); err != nil { return locale.WrapInputError(err, "err_tutorial_signup", "Sign up failed. You could try manually signing up by running `state auth signup`.") } case signUpBrowser: - t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", anaConsts.SrcStateTool, "sign-up-browser") + t.analytics.EventWithLabel(anaConsts.CatTutorial, "authentication-action", "sign-up-browser") err := open.Run(constants.PlatformSignupURL) if err != nil { return locale.WrapInputError(err, "err_tutorial_browser", "Could not open browser, please manually navigate to {{.V0}}.", constants.PlatformSignupURL) @@ -172,7 +172,7 @@ func (t *Tutorial) authFlow() error { return locale.WrapError(err, "err_tutorial_auth", "Could not authenticate after invoking `state auth ..`.") } - t.analytics.Event(anaConsts.CatTutorial, "authentication-flow-complete", anaConsts.SrcStateTool) + t.analytics.Event(anaConsts.CatTutorial, "authentication-flow-complete") return nil } diff --git a/internal/svcctl/comm.go b/internal/svcctl/comm.go index 0582c2dfae..16e820ae2a 100644 --- a/internal/svcctl/comm.go +++ b/internal/svcctl/comm.go @@ -78,7 +78,7 @@ type Resolver interface { } type AnalyticsReporter interface { - Event(category, action, source string, dims ...*dimensions.Values) + EventWithSource(category, action, source string, dims ...*dimensions.Values) } func HeartbeatHandler(cfg *config.Instance, resolver Resolver, analyticsReporter AnalyticsReporter) ipc.RequestHandler { @@ -138,7 +138,7 @@ func HeartbeatHandler(cfg *config.Instance, resolver Resolver, analyticsReporter } logging.Debug("Firing runtime usage events for %s", metaData.Namespace) - analyticsReporter.Event(constants.CatRuntimeUsage, constants.ActRuntimeAttempt, constants.SrcExecutor, dims) + analyticsReporter.EventWithSource(constants.CatRuntimeUsage, constants.ActRuntimeAttempt, constants.SrcExecutor, dims) _, err = resolver.ReportRuntimeUsage(context.Background(), pidNum, hb.ExecPath, dimsJSON) if err != nil { multilog.Critical("Heartbeat Failure: Failed to report runtime usage in heartbeat handler: %s", errs.JoinMessage(err)) diff --git a/internal/updater/checker.go b/internal/updater/checker.go index b6c3031659..1e2e8de265 100644 --- a/internal/updater/checker.go +++ b/internal/updater/checker.go @@ -143,7 +143,7 @@ func (u *Checker) GetUpdateInfo(desiredChannel, desiredVersion string) (*Availab err = errs.Wrap(err, "Could not fetch update info from %s", infoURL) } - u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, anaConst.SrcStateTool, label, &dimensions.Values{ + u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, label, &dimensions.Values{ Version: ptr.To(desiredVersion), Error: ptr.To(msg), }) @@ -159,6 +159,6 @@ func (u *Checker) GetUpdateInfo(desiredChannel, desiredVersion string) (*Availab info.url = u.fileURL + "/" + info.Path - u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, anaConst.SrcStateTool, anaConst.UpdateLabelAvailable, &dimensions.Values{Version: ptr.To(info.Version)}) + u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateCheck, anaConst.UpdateLabelAvailable, &dimensions.Values{Version: ptr.To(info.Version)}) return info, nil } diff --git a/internal/updater/fetcher.go b/internal/updater/fetcher.go index af8d5c41d6..747d75d976 100644 --- a/internal/updater/fetcher.go +++ b/internal/updater/fetcher.go @@ -70,7 +70,7 @@ func (f *Fetcher) Fetch(update *AvailableUpdate, targetDir string) error { } func (f *Fetcher) analyticsEvent(version, msg string) { - f.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, anaConst.SrcStateTool, anaConst.UpdateLabelFailed, &dimensions.Values{ + f.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, anaConst.UpdateLabelFailed, &dimensions.Values{ TargetVersion: ptr.To(version), Error: ptr.To(msg), }) diff --git a/internal/updater/updater.go b/internal/updater/updater.go index d656dd1ebc..d72bcb7881 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -207,5 +207,5 @@ func (u *AvailableUpdate) analyticsEvent(action, label, version, msg string) { dims.Error = ptr.To(msg) } - u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, anaConst.SrcStateTool, label, dims) + u.an.EventWithLabel(anaConst.CatUpdates, anaConst.ActUpdateDownload, label, dims) } diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index 0c0af27b5f..a47b2747c0 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -153,7 +153,7 @@ func ReportError(err error, cmd *captain.Command, an analytics.Dispatcher) { } } - an.EventWithLabel(anaConst.CatDebug, action, anaConst.SrcStateTool, strings.Join(label, " "), &dimensions.Values{ + an.EventWithLabel(anaConst.CatDebug, action, strings.Join(label, " "), &dimensions.Values{ Error: ptr.To(errorMsg), }) diff --git a/pkg/cmdlets/git/git.go b/pkg/cmdlets/git/git.go index 70c7e0e75b..f0e496685a 100644 --- a/pkg/cmdlets/git/git.go +++ b/pkg/cmdlets/git/git.go @@ -122,7 +122,7 @@ func EnsureCorrectProject(owner, name, projectFilePath, repoURL string, out outp if err != nil { return locale.WrapError(err, "err_git_update_mismatch", "Could not update projectfile namespace") } - an.Event(anaConsts.CatMisc, "git-project-mismatch", anaConsts.SrcStateTool) + an.Event(anaConsts.CatMisc, "git-project-mismatch") } return nil diff --git a/pkg/platform/runtime/analytics/analytics.go b/pkg/platform/runtime/analytics/analytics.go new file mode 100644 index 0000000000..91744bf708 --- /dev/null +++ b/pkg/platform/runtime/analytics/analytics.go @@ -0,0 +1,25 @@ +package analytics + +import ( + "github.com/ActiveState/cli/internal/analytics" + anaConsts "github.com/ActiveState/cli/internal/analytics/constants" + "github.com/ActiveState/cli/internal/analytics/dimensions" + "github.com/ActiveState/cli/internal/constants" + "github.com/ActiveState/cli/internal/osutils" +) + +var isExecutor bool + +func init() { + if osutils.Executable() == constants.StateExecutorCmd { + isExecutor = true + } +} + +// Event emits an analytics event with the proper source (State Tool or Executor). +func Event(an analytics.Dispatcher, category, action string, dims ...*dimensions.Values) { + if isExecutor { + an.EventWithSource(category, action, anaConsts.SrcExecutor, dims...) + } + an.Event(category, action, dims...) +} diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index 6b3ee7ee29..a06a2108bb 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -24,6 +24,7 @@ import ( bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" "github.com/ActiveState/cli/pkg/platform/authentication" "github.com/ActiveState/cli/pkg/platform/model" + anaRun "github.com/ActiveState/cli/pkg/platform/runtime/analytics" "github.com/ActiveState/cli/pkg/platform/runtime/envdef" "github.com/ActiveState/cli/pkg/platform/runtime/setup" "github.com/ActiveState/cli/pkg/platform/runtime/setup/buildlog" @@ -50,14 +51,6 @@ func IsNeedsUpdateError(err error) bool { return errs.Matches(err, &NeedsUpdateError{}) } -var analyticsSource string = anaConsts.SrcStateTool - -func init() { - if osutils.Executable() == constants.StateExecutorCmd { - analyticsSource = anaConsts.SrcExecutor - } -} - func newRuntime(target setup.Targeter, an analytics.Dispatcher, svcModel *model.SvcModel) (*Runtime, error) { rt := &Runtime{ target: target, @@ -84,7 +77,7 @@ func New(target setup.Targeter, an analytics.Dispatcher, svcm *model.SvcModel) ( return &Runtime{disabled: true, target: target}, nil } recordAttempt(an, target) - an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeStart, analyticsSource, &dimensions.Values{ + anaRun.Event(an, anaConsts.CatRuntime, anaConsts.ActRuntimeStart, &dimensions.Values{ Trigger: ptr.To(target.Trigger().String()), Headless: ptr.To(strconv.FormatBool(target.Headless())), CommitID: ptr.To(target.CommitUUID().String()), @@ -94,7 +87,7 @@ func New(target setup.Targeter, an analytics.Dispatcher, svcm *model.SvcModel) ( r, err := newRuntime(target, an, svcm) if err == nil { - an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeCache, analyticsSource, &dimensions.Values{ + anaRun.Event(an, anaConsts.CatRuntime, anaConsts.ActRuntimeCache, &dimensions.Values{ CommitID: ptr.To(target.CommitUUID().String()), }) } @@ -233,7 +226,7 @@ func (r *Runtime) recordCompletion(err error) { message = errs.JoinMessage(err) } - r.analytics.Event(anaConsts.CatRuntime, action, analyticsSource, &dimensions.Values{ + anaRun.Event(r.analytics, anaConsts.CatRuntime, action, &dimensions.Values{ CommitID: ptr.To(r.target.CommitUUID().String()), // Note: ProjectID is set by state-svc since ProjectNameSpace is specified. ProjectNameSpace: ptr.To(ns.String()), @@ -265,7 +258,7 @@ func recordAttempt(an analytics.Dispatcher, target setup.Targeter) { return } - an.Event(anaConsts.CatRuntimeUsage, anaConsts.ActRuntimeAttempt, analyticsSource, usageDims(target)) + anaRun.Event(an, anaConsts.CatRuntimeUsage, anaConsts.ActRuntimeAttempt, usageDims(target)) } func usageDims(target setup.Targeter) *dimensions.Values { diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 4441bdc5b3..fb2fc33c50 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -12,9 +12,6 @@ import ( "sync" "time" - bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" - "github.com/ActiveState/cli/pkg/platform/model" - "github.com/ActiveState/cli/internal/analytics" anaConsts "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/analytics/dimensions" @@ -26,15 +23,17 @@ import ( "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/multilog" - "github.com/ActiveState/cli/internal/osutils" "github.com/ActiveState/cli/internal/proxyreader" "github.com/ActiveState/cli/internal/rollbar" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/ActiveState/cli/internal/svcctl" "github.com/ActiveState/cli/internal/unarchiver" + bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" "github.com/ActiveState/cli/pkg/platform/api/inventory/inventory_models" "github.com/ActiveState/cli/pkg/platform/authentication" + "github.com/ActiveState/cli/pkg/platform/model" apimodel "github.com/ActiveState/cli/pkg/platform/model" + anaRun "github.com/ActiveState/cli/pkg/platform/runtime/analytics" "github.com/ActiveState/cli/pkg/platform/runtime/artifact" "github.com/ActiveState/cli/pkg/platform/runtime/artifactcache" "github.com/ActiveState/cli/pkg/platform/runtime/buildplan" @@ -433,12 +432,8 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } // send analytics build event, if a new runtime has to be built in the cloud - analyticsSource := anaConsts.SrcStateTool - if osutils.ExecutableName() == constants.StateExecutorCmd { - analyticsSource = anaConsts.SrcExecutor - } if buildResult.BuildStatus == bpModel.Started { - s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeBuild, analyticsSource, dimensions) + anaRun.Event(s.analytics, anaConsts.CatRuntime, anaConsts.ActRuntimeBuild, dimensions) } changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false) @@ -546,8 +541,8 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // only send the download analytics event, if we have to install artifacts that are not yet installed if len(artifactsToInstall) > 0 { - // if we get here, we dowload artifacts - s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeDownload, analyticsSource, dimensions) + // if we get here, we download artifacts + anaRun.Event(s.analytics, anaConsts.CatRuntime, anaConsts.ActRuntimeDownload, dimensions) } err = s.installArtifactsFromBuild(buildResult, runtimeArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, installFunc, logFilePath) From 55501ea8b60b2d5b50b646b901bee04410c85650 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 13 Sep 2023 14:05:50 -0400 Subject: [PATCH 30/45] Use Go's url package to manipulate URLs instead of strings and fmt packages. --- pkg/cmdlets/auth/login.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/pkg/cmdlets/auth/login.go b/pkg/cmdlets/auth/login.go index 9c32358882..8424bd7c1c 100644 --- a/pkg/cmdlets/auth/login.go +++ b/pkg/cmdlets/auth/login.go @@ -1,9 +1,7 @@ package auth import ( - "fmt" "net/url" - "strings" "time" "github.com/ActiveState/cli/internal/constants" @@ -251,20 +249,36 @@ func authenticateWithBrowser(out output.Outputer, auth *authentication.Auth, pro return errs.New("Invalid response: Missing verification URL.") } - verificationUrl := *response.VerificationURIComplete + verificationURL := *response.VerificationURIComplete if signup { - baseUrl := strings.TrimPrefix(*response.VerificationURIComplete, "https://"+constants.PlatformURL) - verificationUrl = fmt.Sprintf("%s?nextRoute=%s", constants.PlatformSignupURL, url.QueryEscape(baseUrl)) + // verificationURL is of the form: + // https://platform.activestate.com/authorize/device?user-code=... + // Transform it to the form: + // https://platform.activestate.com/create-account?nextRoute=%2Fauthorize%2Fdevice%3Fuser-code%3D... + parsedURL, err := url.Parse(verificationURL) + if err != nil { + return errs.Wrap(err, "Verification URL is not valid") + } + + signupURL, err := url.Parse(constants.PlatformSignupURL) + if err != nil { + return errs.Wrap(err, "constants.PlatformSignupURL is not valid") + } + query := signupURL.Query() + query.Add("nextRoute", parsedURL.RequestURI()) + signupURL.RawQuery = query.Encode() + + verificationURL = signupURL.String() } // Print code to user if response.UserCode == nil { return errs.New("Invalid response: Missing user code.") } - out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, verificationUrl)) + out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, verificationURL)) // Open URL in browser - err = OpenURI(verificationUrl) + err = OpenURI(verificationURL) if err != nil { logging.Warning("Could not open browser: %v", err) out.Notice(locale.Tr("err_browser_open")) From 2422dd1437b603e45bc491a2444077a7c8feb37e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 13 Sep 2023 12:46:16 -0700 Subject: [PATCH 31/45] Add link to follow-up story --- pkg/cmdlets/commit/commit.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cmdlets/commit/commit.go b/pkg/cmdlets/commit/commit.go index 8e5d3676ee..678387c107 100644 --- a/pkg/cmdlets/commit/commit.go +++ b/pkg/cmdlets/commit/commit.go @@ -106,6 +106,7 @@ func FormatChanges(commit *mono_models.Commit) ([]string, []*requirementChange) } // This is a temporary fix until we start getting history in the form of build expressions + // https://activestatef.atlassian.net/browse/DX-2197 if model.NamespaceMatch(change.Namespace, model.NamespaceBuildFlagsMatch) && (strings.Contains(change.Requirement, "docker") || strings.Contains(change.Requirement, "installer")) { requirement = locale.T("namespace_label_packager") From e90f8ea2b29b838da61efcc16c841e11a6260d5a Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 13 Sep 2023 16:20:12 -0400 Subject: [PATCH 32/45] Attempt to fix executor recognition for analytics source and updated integration tests. --- pkg/platform/runtime/analytics/analytics.go | 6 +-- test/integration/analytics_int_test.go | 42 ++++++++++----------- test/integration/offinstall_int_test.go | 4 +- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/pkg/platform/runtime/analytics/analytics.go b/pkg/platform/runtime/analytics/analytics.go index 91744bf708..e870dd057b 100644 --- a/pkg/platform/runtime/analytics/analytics.go +++ b/pkg/platform/runtime/analytics/analytics.go @@ -4,15 +4,15 @@ import ( "github.com/ActiveState/cli/internal/analytics" anaConsts "github.com/ActiveState/cli/internal/analytics/constants" "github.com/ActiveState/cli/internal/analytics/dimensions" - "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/osutils" + "github.com/ActiveState/cli/pkg/platform/runtime/executors" ) var isExecutor bool func init() { - if osutils.Executable() == constants.StateExecutorCmd { - isExecutor = true + if isExec, err := executors.IsExecutor(osutils.Executable()); err == nil { + isExecutor = isExec } } diff --git a/test/integration/analytics_int_test.go b/test/integration/analytics_int_test.go index a6fd606b9c..093a5de32d 100644 --- a/test/integration/analytics_int_test.go +++ b/test/integration/analytics_int_test.go @@ -82,16 +82,16 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { suite.Require().NotEmpty(events) // Runtime:start events - suite.assertNEvents(events, 1, anaConst.CatRuntime, anaConst.ActRuntimeStart, + suite.assertNEvents(events, 1, anaConst.CatRuntime, anaConst.ActRuntimeStart, anaConst.SrcStateTool, fmt.Sprintf("output:\n%s\n%s", cp.Snapshot(), ts.DebugLogs())) // Runtime:success events - suite.assertNEvents(events, 1, anaConst.CatRuntime, anaConst.ActRuntimeSuccess, + suite.assertNEvents(events, 1, anaConst.CatRuntime, anaConst.ActRuntimeSuccess, anaConst.SrcStateTool, fmt.Sprintf("output:\n%s\n%s", cp.Snapshot(), ts.DebugLogs())) - heartbeatInitialCount := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat) + heartbeatInitialCount := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor) if heartbeatInitialCount < 2 { // It's possible due to the timing of the heartbeats and the fact that they are async that we have gotten either // one or two by this point. Technically more is possible, just very unlikely. @@ -104,7 +104,7 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { suite.Require().NotEmpty(events) // Runtime-use:heartbeat events - should now be at least +1 because we waited - suite.assertGtEvents(events, heartbeatInitialCount, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, + suite.assertGtEvents(events, heartbeatInitialCount, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor, fmt.Sprintf("output:\n%s\n%s", cp.Snapshot(), ts.DebugLogs())) @@ -121,11 +121,11 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { } return (*e.Dimensions.Trigger) == target.TriggerExecutor.String() }) - suite.Require().Equal(1, countEvents(executorEvents, anaConst.CatRuntimeUsage, anaConst.ActRuntimeAttempt), + suite.Require().Equal(1, countEvents(executorEvents, anaConst.CatRuntimeUsage, anaConst.ActRuntimeAttempt, anaConst.SrcExecutor), ts.DebugMessage("Should have a runtime attempt, events:\n"+debugEvents(suite.T(), executorEvents))) // It's possible due to the timing of the heartbeats and the fact that they are async that we have gotten either // one or two by this point. Technically more is possible, just very unlikely. - numHeartbeats := countEvents(executorEvents, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat) + numHeartbeats := countEvents(executorEvents, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor) suite.Require().Greater(numHeartbeats, 0, "Should have a heartbeat") suite.Require().LessOrEqual(numHeartbeats, 2, "Should not have excessive heartbeats") var heartbeatEvent *reporters.TestLogEntry @@ -153,13 +153,13 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { events = parseAnalyticsEvents(suite, ts) suite.Require().NotEmpty(events) - eventsAfterExit := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat) + eventsAfterExit := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor) time.Sleep(sleepTime) eventsAfter := parseAnalyticsEvents(suite, ts) suite.Require().NotEmpty(eventsAfter) - eventsAfterWait := countEvents(eventsAfter, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat) + eventsAfterWait := countEvents(eventsAfter, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor) suite.Equal(eventsAfterExit, eventsAfterWait, fmt.Sprintf("Heartbeats should stop ticking after exiting subshell.\n"+ @@ -177,9 +177,9 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { suite.assertSequentialEvents(events) } -func countEvents(events []reporters.TestLogEntry, category, action string) int { +func countEvents(events []reporters.TestLogEntry, category, action, source string) int { filteredEvents := funk.Filter(events, func(e reporters.TestLogEntry) bool { - return e.Category == category && e.Action == action && e.Source != "" + return e.Category == category && e.Action == action && e.Source == source }).([]reporters.TestLogEntry) return len(filteredEvents) } @@ -203,15 +203,15 @@ func filterEvents(events []reporters.TestLogEntry, filters ...func(e reporters.T } func (suite *AnalyticsIntegrationTestSuite) assertNEvents(events []reporters.TestLogEntry, - expectedN int, category, action string, errMsg string) { - suite.Assert().Equal(expectedN, countEvents(events, category, action), + expectedN int, category, action, source string, errMsg string) { + suite.Assert().Equal(expectedN, countEvents(events, category, action, source), "Expected %d %s:%s events.\nFile location: %s\nEvents received:\n%s\nError:\n%s", expectedN, category, action, suite.eventsfile, suite.summarizeEvents(events), errMsg) } func (suite *AnalyticsIntegrationTestSuite) assertGtEvents(events []reporters.TestLogEntry, - greaterThanN int, category, action string, errMsg string) { - suite.Assert().Greater(countEvents(events, category, action), greaterThanN, + greaterThanN int, category, action, source string, errMsg string) { + suite.Assert().Greater(countEvents(events, category, action, source), greaterThanN, fmt.Sprintf("Expected more than %d %s:%s events.\nFile location: %s\nEvents received:\n%s\nError:\n%s", greaterThanN, category, action, suite.eventsfile, suite.summarizeEvents(events), errMsg)) } @@ -254,7 +254,7 @@ func (suite *AnalyticsIntegrationTestSuite) assertSequentialEvents(events []repo func (suite *AnalyticsIntegrationTestSuite) summarizeEvents(events []reporters.TestLogEntry) string { summary := []string{} for _, event := range events { - summary = append(summary, fmt.Sprintf("%s:%s:%s", event.Category, event.Action, event.Label)) + summary = append(summary, fmt.Sprintf("%s:%s:%s (%s)", event.Category, event.Action, event.Label, event.Source)) } return strings.Join(summary, "\n") } @@ -262,8 +262,8 @@ func (suite *AnalyticsIntegrationTestSuite) summarizeEvents(events []reporters.T func (suite *AnalyticsIntegrationTestSuite) summarizeEventSequence(events []reporters.TestLogEntry) string { summary := []string{} for _, event := range events { - summary = append(summary, fmt.Sprintf("%s:%s:%s (seq: %s:%s:%d)\n", - event.Category, event.Action, event.Label, + summary = append(summary, fmt.Sprintf("%s:%s:%s (%s seq: %s:%s:%d)\n", + event.Category, event.Action, event.Label, event.Source, *event.Dimensions.Command, (*event.Dimensions.InstanceID)[0:6], *event.Dimensions.Sequence)) } return strings.Join(summary, "\n") @@ -423,7 +423,7 @@ func (suite *AnalyticsIntegrationTestSuite) TestInputError() { events := parseAnalyticsEvents(suite, ts) suite.assertSequentialEvents(events) - suite.assertNEvents(events, 1, anaConst.CatDebug, anaConst.ActCommandInputError, + suite.assertNEvents(events, 1, anaConst.CatDebug, anaConst.ActCommandInputError, anaConst.SrcStateTool, fmt.Sprintf("output:\n%s\n%s", cp.Snapshot(), ts.DebugLogs())) @@ -466,7 +466,7 @@ func (suite *AnalyticsIntegrationTestSuite) TestAttempts() { for _, e := range events { if strings.Contains(e.Category, "runtime") && strings.Contains(e.Action, "attempt") { foundAttempts++ - if strings.Contains(*e.Dimensions.Trigger, "exec") { + if strings.Contains(*e.Dimensions.Trigger, "exec") && strings.Contains(e.Source, anaConst.SrcExecutor) { foundExecs++ } } @@ -564,8 +564,8 @@ func (suite *AnalyticsIntegrationTestSuite) TestConfigEvents() { suite.Fail("Should find multiple config events") } - suite.assertNEvents(events, 1, anaConst.CatConfig, anaConst.ActConfigSet, "Should be at one config set event") - suite.assertNEvents(events, 1, anaConst.CatConfig, anaConst.ActConfigUnset, "Should be at one config unset event") + suite.assertNEvents(events, 1, anaConst.CatConfig, anaConst.ActConfigSet, anaConst.SrcStateTool, "Should be at one config set event") + suite.assertNEvents(events, 1, anaConst.CatConfig, anaConst.ActConfigUnset, anaConst.SrcStateTool, "Should be at one config unset event") suite.assertSequentialEvents(events) } diff --git a/test/integration/offinstall_int_test.go b/test/integration/offinstall_int_test.go index ec06707b04..b79c7da7f5 100644 --- a/test/integration/offinstall_int_test.go +++ b/test/integration/offinstall_int_test.go @@ -97,7 +97,7 @@ func (suite *OffInstallIntegrationTestSuite) TestInstallAndUninstall() { heartbeat := suite.filterEvent(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat) suite.assertDimensions(heartbeat) - nDelete := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeDelete) + nDelete := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeDelete, anaConst.SrcOfflineInstaller) if nDelete != 0 { suite.FailNow(fmt.Sprintf("Expected 0 delete events, got %d, events:\n%#v", nDelete, events)) } @@ -147,7 +147,7 @@ func (suite *OffInstallIntegrationTestSuite) TestInstallAndUninstall() { // Verify that our analytics event was fired events := parseAnalyticsEvents(suite, ts) suite.Require().NotEmpty(events) - nHeartbeat := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat) + nHeartbeat := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor) if nHeartbeat != 1 { suite.FailNow(fmt.Sprintf("Expected 1 heartbeat event, got %d, events:\n%#v", nHeartbeat, events)) } From 6cbaf7a24194c77ba68fe04a232b452082c855a9 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 13 Sep 2023 16:01:22 -0700 Subject: [PATCH 33/45] Address PR review feedback --- internal/errs/centralized.go | 5 --- internal/errs/errs.go | 54 ------------------------- internal/errs/userfacing.go | 40 ++++++++++++++++++ internal/runbits/errors/centralized.go | 7 ++++ internal/runners/hello/hello_example.go | 38 ++++++++--------- pkg/cmdlets/errors/errors.go | 6 +-- 6 files changed, 69 insertions(+), 81 deletions(-) delete mode 100644 internal/errs/centralized.go create mode 100644 internal/errs/userfacing.go create mode 100644 internal/runbits/errors/centralized.go diff --git a/internal/errs/centralized.go b/internal/errs/centralized.go deleted file mode 100644 index a86557d417..0000000000 --- a/internal/errs/centralized.go +++ /dev/null @@ -1,5 +0,0 @@ -package errs - -type ErrNoProject struct { - *WrapperError -} diff --git a/internal/errs/errs.go b/internal/errs/errs.go index 2c45abf6f6..252b4543b8 100644 --- a/internal/errs/errs.go +++ b/internal/errs/errs.go @@ -36,33 +36,6 @@ type TransientError interface { IsTransient() } -type UserFacingError interface { - error - UserError() string -} - -type userFacingError struct { - wrapped error - message string - tips []string -} - -func (e *userFacingError) Error() string { - return JoinMessage(Wrap(e.wrapped, e.message)) -} - -func (e *userFacingError) UserError() string { - return e.message -} - -func (e *userFacingError) AddTips(tips ...string) { - e.tips = append(e.tips, tips...) -} - -func (e *userFacingError) ErrorTips() []string { - return e.tips -} - // PackedErrors represents a collection of errors that aren't necessarily related to each other // note that rtutils replicates this functionality to avoid import cycles type PackedErrors struct { @@ -131,22 +104,6 @@ func Wrap(wrapTarget error, message string, args ...interface{}) *WrapperError { return newError(msg, wrapTarget) } -func NewUserFacingError(message string, args ...interface{}) *userFacingError { - return &userFacingError{ - nil, - fmt.Sprintf(message, args...), - nil, - } -} - -func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) *userFacingError { - return &userFacingError{ - wrapTarget, - fmt.Sprintf(message, args...), - nil, - } -} - // Pack creates a new error that packs the given errors together, allowing for multiple errors to be returned func Pack(err error, errs ...error) error { return &PackedErrors{append([]error{err}, errs...)} @@ -301,14 +258,3 @@ func Unpack(err error) []error { } return result } - -// IsUserFacing identifies whether this error was curated for end-users and -// returns that error. This is NOT the same as an error that is localized. -func IsUserFacing(err error) (error, bool) { - var userFacingError UserFacingError - if errors.As(err, &userFacingError) { - return userFacingError, true - } - - return err, false -} diff --git a/internal/errs/userfacing.go b/internal/errs/userfacing.go new file mode 100644 index 0000000000..c5c5a2d454 --- /dev/null +++ b/internal/errs/userfacing.go @@ -0,0 +1,40 @@ +package errs + +type UserFacingError interface { + error + UserError() string +} + +type userFacingError struct { + wrapped error + message string + tips []string +} + +func (e *userFacingError) Error() string { + return "User Facing Error: " + e.UserError() +} + +func (e *userFacingError) UserError() string { + return e.message +} + +func (e *userFacingError) AddTips(tips ...string) { + e.tips = append(e.tips, tips...) +} + +func (e *userFacingError) ErrorTips() []string { + return e.tips +} + +func NewUserFacingError(message string, tips ...string) *userFacingError { + return WrapUserFacingError(nil, message) +} + +func WrapUserFacingError(wrapTarget error, message string, tips ...string) *userFacingError { + return &userFacingError{ + wrapTarget, + message, + tips, + } +} diff --git a/internal/runbits/errors/centralized.go b/internal/runbits/errors/centralized.go new file mode 100644 index 0000000000..3f005a1fda --- /dev/null +++ b/internal/runbits/errors/centralized.go @@ -0,0 +1,7 @@ +package errors + +import "github.com/ActiveState/cli/internal/errs" + +type ErrNoProject struct { + *errs.WrapperError +} diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index b125532799..235fd5efe4 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -13,6 +13,7 @@ import ( "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/primer" "github.com/ActiveState/cli/internal/runbits" + "github.com/ActiveState/cli/internal/runbits/errors" "github.com/ActiveState/cli/pkg/platform/model" "github.com/ActiveState/cli/pkg/project" ) @@ -60,24 +61,23 @@ func New(p primeable) *Hello { // useful for ensuring that errors are wrapped in a user-facing error and // localized. func processError(err *error) { - if err != nil { - switch { - case errs.Matches(*err, &runbits.NoNameProvidedError{}): - // Errors that we are looking for should be wrapped in a user-facing error. - // Ensure we wrap the top-level error returned from the runner and not - // the unpacked error that we are inspecting. - *err = errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) - case errs.Matches(*err, &errs.ErrNoProject{}): - werr := errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory.")) - - // It's useful to offer users reasonable tips on recourses. - *err = errs.AddTips(werr, locale.Tl( - "hello_suggest_checkout", - "Try using [ACTIONABLE]`state checkout`[/RESET] first.", - )) - } - - *err = errs.Wrap(*err, "Cannot say hello") + if err == nil { + return + } + + switch { + case errs.Matches(*err, &runbits.NoNameProvidedError{}): + // Errors that we are looking for should be wrapped in a user-facing error. + // Ensure we wrap the top-level error returned from the runner and not + // the unpacked error that we are inspecting. + *err = errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided.")) + case errs.Matches(*err, &errors.ErrNoProject{}): + // It's useful to offer users reasonable tips on recourses. + *err = errs.WrapUserFacingError( + *err, + locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory."), + locale.Tl("hello_suggest_checkout", "Try using [ACTIONABLE]`state checkout`[/RESET] first."), + ) } } @@ -88,7 +88,7 @@ func (h *Hello) Run(params *RunParams) (rerr error) { h.out.Print(locale.Tl("hello_notice", "This command is for example use only")) if h.project == nil { - return &errs.ErrNoProject{errs.New("Not in a project directory")} + return &errors.ErrNoProject{errs.New("Not in a project directory")} } // Reusable runner logic is contained within the runbits package. diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index 3c74eb216d..d9d8dc49c6 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -100,10 +100,10 @@ func ParseUserFacing(err error) (int, error) { // If there is a user facing error in the error stack we want to ensure // that is it forwarded to the user. - uerr, ok := errs.IsUserFacing(err) - if ok { + var userFacingError errs.UserFacingError + if errors.As(err, &userFacingError) { logging.Debug("Returning user facing error, error stack: \n%s", errs.JoinMessage(err)) - return code, &OutputError{uerr} + return code, &OutputError{userFacingError} } if errs.IsSilent(err) { From 2733e65784178af6e73077acac53a4458d9a9050 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 14 Sep 2023 12:42:01 -0400 Subject: [PATCH 34/45] Use org URL name, not display name when creating projects and project files. --- internal/runners/fork/fork.go | 8 +++++++- internal/runners/initialize/init.go | 4 ++-- pkg/cmdlets/checkout/checkout.go | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/runners/fork/fork.go b/internal/runners/fork/fork.go index 1bfb64c5d9..15fbbcc914 100644 --- a/internal/runners/fork/fork.go +++ b/internal/runners/fork/fork.go @@ -95,11 +95,17 @@ func determineOwner(username string, prompter prompt.Prompter) (string, error) { } options := make([]string, len(orgs)) + displayNameToURLNameMap := make(map[string]string) for i, org := range orgs { options[i] = org.DisplayName + displayNameToURLNameMap[org.DisplayName] = org.URLname } options = append([]string{username}, options...) r, err := prompter.Select(locale.Tl("fork_owner_title", "Owner"), locale.Tl("fork_select_org", "Who should the new project belong to?"), options, new(string)) - return r, err + owner, exists := displayNameToURLNameMap[r] + if !exists { + return "", errs.New("Selected organization does not have a URL name") + } + return owner, err } diff --git a/internal/runners/initialize/init.go b/internal/runners/initialize/init.go index dc5e5a3e96..6186975148 100644 --- a/internal/runners/initialize/init.go +++ b/internal/runners/initialize/init.go @@ -156,8 +156,8 @@ func (r *Initialize) Run(params *RunParams) (rerr error) { return errs.Wrap(err, "Unable to get the user's writable orgs") } for _, org := range orgs { - if strings.EqualFold(org.DisplayName, params.Namespace.Owner) { - owner = org.DisplayName + if strings.EqualFold(org.URLname, params.Namespace.Owner) { + owner = org.URLname break } } diff --git a/pkg/cmdlets/checkout/checkout.go b/pkg/cmdlets/checkout/checkout.go index 42068316dc..a75201bc99 100644 --- a/pkg/cmdlets/checkout/checkout.go +++ b/pkg/cmdlets/checkout/checkout.go @@ -112,7 +112,7 @@ func (r *Checkout) Run(ns *project.Namespaced, branchName, cachePath, targetPath if len(owners) == 0 { return "", locale.NewInputError("err_no_org_name", "Your project's organization name could not be found") } - owner := owners[0].DisplayName + owner := owners[0].URLName // Create the config file, if the repo clone didn't already create it configFile := filepath.Join(path, constants.ConfigFileName) From 7c7ae9cc2a8d254436a7b801ab9a2b14ebbd588c Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 14 Sep 2023 14:55:14 -0400 Subject: [PATCH 35/45] Remove special analytics source handling from runtime. Executors should not invoke this code directly, so there should be no analytics source confusion. --- pkg/platform/runtime/analytics/analytics.go | 25 --------------------- pkg/platform/runtime/runtime.go | 9 ++++---- pkg/platform/runtime/setup/setup.go | 5 ++--- 3 files changed, 6 insertions(+), 33 deletions(-) delete mode 100644 pkg/platform/runtime/analytics/analytics.go diff --git a/pkg/platform/runtime/analytics/analytics.go b/pkg/platform/runtime/analytics/analytics.go deleted file mode 100644 index e870dd057b..0000000000 --- a/pkg/platform/runtime/analytics/analytics.go +++ /dev/null @@ -1,25 +0,0 @@ -package analytics - -import ( - "github.com/ActiveState/cli/internal/analytics" - anaConsts "github.com/ActiveState/cli/internal/analytics/constants" - "github.com/ActiveState/cli/internal/analytics/dimensions" - "github.com/ActiveState/cli/internal/osutils" - "github.com/ActiveState/cli/pkg/platform/runtime/executors" -) - -var isExecutor bool - -func init() { - if isExec, err := executors.IsExecutor(osutils.Executable()); err == nil { - isExecutor = isExec - } -} - -// Event emits an analytics event with the proper source (State Tool or Executor). -func Event(an analytics.Dispatcher, category, action string, dims ...*dimensions.Values) { - if isExecutor { - an.EventWithSource(category, action, anaConsts.SrcExecutor, dims...) - } - an.Event(category, action, dims...) -} diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index a06a2108bb..b5eb63d17d 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -24,7 +24,6 @@ import ( bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" "github.com/ActiveState/cli/pkg/platform/authentication" "github.com/ActiveState/cli/pkg/platform/model" - anaRun "github.com/ActiveState/cli/pkg/platform/runtime/analytics" "github.com/ActiveState/cli/pkg/platform/runtime/envdef" "github.com/ActiveState/cli/pkg/platform/runtime/setup" "github.com/ActiveState/cli/pkg/platform/runtime/setup/buildlog" @@ -77,7 +76,7 @@ func New(target setup.Targeter, an analytics.Dispatcher, svcm *model.SvcModel) ( return &Runtime{disabled: true, target: target}, nil } recordAttempt(an, target) - anaRun.Event(an, anaConsts.CatRuntime, anaConsts.ActRuntimeStart, &dimensions.Values{ + an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeStart, &dimensions.Values{ Trigger: ptr.To(target.Trigger().String()), Headless: ptr.To(strconv.FormatBool(target.Headless())), CommitID: ptr.To(target.CommitUUID().String()), @@ -87,7 +86,7 @@ func New(target setup.Targeter, an analytics.Dispatcher, svcm *model.SvcModel) ( r, err := newRuntime(target, an, svcm) if err == nil { - anaRun.Event(an, anaConsts.CatRuntime, anaConsts.ActRuntimeCache, &dimensions.Values{ + an.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeCache, &dimensions.Values{ CommitID: ptr.To(target.CommitUUID().String()), }) } @@ -226,7 +225,7 @@ func (r *Runtime) recordCompletion(err error) { message = errs.JoinMessage(err) } - anaRun.Event(r.analytics, anaConsts.CatRuntime, action, &dimensions.Values{ + r.analytics.Event(anaConsts.CatRuntime, action, &dimensions.Values{ CommitID: ptr.To(r.target.CommitUUID().String()), // Note: ProjectID is set by state-svc since ProjectNameSpace is specified. ProjectNameSpace: ptr.To(ns.String()), @@ -258,7 +257,7 @@ func recordAttempt(an analytics.Dispatcher, target setup.Targeter) { return } - anaRun.Event(an, anaConsts.CatRuntimeUsage, anaConsts.ActRuntimeAttempt, usageDims(target)) + an.Event(anaConsts.CatRuntimeUsage, anaConsts.ActRuntimeAttempt, usageDims(target)) } func usageDims(target setup.Targeter) *dimensions.Values { diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index fb2fc33c50..b5307d8217 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -33,7 +33,6 @@ import ( "github.com/ActiveState/cli/pkg/platform/authentication" "github.com/ActiveState/cli/pkg/platform/model" apimodel "github.com/ActiveState/cli/pkg/platform/model" - anaRun "github.com/ActiveState/cli/pkg/platform/runtime/analytics" "github.com/ActiveState/cli/pkg/platform/runtime/artifact" "github.com/ActiveState/cli/pkg/platform/runtime/artifactcache" "github.com/ActiveState/cli/pkg/platform/runtime/buildplan" @@ -433,7 +432,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // send analytics build event, if a new runtime has to be built in the cloud if buildResult.BuildStatus == bpModel.Started { - anaRun.Event(s.analytics, anaConsts.CatRuntime, anaConsts.ActRuntimeBuild, dimensions) + s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeBuild, dimensions) } changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false) @@ -542,7 +541,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // only send the download analytics event, if we have to install artifacts that are not yet installed if len(artifactsToInstall) > 0 { // if we get here, we download artifacts - anaRun.Event(s.analytics, anaConsts.CatRuntime, anaConsts.ActRuntimeDownload, dimensions) + s.analytics.Event(anaConsts.CatRuntime, anaConsts.ActRuntimeDownload, dimensions) } err = s.installArtifactsFromBuild(buildResult, runtimeArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, installFunc, logFilePath) From 137921f2fa563812bfcd32d8cf813315b631f0ba Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 14 Sep 2023 14:02:40 -0700 Subject: [PATCH 36/45] Remove addition of build-time artifacts from event handling --- pkg/platform/runtime/setup/setup.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 51049739a2..051295b4a6 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -485,7 +485,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ) artifactsToInstall := []artifact.ArtifactID{} - buildtimeArtifacts := runtimeArtifacts + // buildtimeArtifacts := runtimeArtifacts if buildResult.BuildReady { // If the build is already done we can just look at the downloadable artifacts as they will be a fully accurate // prediction of what we will be installing. @@ -502,13 +502,6 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal artifactsToInstall = append(artifactsToInstall, a.ArtifactID) } } - - // We also caclulate the artifacts to be built which includes more than the runtime artifacts. - // This is used to determine if we need to show the "build in progress" screen. - buildtimeArtifacts, err = buildplan.BuildtimeArtifacts(buildResult.Build, false) - if err != nil { - return nil, nil, errs.Wrap(err, "Could not get buildtime artifacts") - } } // The log file we want to use for builds @@ -525,7 +518,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ArtifactNames: artifactNames, LogFilePath: logFilePath, ArtifactsToBuild: func() []artifact.ArtifactID { - return artifact.ArtifactIDsFromBuildPlanMap(buildtimeArtifacts) // This does not account for cached builds + return artifact.ArtifactIDsFromBuildPlanMap(runtimeArtifacts) // This does not account for cached builds }(), // Yes these have the same value; this is intentional. // Separating these out just allows us to be more explicit and intentional in our event handling logic. From a6928da580c2ec0bfaf66c05f69900d59b83e9ce Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Sep 2023 10:27:56 -0400 Subject: [PATCH 37/45] State Tool can emit heartbeats too, not just executors. --- cmd/state-svc/internal/rtwatcher/watcher.go | 8 +++++++- test/integration/analytics_int_test.go | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/state-svc/internal/rtwatcher/watcher.go b/cmd/state-svc/internal/rtwatcher/watcher.go index 0b68b11895..cd8948842e 100644 --- a/cmd/state-svc/internal/rtwatcher/watcher.go +++ b/cmd/state-svc/internal/rtwatcher/watcher.go @@ -3,6 +3,7 @@ package rtwatcher import ( "encoding/json" "os" + "path/filepath" "runtime/debug" "strconv" "time" @@ -14,6 +15,7 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/multilog" + "github.com/ActiveState/cli/internal/osutils" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/ActiveState/cli/internal/runbits/panics" ) @@ -97,7 +99,11 @@ func (w *Watcher) check() { func (w *Watcher) RecordUsage(e entry) { logging.Debug("Recording usage of %s (%d)", e.Exec, e.PID) - w.an.EventWithSource(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor, e.Dims) + source := anaConst.SrcExecutor + if filepath.Base(e.Exec) == constants.StateCmd+osutils.ExeExt { + source = anaConst.SrcStateTool + } + w.an.EventWithSource(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, source, e.Dims) } func (w *Watcher) Close() error { diff --git a/test/integration/analytics_int_test.go b/test/integration/analytics_int_test.go index 093a5de32d..0ae8d6196a 100644 --- a/test/integration/analytics_int_test.go +++ b/test/integration/analytics_int_test.go @@ -91,7 +91,7 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { fmt.Sprintf("output:\n%s\n%s", cp.Snapshot(), ts.DebugLogs())) - heartbeatInitialCount := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor) + heartbeatInitialCount := countEvents(events, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcStateTool) if heartbeatInitialCount < 2 { // It's possible due to the timing of the heartbeats and the fact that they are async that we have gotten either // one or two by this point. Technically more is possible, just very unlikely. @@ -104,7 +104,7 @@ func (suite *AnalyticsIntegrationTestSuite) TestActivateEvents() { suite.Require().NotEmpty(events) // Runtime-use:heartbeat events - should now be at least +1 because we waited - suite.assertGtEvents(events, heartbeatInitialCount, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcExecutor, + suite.assertGtEvents(events, heartbeatInitialCount, anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, anaConst.SrcStateTool, fmt.Sprintf("output:\n%s\n%s", cp.Snapshot(), ts.DebugLogs())) From bfef23de2dfd736d47da7d93c579ba6f63eea2e1 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Sep 2023 12:17:22 -0400 Subject: [PATCH 38/45] Heartbeats should contain their source. --- cmd/state-svc/internal/resolver/resolver.go | 4 ++-- cmd/state-svc/internal/rtwatcher/entry.go | 7 +++--- cmd/state-svc/internal/rtwatcher/watcher.go | 12 +++------- .../internal/server/generated/generated.go | 23 +++++++++++++------ cmd/state-svc/schema/schema.graphqls | 2 +- internal/svcctl/comm.go | 4 ++-- pkg/platform/api/svc/request/reportrtusage.go | 9 +++++--- pkg/platform/model/svc.go | 4 ++-- pkg/platform/runtime/runtime.go | 2 +- 9 files changed, 37 insertions(+), 30 deletions(-) diff --git a/cmd/state-svc/internal/resolver/resolver.go b/cmd/state-svc/internal/resolver/resolver.go index 4f23ea3754..ac3297a0e8 100644 --- a/cmd/state-svc/internal/resolver/resolver.go +++ b/cmd/state-svc/internal/resolver/resolver.go @@ -195,7 +195,7 @@ func (r *Resolver) AnalyticsEvent(_ context.Context, category, action, source st return &graph.AnalyticsEventResponse{Sent: true}, nil } -func (r *Resolver) ReportRuntimeUsage(_ context.Context, pid int, exec string, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) { +func (r *Resolver) ReportRuntimeUsage(_ context.Context, pid int, exec, source string, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) { defer func() { handlePanics(recover(), debug.Stack()) }() logging.Debug("Runtime usage resolver: %d - %s", pid, exec) @@ -204,7 +204,7 @@ func (r *Resolver) ReportRuntimeUsage(_ context.Context, pid int, exec string, d return &graph.ReportRuntimeUsageResponse{Received: false}, errs.Wrap(err, "Could not unmarshal") } - r.rtwatch.Watch(pid, exec, dims) + r.rtwatch.Watch(pid, exec, source, dims) return &graph.ReportRuntimeUsageResponse{Received: true}, nil } diff --git a/cmd/state-svc/internal/rtwatcher/entry.go b/cmd/state-svc/internal/rtwatcher/entry.go index 78275ad703..7626e57af4 100644 --- a/cmd/state-svc/internal/rtwatcher/entry.go +++ b/cmd/state-svc/internal/rtwatcher/entry.go @@ -11,9 +11,10 @@ import ( ) type entry struct { - PID int `json:"pid"` - Exec string `json:"exec"` - Dims *dimensions.Values `json:"dims"` + PID int `json:"pid"` + Exec string `json:"exec"` + Source string `json:"source"` + Dims *dimensions.Values `json:"dims"` } func (e entry) IsRunning() (bool, error) { diff --git a/cmd/state-svc/internal/rtwatcher/watcher.go b/cmd/state-svc/internal/rtwatcher/watcher.go index cd8948842e..dac20f12b2 100644 --- a/cmd/state-svc/internal/rtwatcher/watcher.go +++ b/cmd/state-svc/internal/rtwatcher/watcher.go @@ -3,7 +3,6 @@ package rtwatcher import ( "encoding/json" "os" - "path/filepath" "runtime/debug" "strconv" "time" @@ -15,7 +14,6 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/multilog" - "github.com/ActiveState/cli/internal/osutils" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/ActiveState/cli/internal/runbits/panics" ) @@ -99,11 +97,7 @@ func (w *Watcher) check() { func (w *Watcher) RecordUsage(e entry) { logging.Debug("Recording usage of %s (%d)", e.Exec, e.PID) - source := anaConst.SrcExecutor - if filepath.Base(e.Exec) == constants.StateCmd+osutils.ExeExt { - source = anaConst.SrcStateTool - } - w.an.EventWithSource(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, source, e.Dims) + w.an.EventWithSource(anaConst.CatRuntimeUsage, anaConst.ActRuntimeHeartbeat, e.Source, e.Dims) } func (w *Watcher) Close() error { @@ -122,10 +116,10 @@ func (w *Watcher) Close() error { return nil } -func (w *Watcher) Watch(pid int, exec string, dims *dimensions.Values) { +func (w *Watcher) Watch(pid int, exec, source string, dims *dimensions.Values) { logging.Debug("Watching %s (%d)", exec, pid) dims.Sequence = ptr.To(-1) // sequence is meaningless for heartbeat events - e := entry{pid, exec, dims} + e := entry{pid, exec, source, dims} w.watching = append(w.watching, e) go w.RecordUsage(e) // initial event } diff --git a/cmd/state-svc/internal/server/generated/generated.go b/cmd/state-svc/internal/server/generated/generated.go index 104c805ca9..78720300ff 100644 --- a/cmd/state-svc/internal/server/generated/generated.go +++ b/cmd/state-svc/internal/server/generated/generated.go @@ -85,7 +85,7 @@ type ComplexityRoot struct { ConfigChanged func(childComplexity int, key string) int FetchLogTail func(childComplexity int) int Projects func(childComplexity int) int - ReportRuntimeUsage func(childComplexity int, pid int, exec string, dimensionsJSON string) int + ReportRuntimeUsage func(childComplexity int, pid int, exec string, source string, dimensionsJSON string) int Version func(childComplexity int) int } @@ -111,7 +111,7 @@ type QueryResolver interface { AvailableUpdate(ctx context.Context) (*graph.AvailableUpdate, error) Projects(ctx context.Context) ([]*graph.Project, error) AnalyticsEvent(ctx context.Context, category string, action string, source string, label *string, dimensionsJSON string) (*graph.AnalyticsEventResponse, error) - ReportRuntimeUsage(ctx context.Context, pid int, exec string, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) + ReportRuntimeUsage(ctx context.Context, pid int, exec string, source string, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) CheckRuntimeUsage(ctx context.Context, organizationName string) (*graph.CheckRuntimeUsageResponse, error) CheckMessages(ctx context.Context, command string, flags []string) ([]*graph.MessageInfo, error) ConfigChanged(ctx context.Context, key string) (*graph.ConfigChangedResponse, error) @@ -331,7 +331,7 @@ func (e *executableSchema) Complexity(typeName, field string, childComplexity in return 0, false } - return e.complexity.Query.ReportRuntimeUsage(childComplexity, args["pid"].(int), args["exec"].(string), args["dimensionsJson"].(string)), true + return e.complexity.Query.ReportRuntimeUsage(childComplexity, args["pid"].(int), args["exec"].(string), args["source"].(string), args["dimensionsJson"].(string)), true case "Query.version": if e.complexity.Query.Version == nil { @@ -513,7 +513,7 @@ type Query { availableUpdate: AvailableUpdate projects: [Project]! analyticsEvent(category: String!, action: String!, source: String!, label: String, dimensionsJson: String!): AnalyticsEventResponse - reportRuntimeUsage(pid: Int!, exec: String!, dimensionsJson: String!): ReportRuntimeUsageResponse + reportRuntimeUsage(pid: Int!, exec: String!, source: String!, dimensionsJson: String!): ReportRuntimeUsageResponse checkRuntimeUsage(organizationName: String!): CheckRuntimeUsageResponse checkMessages(command: String!, flags: [String!]!): [MessageInfo!]! configChanged(key: String!): ConfigChangedResponse @@ -673,14 +673,23 @@ func (ec *executionContext) field_Query_reportRuntimeUsage_args(ctx context.Cont } args["exec"] = arg1 var arg2 string + if tmp, ok := rawArgs["source"]; ok { + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("source")) + arg2, err = ec.unmarshalNString2string(ctx, tmp) + if err != nil { + return nil, err + } + } + args["source"] = arg2 + var arg3 string if tmp, ok := rawArgs["dimensionsJson"]; ok { ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("dimensionsJson")) - arg2, err = ec.unmarshalNString2string(ctx, tmp) + arg3, err = ec.unmarshalNString2string(ctx, tmp) if err != nil { return nil, err } } - args["dimensionsJson"] = arg2 + args["dimensionsJson"] = arg3 return args, nil } @@ -1684,7 +1693,7 @@ func (ec *executionContext) _Query_reportRuntimeUsage(ctx context.Context, field }() resTmp, err := ec.ResolverMiddleware(ctx, func(rctx context.Context) (interface{}, error) { ctx = rctx // use context from middleware stack in children - return ec.resolvers.Query().ReportRuntimeUsage(rctx, fc.Args["pid"].(int), fc.Args["exec"].(string), fc.Args["dimensionsJson"].(string)) + return ec.resolvers.Query().ReportRuntimeUsage(rctx, fc.Args["pid"].(int), fc.Args["exec"].(string), fc.Args["source"].(string), fc.Args["dimensionsJson"].(string)) }) if err != nil { ec.Error(ctx, err) diff --git a/cmd/state-svc/schema/schema.graphqls b/cmd/state-svc/schema/schema.graphqls index 6af4224bbd..3b22d3c0b1 100644 --- a/cmd/state-svc/schema/schema.graphqls +++ b/cmd/state-svc/schema/schema.graphqls @@ -70,7 +70,7 @@ type Query { availableUpdate: AvailableUpdate projects: [Project]! analyticsEvent(category: String!, action: String!, source: String!, label: String, dimensionsJson: String!): AnalyticsEventResponse - reportRuntimeUsage(pid: Int!, exec: String!, dimensionsJson: String!): ReportRuntimeUsageResponse + reportRuntimeUsage(pid: Int!, exec: String!, source: String!, dimensionsJson: String!): ReportRuntimeUsageResponse checkRuntimeUsage(organizationName: String!): CheckRuntimeUsageResponse checkMessages(command: String!, flags: [String!]!): [MessageInfo!]! configChanged(key: String!): ConfigChangedResponse diff --git a/internal/svcctl/comm.go b/internal/svcctl/comm.go index 16e820ae2a..7ada81cdcf 100644 --- a/internal/svcctl/comm.go +++ b/internal/svcctl/comm.go @@ -73,7 +73,7 @@ func (c *Comm) GetLogFileName(ctx context.Context) (string, error) { } type Resolver interface { - ReportRuntimeUsage(ctx context.Context, pid int, exec, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) + ReportRuntimeUsage(ctx context.Context, pid int, exec, source string, dimensionsJSON string) (*graph.ReportRuntimeUsageResponse, error) CheckRuntimeUsage(ctx context.Context, organizationName string) (*graph.CheckRuntimeUsageResponse, error) } @@ -139,7 +139,7 @@ func HeartbeatHandler(cfg *config.Instance, resolver Resolver, analyticsReporter logging.Debug("Firing runtime usage events for %s", metaData.Namespace) analyticsReporter.EventWithSource(constants.CatRuntimeUsage, constants.ActRuntimeAttempt, constants.SrcExecutor, dims) - _, err = resolver.ReportRuntimeUsage(context.Background(), pidNum, hb.ExecPath, dimsJSON) + _, err = resolver.ReportRuntimeUsage(context.Background(), pidNum, hb.ExecPath, constants.SrcExecutor, dimsJSON) if err != nil { multilog.Critical("Heartbeat Failure: Failed to report runtime usage in heartbeat handler: %s", errs.JoinMessage(err)) return diff --git a/pkg/platform/api/svc/request/reportrtusage.go b/pkg/platform/api/svc/request/reportrtusage.go index 48ff8b4da8..027c04b6ca 100644 --- a/pkg/platform/api/svc/request/reportrtusage.go +++ b/pkg/platform/api/svc/request/reportrtusage.go @@ -3,20 +3,22 @@ package request type ReportRuntimeUsage struct { pid int exec string + source string dimensionsJson string } -func NewReportRuntimeUsage(pid int, exec string, dimensionsJson string) *ReportRuntimeUsage { +func NewReportRuntimeUsage(pid int, exec, source string, dimensionsJson string) *ReportRuntimeUsage { return &ReportRuntimeUsage{ pid: pid, exec: exec, + source: source, dimensionsJson: dimensionsJson, } } func (e *ReportRuntimeUsage) Query() string { - return `query($pid: Int!, $exec: String!, $dimensionsJson: String!) { - reportRuntimeUsage(pid: $pid, exec: $exec, dimensionsJson: $dimensionsJson) { + return `query($pid: Int!, $exec: String!, $source: String!, $dimensionsJson: String!) { + reportRuntimeUsage(pid: $pid, exec: $exec, source: $source, dimensionsJson: $dimensionsJson) { received } }` @@ -26,6 +28,7 @@ func (e *ReportRuntimeUsage) Vars() map[string]interface{} { return map[string]interface{}{ "pid": e.pid, "exec": e.exec, + "source": e.source, "dimensionsJson": e.dimensionsJson, } } diff --git a/pkg/platform/model/svc.go b/pkg/platform/model/svc.go index 4a0f3b2bfe..ce074e398c 100644 --- a/pkg/platform/model/svc.go +++ b/pkg/platform/model/svc.go @@ -109,10 +109,10 @@ func (m *SvcModel) AnalyticsEvent(ctx context.Context, category, action, source, return nil } -func (m *SvcModel) ReportRuntimeUsage(ctx context.Context, pid int, exec string, dimJson string) error { +func (m *SvcModel) ReportRuntimeUsage(ctx context.Context, pid int, exec, source string, dimJson string) error { defer profile.Measure("svc:ReportRuntimeUsage", time.Now()) - r := request.NewReportRuntimeUsage(pid, exec, dimJson) + r := request.NewReportRuntimeUsage(pid, exec, source, dimJson) u := graph.ReportRuntimeUsageResponse{} if err := m.request(ctx, r, &u); err != nil { return errs.Wrap(err, "Error sending report runtime usage event via state-svc") diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index b5eb63d17d..4ccb330ed8 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -247,7 +247,7 @@ func (r *Runtime) recordUsage() { multilog.Critical("Could not marshal dimensions for runtime-usage: %s", errs.JoinMessage(err)) } if r.svcm != nil { - r.svcm.ReportRuntimeUsage(context.Background(), os.Getpid(), osutils.Executable(), dimsJson) + r.svcm.ReportRuntimeUsage(context.Background(), os.Getpid(), osutils.Executable(), anaConsts.SrcStateTool, dimsJson) } } From a85fdc858e165ee28bcf283664cf21718c2da37d Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 15 Sep 2023 11:28:14 -0700 Subject: [PATCH 39/45] Address interface collision Update error tip handling --- internal/errs/userfacing.go | 32 +++++++++++++++---- internal/locale/errors.go | 8 ++--- internal/osutils/user/user.go | 2 +- internal/runners/hello/hello_example.go | 4 ++- pkg/cmdlets/errors/errors.go | 12 +++---- .../api/buildplanner/model/buildplan.go | 2 +- pkg/platform/runtime/setup/setup.go | 2 +- 7 files changed, 41 insertions(+), 21 deletions(-) diff --git a/internal/errs/userfacing.go b/internal/errs/userfacing.go index c5c5a2d454..88b032d181 100644 --- a/internal/errs/userfacing.go +++ b/internal/errs/userfacing.go @@ -1,10 +1,14 @@ package errs +import "errors" + type UserFacingError interface { error UserError() string } +type ErrOpts func(err *userFacingError) *userFacingError + type userFacingError struct { wrapped error message string @@ -19,10 +23,6 @@ func (e *userFacingError) UserError() string { return e.message } -func (e *userFacingError) AddTips(tips ...string) { - e.tips = append(e.tips, tips...) -} - func (e *userFacingError) ErrorTips() []string { return e.tips } @@ -31,10 +31,28 @@ func NewUserFacingError(message string, tips ...string) *userFacingError { return WrapUserFacingError(nil, message) } -func WrapUserFacingError(wrapTarget error, message string, tips ...string) *userFacingError { - return &userFacingError{ +func WrapUserFacingError(wrapTarget error, message string, opts ...ErrOpts) *userFacingError { + err := &userFacingError{ wrapTarget, message, - tips, + nil, + } + + for _, opt := range opts { + err = opt(err) + } + + return err +} + +func IsUserFacing(err error) bool { + var userFacingError UserFacingError + return errors.As(err, &userFacingError) +} + +func WithTips(tips ...string) ErrOpts { + return func(err *userFacingError) *userFacingError { + err.tips = append(err.tips, tips...) + return err } } diff --git a/internal/locale/errors.go b/internal/locale/errors.go index ae3460afe6..61cde512e5 100644 --- a/internal/locale/errors.go +++ b/internal/locale/errors.go @@ -28,7 +28,7 @@ func (e *LocalizedError) Error() string { } // UserError is the user facing error message, it's the same as Error() but identifies it as being user facing -func (e *LocalizedError) UserError() string { +func (e *LocalizedError) LocalizedError() string { return e.localized } @@ -58,7 +58,7 @@ func (e *LocalizedError) AddTips(tips ...string) { // ErrorLocalizer represents a localized error type ErrorLocalizer interface { error - UserError() string + LocalizedError() string } type AsError interface { @@ -162,7 +162,7 @@ func JoinedErrorMessage(err error) string { var message []string for _, err := range UnpackError(err) { if lerr, isLocaleError := err.(ErrorLocalizer); isLocaleError { - message = append(message, lerr.UserError()) + message = append(message, lerr.LocalizedError()) } } if len(message) == 0 { @@ -177,7 +177,7 @@ func JoinedErrorMessage(err error) string { func ErrorMessage(err error) string { if errr, ok := err.(ErrorLocalizer); ok { - return errr.UserError() + return errr.LocalizedError() } return err.Error() } diff --git a/internal/osutils/user/user.go b/internal/osutils/user/user.go index 44b51d7c4f..56bacb3791 100644 --- a/internal/osutils/user/user.go +++ b/internal/osutils/user/user.go @@ -23,7 +23,7 @@ func (e *HomeDirNotFoundError) Error() string { return homeDirNotFoundErrorMessage } -func (e *HomeDirNotFoundError) UserError() string { +func (e *HomeDirNotFoundError) LocalizedError() string { return homeDirNotFoundErrorMessage } diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index 235fd5efe4..41f43a8064 100644 --- a/internal/runners/hello/hello_example.go +++ b/internal/runners/hello/hello_example.go @@ -76,7 +76,9 @@ func processError(err *error) { *err = errs.WrapUserFacingError( *err, locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory."), - locale.Tl("hello_suggest_checkout", "Try using [ACTIONABLE]`state checkout`[/RESET] first."), + errs.WithTips( + locale.Tl("hello_suggest_checkout", "Try using [ACTIONABLE]`state checkout`[/RESET] first."), + ), ) } } diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index d9d8dc49c6..76755631e3 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -98,6 +98,11 @@ func ParseUserFacing(err error) (int, error) { // unwrap exit code before we remove un-localized wrapped errors from err variable code := errs.ParseExitCode(err) + if errs.IsSilent(err) { + logging.Debug("Suppressing silent failure: %v", err.Error()) + return code, nil + } + // If there is a user facing error in the error stack we want to ensure // that is it forwarded to the user. var userFacingError errs.UserFacingError @@ -106,11 +111,6 @@ func ParseUserFacing(err error) (int, error) { return code, &OutputError{userFacingError} } - if errs.IsSilent(err) { - logging.Debug("Suppressing silent failure: %v", err.Error()) - return code, nil - } - // If the error already has a marshalling function we do not want to wrap // it again in the OutputError type. if hasMarshaller { @@ -167,7 +167,7 @@ func ReportError(err error, cmd *captain.Command, an analytics.Dispatcher) { Error: ptr.To(errorMsg), }) - if !locale.HasError(err) && isErrs && !hasMarshaller { + if (!locale.HasError(err) && !errs.IsUserFacing(err)) && isErrs && !hasMarshaller { multilog.Error("MUST ADDRESS: Error does not have localization: %s", errs.JoinMessage(err)) // If this wasn't built via CI then this is a dev workstation, and we should be more aggressive diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index 8c7f64b137..250c9284ad 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -106,7 +106,7 @@ func (e *BuildPlannerError) InputError() bool { // UserError returns the error message to be displayed to the user. // This function is added so that BuildPlannerErrors will be displayed // to the user -func (e *BuildPlannerError) UserError() string { +func (e *BuildPlannerError) LocalizedError() string { return e.Error() } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 15b85b36f6..21686ad405 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -93,7 +93,7 @@ func (a *ArtifactSetupErrors) Errors() []error { } // UserError returns a message including all user-facing sub-error messages -func (a *ArtifactSetupErrors) UserError() string { +func (a *ArtifactSetupErrors) LocalizedError() string { var errStrings []string for _, err := range a.errs { errStrings = append(errStrings, locale.JoinedErrorMessage(err)) From 59e441095ee69f0c3ed0fedfae03af03cd5d4d00 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Sep 2023 15:24:21 -0400 Subject: [PATCH 40/45] Attempt to trap "flisten in use" errors on CI for further debugging. --- internal/testhelpers/e2e/session.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index 663647bd24..801272150a 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -614,13 +614,18 @@ func (s *Session) DebugLogs() string { return result } +var errorOrPanicRegex = regexp.MustCompile(`(?:\[ERR:|Panic:)`) +var flistenInUseRegex = regexp.MustCompile(`flisten in use`) + func (s *Session) DetectLogErrors() { - rx := regexp.MustCompile(`(?:\[ERR:|Panic:)`) for _, path := range s.LogFiles() { - if contents := string(fileutils.ReadFileUnsafe(path)); rx.MatchString(contents) { + if contents := string(fileutils.ReadFileUnsafe(path)); errorOrPanicRegex.MatchString(contents) { s.t.Errorf("Found error and/or panic in log file %s, contents:\n%s", path, contents) } } + if contents := s.SvcLog(); flistenInUseRegex.MatchString(contents) { + s.t.Errorf("Found flisten in use error in state-svc log file, contents:\n%s", contents) + } } func RunningOnCI() bool { From 789635d587483224ac69fa685e18456473aabdbb Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 15 Sep 2023 15:08:30 -0700 Subject: [PATCH 41/45] OutputError recognizes user facing errors --- pkg/cmdlets/errors/errors.go | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index 76755631e3..9db6da7a6c 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -39,18 +39,23 @@ func (o *OutputError) MarshalOutput(f output.Format) interface{} { outLines = append(outLines, output.Title(locale.Tl("err_what_happened", "[ERROR]Something Went Wrong[/RESET]")).String()) } - rerrs := locale.UnpackError(o.error) - if len(rerrs) == 0 { - // It's possible the error came from cobra or something else low level that doesn't use localization - logging.Warning("Error does not have localization: %s", errs.JoinMessage(o.error)) - rerrs = []error{o.error} - } - for _, errv := range rerrs { - message := trimError(locale.ErrorMessage(errv)) - if f == output.PlainFormatName { - outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", message)) - } else { - outLines = append(outLines, message) + var userFacingError errs.UserFacingError + if errors.As(o.error, &userFacingError) { + outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", userFacingError.UserError())) + } else { + rerrs := locale.UnpackError(o.error) + if len(rerrs) == 0 { + // It's possible the error came from cobra or something else low level that doesn't use localization + logging.Warning("Error does not have localization: %s", errs.JoinMessage(o.error)) + rerrs = []error{o.error} + } + for _, errv := range rerrs { + message := trimError(locale.ErrorMessage(errv)) + if f == output.PlainFormatName { + outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", message)) + } else { + outLines = append(outLines, message) + } } } From 3c674a1d2c294b9cb7a68889902e823a0d91b00c Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 15 Sep 2023 15:09:49 -0700 Subject: [PATCH 42/45] Rename type --- internal/errs/userfacing.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/errs/userfacing.go b/internal/errs/userfacing.go index 88b032d181..5ae0df22dc 100644 --- a/internal/errs/userfacing.go +++ b/internal/errs/userfacing.go @@ -7,7 +7,7 @@ type UserFacingError interface { UserError() string } -type ErrOpts func(err *userFacingError) *userFacingError +type ErrOpt func(err *userFacingError) *userFacingError type userFacingError struct { wrapped error @@ -31,7 +31,7 @@ func NewUserFacingError(message string, tips ...string) *userFacingError { return WrapUserFacingError(nil, message) } -func WrapUserFacingError(wrapTarget error, message string, opts ...ErrOpts) *userFacingError { +func WrapUserFacingError(wrapTarget error, message string, opts ...ErrOpt) *userFacingError { err := &userFacingError{ wrapTarget, message, @@ -50,7 +50,7 @@ func IsUserFacing(err error) bool { return errors.As(err, &userFacingError) } -func WithTips(tips ...string) ErrOpts { +func WithTips(tips ...string) ErrOpt { return func(err *userFacingError) *userFacingError { err.tips = append(err.tips, tips...) return err From 1724df18e0754d8ed080c8d3fabb0d3c1665d986 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 15 Sep 2023 15:48:25 -0700 Subject: [PATCH 43/45] Add formatter conditional --- pkg/cmdlets/errors/errors.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index 9db6da7a6c..7c2691de79 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/pkg/cmdlets/errors/errors.go @@ -41,7 +41,12 @@ func (o *OutputError) MarshalOutput(f output.Format) interface{} { var userFacingError errs.UserFacingError if errors.As(o.error, &userFacingError) { - outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", userFacingError.UserError())) + message := userFacingError.UserError() + if f == output.PlainFormatName { + outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", message)) + } else { + outLines = append(outLines, message) + } } else { rerrs := locale.UnpackError(o.error) if len(rerrs) == 0 { From 51355d023ab72a5d3da91b7ca19b320f81be3153 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 18 Sep 2023 11:24:48 -0400 Subject: [PATCH 44/45] Move trap in previous commit to session.Close() method. This is so the check happens after every session finishes. --- internal/testhelpers/e2e/session.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index 801272150a..76529b72d8 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -526,6 +526,10 @@ func (s *Session) Close() error { } } + // Trap "flisten in use" errors to help debug DX-2090. + svcLogContents := s.SvcLog() + require.NotContains(s.t, svcLogContents, "flisten in use", "Found flisten in use error in state-svc log file, contents:\n%s", svcLogContents) + return nil } @@ -615,7 +619,6 @@ func (s *Session) DebugLogs() string { } var errorOrPanicRegex = regexp.MustCompile(`(?:\[ERR:|Panic:)`) -var flistenInUseRegex = regexp.MustCompile(`flisten in use`) func (s *Session) DetectLogErrors() { for _, path := range s.LogFiles() { @@ -623,9 +626,6 @@ func (s *Session) DetectLogErrors() { s.t.Errorf("Found error and/or panic in log file %s, contents:\n%s", path, contents) } } - if contents := s.SvcLog(); flistenInUseRegex.MatchString(contents) { - s.t.Errorf("Found flisten in use error in state-svc log file, contents:\n%s", contents) - } } func RunningOnCI() bool { From 2144f5f98b48da58ae0a16f4851669d204ed953e Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 18 Sep 2023 12:55:22 -0400 Subject: [PATCH 45/45] If the previous trap caught something, print all the logs. --- internal/testhelpers/e2e/session.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/testhelpers/e2e/session.go b/internal/testhelpers/e2e/session.go index 76529b72d8..711feb8649 100644 --- a/internal/testhelpers/e2e/session.go +++ b/internal/testhelpers/e2e/session.go @@ -527,8 +527,9 @@ func (s *Session) Close() error { } // Trap "flisten in use" errors to help debug DX-2090. - svcLogContents := s.SvcLog() - require.NotContains(s.t, svcLogContents, "flisten in use", "Found flisten in use error in state-svc log file, contents:\n%s", svcLogContents) + if contents := s.SvcLog(); strings.Contains(contents, "flisten in use") { + s.t.Fatal(s.DebugMessage("Found 'flisten in use' error in state-svc log file")) + } return nil }