From beb7fde12f787d05cc80c69a69722b9c32e86e18 Mon Sep 17 00:00:00 2001 From: Prem Kumar Kalle Date: Thu, 18 Jan 2024 16:06:14 -0800 Subject: [PATCH] Clear all the local metrics data when user opt out of CEIP - If user opt out of CEIP CLI would clear all the local metrics data and will not store the metrics. Signed-off-by: Prem Kumar Kalle --- pkg/telemetry/client.go | 10 +++++ pkg/telemetry/client_test.go | 60 ++++++++++++++++++++++++++++-- pkg/telemetry/metric_db.go | 3 ++ pkg/telemetry/sqlite_metrics_db.go | 25 +++++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/pkg/telemetry/client.go b/pkg/telemetry/client.go index 678460de3..ecf7f0628 100644 --- a/pkg/telemetry/client.go +++ b/pkg/telemetry/client.go @@ -139,6 +139,16 @@ func (tc *telemetryClient) SaveMetrics() error { return errors.Wrap(err, "unable to create the telemetry schema") } + // If user opts-out from CEIP, clear the metrics data from the local database + // TODO(prkalle): Once the DB is emptied, the subsequent calls will keep trying to delete the rows from the empty DB. + // So this need to be optimized. One possible future enhancement/optimization could be to let the telemetry plugin to empty + // the DB as soon as the user updates the opt-out choice and CLI would just ignore saving the metrics if user opts-out. + ceipOptInConfigVal, _ := configlib.GetCEIPOptIn() + optIn, _ := strconv.ParseBool(ceipOptInConfigVal) + if !optIn { + return tc.metricsDB.ClearMetricData() + } + return tc.metricsDB.SaveOperationMetric(tc.currentOperationMetrics) } diff --git a/pkg/telemetry/client_test.go b/pkg/telemetry/client_test.go index d36a4c096..4a406a207 100644 --- a/pkg/telemetry/client_test.go +++ b/pkg/telemetry/client_test.go @@ -40,6 +40,8 @@ type mockMetricsDB struct { saveOperationMetricReturnError error getRowCountError error getRowCountReturnVal int + clearMetricDataCalled bool + clearMetricDataReturnError error } func (mc *mockMetricsDB) CreateSchema() error { @@ -55,6 +57,10 @@ func (mc *mockMetricsDB) GetRowCount() (int, error) { mc.getRowCountCalled = true return mc.getRowCountReturnVal, mc.getRowCountError } +func (mc *mockMetricsDB) ClearMetricData() error { + mc.clearMetricDataCalled = true + return mc.clearMetricDataReturnError +} var _ = Describe("Unit tests for UpdateCmdPreRunMetrics()", func() { const True = "true" @@ -517,9 +523,11 @@ func TestTelemetryClient_UpdateCmdPostRunMetrics(t *testing.T) { var _ = Describe("Unit tests for SaveMetrics()", func() { var ( - tc *telemetryClient - metricsDB *mockMetricsDB - err error + tc *telemetryClient + metricsDB *mockMetricsDB + configFile *os.File + configFileNG *os.File + err error ) BeforeEach(func() { metricsDB = &mockMetricsDB{} @@ -529,6 +537,24 @@ var _ = Describe("Unit tests for SaveMetrics()", func() { }, metricsDB: metricsDB, } + configFile, err = os.CreateTemp("", "config") + Expect(err).To(BeNil()) + os.Setenv("TANZU_CONFIG", configFile.Name()) + + configFileNG, err = os.CreateTemp("", "config_ng") + Expect(err).To(BeNil()) + os.Setenv("TANZU_CONFIG_NEXT_GEN", configFileNG.Name()) + + err = configlib.SetCEIPOptIn("true") + Expect(err).ToNot(HaveOccurred(), "failed to set the CEIP OptIn") + }) + AfterEach(func() { + os.Unsetenv("TANZU_CONFIG") + os.Unsetenv("TANZU_CONFIG_NEXT_GEN") + + os.RemoveAll(configFile.Name()) + os.RemoveAll(configFileNG.Name()) + }) Context("when the start time is zero", func() { @@ -552,6 +578,34 @@ var _ = Describe("Unit tests for SaveMetrics()", func() { Expect(metricsDB.saveOperationMetricCalled).To(BeFalse()) }) }) + Context("when the user opt-out of CEIP", func() { + It("should not return error and should delete the existing metric data in the DB", func() { + tc.currentOperationMetrics.StartTime = time.Now() + err = configlib.SetCEIPOptIn("false") + Expect(err).ToNot(HaveOccurred(), "failed to set the CEIP OptOut") + + err = tc.SaveMetrics() + Expect(err).ToNot(HaveOccurred()) + Expect(metricsDB.createSchemaCalled).To(BeTrue()) + Expect(metricsDB.clearMetricDataCalled).To(BeTrue()) + Expect(metricsDB.saveOperationMetricCalled).To(BeFalse()) + }) + }) + Context("when the user opt-out of CEIP and when deleting the existing metric data failed", func() { + It("should return error", func() { + tc.currentOperationMetrics.StartTime = time.Now() + err = configlib.SetCEIPOptIn("false") + Expect(err).ToNot(HaveOccurred(), "failed to set the CEIP OptOut") + metricsDB.clearMetricDataReturnError = errors.New("fake clear metrics error") + + err = tc.SaveMetrics() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("fake clear metrics error")) + Expect(metricsDB.createSchemaCalled).To(BeTrue()) + Expect(metricsDB.clearMetricDataCalled).To(BeTrue()) + Expect(metricsDB.saveOperationMetricCalled).To(BeFalse()) + }) + }) Context("when DB returns error to save the metrics", func() { It("should return failure", func() { tc.currentOperationMetrics.StartTime = time.Now() diff --git a/pkg/telemetry/metric_db.go b/pkg/telemetry/metric_db.go index 91fdc37ab..6084df998 100644 --- a/pkg/telemetry/metric_db.go +++ b/pkg/telemetry/metric_db.go @@ -13,4 +13,7 @@ type MetricsDB interface { // GetRowCount gets metrics table current row count GetRowCount() (int, error) + + // ClearMetricData clears all the CLI operation metrics collected in the database + ClearMetricData() error } diff --git a/pkg/telemetry/sqlite_metrics_db.go b/pkg/telemetry/sqlite_metrics_db.go index 88b23ac37..eebaa01a0 100644 --- a/pkg/telemetry/sqlite_metrics_db.go +++ b/pkg/telemetry/sqlite_metrics_db.go @@ -33,6 +33,9 @@ const ( // cliOperationMetricRowClause is the SELECT section of the SQL query to be used when querying the Metric DB row count. cliOperationMetricRowClause = "SELECT count(*) FROM tanzu_cli_operations" + + // cliOperationMetricClearAllDataClause is the SQL query to be used to clear all the metrics data collected so far. + cliOperationMetricClearAllDataClause = "DELETE FROM tanzu_cli_operations" ) // Structure of each row of the PluginBinaries table within the SQLite database @@ -160,6 +163,28 @@ func (b *sqliteMetricsDB) GetRowCount() (int, error) { } return count, err } + +func (b *sqliteMetricsDB) ClearMetricData() error { + err := AcquireTanzuMetricDBLock() + if err != nil { + return err + } + defer ReleaseTanzuMetricDBLock() + db, err := sql.Open("sqlite", b.metricsDBFile) + if err != nil { + return errors.Wrapf(err, "failed to open the DB from '%s' file", b.metricsDBFile) + } + defer db.Close() + + dbQuery := cliOperationMetricClearAllDataClause + _, err = db.Exec(dbQuery) + + if err != nil { + return errors.Wrapf(err, "failed to execute the DB query : %v", dbQuery) + } + return err +} + func isDBRowCountThresholdReached(db *sql.DB) (bool, error) { dbQuery := cliOperationMetricRowClause rows, err := db.Query(dbQuery) //nolint:rowserrcheck // rows.Err must be checked (rowserrcheck)