Added Usage metrics to support bundle tool.

Neejor Chakma · copybara-github · commit 8cf166355d6e · 2025-05-26T23:04:02.000-07:00
PiperOrigin-RevId: 763637841
diff --git a/internal/onetime/supportbundle/supportbundle.go b/internal/onetime/supportbundle/supportbundle.go
@@ -360,7 +360,7 @@ func (*SupportBundle) Synopsis() string {
 // Usage implements the subcommand interface for support bundle report collection for support team.
 func (*SupportBundle) Usage() string {
 	return `Usage: supportbundle [-sid=<SAP System Identifier>] [-instance-numbers=<Instance numbers>]
-	[-hostname=<Hostname>] [agent-logs-only=true|false] [-metrics] [-metrics]
+	[-hostname=<Hostname>] [agent-logs-only=true|false] [-pacemaker-diagnosis=true|false] [-metrics=true|false]
 	[-timestamp=<YYYY-MM-DD HH:MM:SS>] [-before-duration=<duration in seconds>] [-after-duration=<duration in seconds>]
 	[-h] [-loglevel=<debug|info|warn|error>]
 	[-result-bucket=<name of the result bucket where bundle zip is uploaded>] [-log-path=<log-path>]
@@ -433,6 +433,7 @@ func (s *SupportBundle) supportBundleHandler(ctx context.Context, destFilePathPr
 		s.oteLogger.LogErrorToFileAndConsole(ctx, "Invalid params for collecting support bundle Report for Agent for SAP", errors.New(errMessage))
 		return fmt.Sprintf("Invalid params for collecting support bundle Report for Agent for SAP: %s", errMessage), subcommands.ExitUsageError
 	}
+	s.oteLogger.LogUsageAction(usagemetrics.SupportBundle)
 	s.Sid = strings.ToUpper(s.Sid)
 	bundlename := fmt.Sprintf("supportbundle-%s-%s", s.Hostname, strings.Replace(time.Now().Format(time.RFC3339), ":", "-", -1))
 	destFilesPath := fmt.Sprintf("%s%s", destFilePathPrefix, bundlename)
@@ -535,10 +536,12 @@ func (s *SupportBundle) supportBundleHandler(ctx context.Context, destFilePathPr
 	}
 
 	if s.ResultBucket != "" {
+		s.oteLogger.LogUsageAction(usagemetrics.SupportBundleUploadStarted)
 		if err := s.uploadZip(ctx, zipfile, bundlename, storage.ConnectToBucket, getReadWriter, fs, st.NewClient); err != nil {
 			errMessage := fmt.Sprintf("Error while uploading zip file %s to bucket %s", destFilePathPrefix+".zip", s.ResultBucket)
 			s.oteLogger.LogMessageToConsole(fmt.Sprintf(errMessage, " Error: ", err))
 			failureMsgs = append(failureMsgs, errMessage)
+			s.oteLogger.LogUsageError(usagemetrics.SupportBundleUploadFailure)
 		} else {
 			msg := fmt.Sprintf("Bundle uploaded to bucket %s, path: %s", s.ResultBucket, fmt.Sprintf("gs://%s/%s/%s.zip", s.ResultBucket, s.Name(), bundlename))
 			successMsgs = append(successMsgs, msg)
@@ -549,6 +552,8 @@ func (s *SupportBundle) supportBundleHandler(ctx context.Context, destFilePathPr
 				failureMsgs = append(failureMsgs, errMessage)
 			}
 		}
+	} else {
+		s.oteLogger.LogUsageAction(usagemetrics.SupportBundleLocalCollection)
 	}
 
 	// Rotate out old support bundles so we don't fill the file system.
@@ -1437,7 +1442,7 @@ func (s *SupportBundle) getMetricsClients(ctx context.Context) error {
 
 	qc, err := s.createQueryClient(ctx)
 	if err != nil {
-		usagemetrics.Error(usagemetrics.QueryClientCreateFailure)
+		s.oteLogger.LogUsageError(usagemetrics.QueryClientCreateFailure)
 		return err
 	}
 	cmr := &cloudmetricreader.CloudMetricReader{
@@ -1447,7 +1452,7 @@ func (s *SupportBundle) getMetricsClients(ctx context.Context) error {
 
 	mmc, err := s.createMetricClient(ctx)
 	if err != nil {
-		usagemetrics.Error(usagemetrics.MetricClientCreateFailure)
+		s.oteLogger.LogUsageError(usagemetrics.MetricClientCreateFailure)
 		return err
 	}
 
@@ -1578,21 +1583,21 @@ func (s *SupportBundle) fetchLabelDescriptors(ctx context.Context, cp *ipb.Cloud
 }
 
 // newQueryClient abstracts the creation of a new Cloud Monitoring query client for testing purposes.
-func newQueryClient(ctx context.Context) (cloudmonitoring.TimeSeriesQuerier, error) {
+func (s *SupportBundle) newQueryClient(ctx context.Context) (cloudmonitoring.TimeSeriesQuerier, error) {
 	mqc, err := monitoring.NewQueryClient(ctx)
 	if err != nil {
-		usagemetrics.Error(usagemetrics.QueryClientCreateFailure)
+		s.oteLogger.LogUsageError(usagemetrics.QueryClientCreateFailure)
 		return nil, fmt.Errorf("failed to create Cloud Monitoring query client: %v", err)
 	}
 
 	return &cloudmetricreader.QueryClient{Client: mqc}, nil
 }
 
 // newMetricClient abstracts the creation of a new Cloud Monitoring metric client for testing purposes.
-func newMetricClient(ctx context.Context) (cloudmonitoring.TimeSeriesDescriptorQuerier, error) {
+func (s *SupportBundle) newMetricClient(ctx context.Context) (cloudmonitoring.TimeSeriesDescriptorQuerier, error) {
 	mmc, err := monitoring.NewMetricClient(ctx)
 	if err != nil {
-		usagemetrics.Error(usagemetrics.MetricClientCreateFailure)
+		s.oteLogger.LogUsageError(usagemetrics.MetricClientCreateFailure)
 		return nil, fmt.Errorf("failed to create Cloud Monitoring metric client: %v", err)
 	}
 
@@ -1627,8 +1632,8 @@ func (s *SupportBundle) validateParams() []string {
 	s.rest = &rest.Rest{}
 	s.rest.NewRest()
 
-	s.createQueryClient = newQueryClient
-	s.createMetricClient = newMetricClient
+	s.createQueryClient = s.newQueryClient
+	s.createMetricClient = s.newMetricClient
 
 	return errs
 }
diff --git a/internal/onetime/supportbundle/supportbundle_test.go b/internal/onetime/supportbundle/supportbundle_test.go
@@ -2939,6 +2939,7 @@ func TestGetProcessMetricsClients(t *testing.T) {
 	ctx := context.Background()
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
+			tc.s.oteLogger = defaultOTELogger
 			err := tc.s.getMetricsClients(ctx)
 			if !cmp.Equal(err, tc.wantErr, cmpopts.EquateErrors()) {
 				t.Errorf("getProcessMetricsClients() returned an unexpected error: %v", err)
@@ -3580,7 +3581,10 @@ func TestNewQueryClient(t *testing.T) {
 	ctx := context.Background()
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := newQueryClient(ctx)
+			var s = &SupportBundle{
+				oteLogger: defaultOTELogger,
+			}
+			_, err := s.newQueryClient(ctx)
 			if diff := cmp.Diff(err, tc.wantErr, cmpopts.EquateErrors()); diff != "" {
 				t.Errorf("NewQueryClient() returned an unexpected error: %v", diff)
 			}
@@ -3602,7 +3606,10 @@ func TestNewMetricClient(t *testing.T) {
 	ctx := context.Background()
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := newMetricClient(ctx)
+			s := &SupportBundle{
+				oteLogger: defaultOTELogger,
+			}
+			_, err := s.newMetricClient(ctx)
 			if diff := cmp.Diff(err, tc.wantErr, cmpopts.EquateErrors()); diff != "" {
 				t.Errorf("NewMetricClient() returned an unexpected error: %v", diff)
 			}
diff --git a/internal/usagemetrics/usagemetrics.go b/internal/usagemetrics/usagemetrics.go
@@ -124,6 +124,7 @@ const (
 	HANAChangeDiskTypeSnapshotFailure              = 83 //	HANAChangeDiskTypeSnapshotFailure
 	HANAChangeDiskTypeRestoreFailure               = 84 //	HANAChangeDiskTypeRestoreFailure
 	RemoteValidationOTEFailure                     = 85 //	RemoteValidationFailure
+	SupportBundleUploadFailure                     = 86 //	SupportBundleUploadFailure
 )
 
 // Agent wide action mappings - Only append the action codes at the end of the list.
@@ -219,6 +220,9 @@ const (
 	HANAChangeDiskTypeFinished              = 86 //	HANAChangeDiskTypeFinished
 	RemoteValidationOTEStarted              = 87 //	RemoteValidationOTEStarted
 	RemoteValidationOTEFinished             = 88 //	RemoteValidationOTEFinished
+	SupportBundle                           = 89 //	SupportBundle
+	SupportBundleUploadStarted              = 90 //	SupportBundleUploadStarted
+	SupportBundleLocalCollection            = 91 //	SupportBundleLocalCollection
 )
 
 // projectNumbers contains known project numbers for test instances.
diff --git a/internal/usagemetrics/usagemetrics_test.go b/internal/usagemetrics/usagemetrics_test.go
@@ -277,6 +277,9 @@ func TestErrorConstants(t *testing.T) {
 	if RemoteValidationOTEFailure != 85 {
 		t.Errorf("RemoteValidationOTEFailure = %v, want 85", RemoteValidationOTEFailure)
 	}
+	if SupportBundleUploadFailure != 86 {
+		t.Errorf("PacemakerDiagnosisFailure = %v, want 83", SupportBundleUploadFailure)
+	}
 }
 
 func TestActionConstants(t *testing.T) {
@@ -547,4 +550,13 @@ func TestActionConstants(t *testing.T) {
 	if RemoteValidationOTEFinished != 88 {
 		t.Errorf("RemoteValidationOTEFinished = %v, want 88", RemoteValidationOTEFinished)
 	}
+	if SupportBundle != 89 {
+		t.Errorf("SupportBundle = %v, want 85", SupportBundle)
+	}
+	if SupportBundleUploadStarted != 90 {
+		t.Errorf("SupportBundleUploadStarted = %v, want 87", SupportBundleUploadStarted)
+	}
+	if SupportBundleLocalCollection != 91 {
+		t.Errorf("SupportBundleLocalCollection = %v, want 88", SupportBundleLocalCollection)
+	}
 }

Original file line number	Diff line number	Diff line change
`@@ -277,6 +277,9 @@ func TestErrorConstants(t *testing.T) {`
`277`	`277`	`if RemoteValidationOTEFailure != 85 {`
`278`	`278`	`t.Errorf("RemoteValidationOTEFailure = %v, want 85", RemoteValidationOTEFailure)`
`279`	`279`	`}`
	`280`	`+ if SupportBundleUploadFailure != 86 {`
	`281`	`+ t.Errorf("PacemakerDiagnosisFailure = %v, want 83", SupportBundleUploadFailure)`
	`282`	`+ }`
`280`	`283`	`}`
`281`	`284`
`282`	`285`	`func TestActionConstants(t *testing.T) {`
`@@ -547,4 +550,13 @@ func TestActionConstants(t *testing.T) {`
`547`	`550`	`if RemoteValidationOTEFinished != 88 {`
`548`	`551`	`t.Errorf("RemoteValidationOTEFinished = %v, want 88", RemoteValidationOTEFinished)`
`549`	`552`	`}`
	`553`	`+ if SupportBundle != 89 {`
	`554`	`+ t.Errorf("SupportBundle = %v, want 85", SupportBundle)`
	`555`	`+ }`
	`556`	`+ if SupportBundleUploadStarted != 90 {`
	`557`	`+ t.Errorf("SupportBundleUploadStarted = %v, want 87", SupportBundleUploadStarted)`
	`558`	`+ }`
	`559`	`+ if SupportBundleLocalCollection != 91 {`
	`560`	`+ t.Errorf("SupportBundleLocalCollection = %v, want 88", SupportBundleLocalCollection)`
	`561`	`+ }`
`550`	`562`	`}`