Track preference options usage (#6843)

* Add integration test highlighting the expectations * Record parameter (and value) set or unset using the 'odo preference set/unset' commands By default, values are anonymized. Only parameters explicitly declared list will be recorded verbatim. This list is currently empty, but that might change later on. * Mark all current preference parameters except ImageRegistry as clear-text Co-authored-by: Philippe Martin <[email protected]> --------- Co-authored-by: Philippe Martin <[email protected]>
redhat-developer · May 30, 2023 · a79c953 · a79c953
1 parent 78c9bed
commit a79c953
Show file tree

Hide file tree

Showing 5 changed files with 197 additions and 41 deletions.
diff --git a/pkg/odo/cli/preference/set.go b/pkg/odo/cli/preference/set.go
@@ -8,6 +8,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/odo/cmdline"
 	"github.com/redhat-developer/odo/pkg/odo/util"
+	scontext "github.com/redhat-developer/odo/pkg/segment/context"
 
 	"github.com/redhat-developer/odo/pkg/odo/cli/ui"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
@@ -83,6 +84,8 @@ func (o *SetOptions) Run(ctx context.Context) (err error) {
 	}
 
 	log.Successf("Value of '%s' preference was set to '%s'", o.paramName, o.paramValue)
+
+	scontext.SetPreferenceParameter(ctx, o.paramName, &o.paramValue)
 	return nil
 }
 

diff --git a/pkg/odo/cli/preference/unset.go b/pkg/odo/cli/preference/unset.go
@@ -10,6 +10,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/odo/cmdline"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
+	scontext "github.com/redhat-developer/odo/pkg/segment/context"
 
 	"github.com/spf13/cobra"
 	ktemplates "k8s.io/kubectl/pkg/util/templates"
@@ -83,6 +84,8 @@ func (o *UnsetOptions) Run(ctx context.Context) (err error) {
 	}
 
 	log.Successf("Value of '%s' preference was removed from preferences. Its default value will be used.", o.paramName)
+
+	scontext.SetPreferenceParameter(ctx, o.paramName, nil)
 	return nil
 
 }

diff --git a/pkg/segment/context/context.go b/pkg/segment/context/context.go
@@ -3,7 +3,9 @@ package context
 import (
 	"context"
 	"fmt"
+	"hash/adler32"
 	"os"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -34,6 +36,8 @@ const (
 	Flags                   = "flags"
 	Platform                = "platform"
 	PlatformVersion         = "platformVersion"
+	PreferenceParameter     = "parameter"
+	PreferenceValue         = "value"
 )
 
 const (
@@ -42,6 +46,16 @@ const (
 	JBoss    = "jboss"
 )
 
+// Add the (case-insensitive) preference parameter name here to have the corresponding value sent verbatim to telemetry.
+var clearTextPreferenceParams = []string{
+	"ConsentTelemetry",
+	"Ephemeral",
+	"PushTimeout",
+	"RegistryCacheTime",
+	"Timeout",
+	"UpdateNotification",
+}
+
 type contextKey struct{}
 
 var key = contextKey{}
@@ -228,6 +242,34 @@ func SetCaller(ctx context.Context, caller string) error {
 	return err
 }
 
+// SetPreferenceParameter tracks the preferences options usage, by recording both the parameter name and value.
+// By default, values are anonymized. Only parameters explicitly declared in the 'clearTextPreferenceParams' list will be recorded verbatim.
+// Setting value to nil means that the parameter has been unset in the preferences; so the value will not be recorded.
+func SetPreferenceParameter(ctx context.Context, param string, value *string) {
+	setContextProperty(ctx, PreferenceParameter, param)
+
+	if value == nil {
+		return
+	}
+
+	isClearTextParam := func() bool {
+		for _, clearTextParam := range clearTextPreferenceParams {
+			if strings.EqualFold(param, clearTextParam) {
+				return true
+			}
+		}
+		return false
+	}
+
+	recordedValue := *value
+	if !isClearTextParam() {
+		// adler32 for fast (and short) checksum computation, while minimizing the probability of collisions (which are not that important here).
+		// We just want to make sure that the same value returns the same anonymized string, while making it hard to guess the original string.
+		recordedValue = strconv.FormatUint(uint64(adler32.Checksum([]byte(recordedValue))), 16)
+	}
+	setContextProperty(ctx, PreferenceValue, recordedValue)
+}
+
 // GetPreviousTelemetryStatus gets the telemetry status that was seen before a command is run
 func GetPreviousTelemetryStatus(ctx context.Context) bool {
 	wasEnabled, ok := GetContextProperties(ctx)[PreviousTelemetryStatus]

diff --git a/tests/helper/helper_telemetry.go b/tests/helper/helper_telemetry.go
@@ -47,6 +47,11 @@ func GetDebugTelemetryFile() string {
 	return os.Getenv(DebugTelemetryFileEnv)
 }
 
+func ClearTelemetryFile() {
+	err := os.Truncate(GetDebugTelemetryFile(), 0)
+	Expect(err).ShouldNot(HaveOccurred())
+}
+
 // GetTelemetryDebugData gets telemetry data dumped into temp file for testing/debugging
 func GetTelemetryDebugData() segment.TelemetryData {
 	var data []byte

diff --git a/tests/integration/cmd_pref_config_test.go b/tests/integration/cmd_pref_config_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -204,64 +205,166 @@ OdoSettings:
 				})
 			})
 
-			When("Telemetry is enabled in preferences", func() {
+			When("telemetry is enabled", func() {
+				var prefClient preference.Client
+
 				BeforeEach(func() {
-					prefClient := helper.EnableTelemetryDebug()
-					err := prefClient.SetConfiguration(preference.ConsentTelemetrySetting, "true")
-					Expect(err).ShouldNot(HaveOccurred())
-					Expect(os.Unsetenv(segment.TrackingConsentEnv)).NotTo(HaveOccurred())
+					prefClient = helper.EnableTelemetryDebug()
 				})
 
 				AfterEach(func() {
 					helper.ResetTelemetry()
 				})
 
-				When("setting ConsentTelemetry to false", func() {
-					BeforeEach(func() {
-						helper.Cmd("odo", "preference", "set", "ConsentTelemetry", "false", "--force").ShouldPass()
+				for _, tt := range []struct {
+					prefParam                  string
+					value                      string
+					differentValueIfAnonymized string
+					clearText                  bool
+				}{
+					{"ConsentTelemetry", "true", "", true},
+					{"Ephemeral", "false", "", true},
+					{"UpdateNotification", "true", "", true},
+					{"PushTimeout", "1m", "", true},
+					{"RegistryCacheTime", "2s", "", true},
+					{"Timeout", "30s", "", true},
+					{"ImageRegistry", "quay.io/some-org", "ghcr.io/my-org", false},
+				} {
+					tt := tt
+					form := "hashed"
+					if tt.clearText {
+						form = "plain"
+					}
+
+					When("unsetting value for preference "+tt.prefParam, func() {
+						BeforeEach(func() {
+							helper.Cmd("odo", "preference", "unset", tt.prefParam, "--force").ShouldPass()
+						})
+
+						It("should track parameter that is unset without any value", func() {
+							helper.Cmd("odo", "preference", "unset", tt.prefParam, "--force").ShouldPass()
+							td := helper.GetTelemetryDebugData()
+							Expect(td.Event).To(ContainSubstring("odo preference unset"))
+							Expect(td.Properties.Success).To(BeTrue())
+							Expect(td.Properties.Error).To(BeEmpty())
+							Expect(td.Properties.ErrorType).To(BeEmpty())
+							Expect(td.Properties.CmdProperties[segmentContext.Flags]).To(Equal("force"))
+							Expect(td.Properties.CmdProperties[segmentContext.PreferenceParameter]).To(Equal(strings.ToLower(tt.prefParam)))
+							valueRecorded, present := td.Properties.CmdProperties[segmentContext.PreferenceValue]
+							Expect(present).To(BeFalse(), fmt.Sprintf("no value should be recorded for preference %q, fot %q", tt.prefParam, valueRecorded))
+						})
 					})
 
-					// https://github.com/redhat-developer/odo/issues/6790
-					It("should record the odo-preference-set command in telemetry", func() {
-						td := helper.GetTelemetryDebugData()
-						Expect(td.Event).To(ContainSubstring("odo preference set"))
-						Expect(td.Properties.Success).To(BeTrue())
-						Expect(td.Properties.Error).To(BeEmpty())
-						Expect(td.Properties.ErrorType).To(BeEmpty())
-						Expect(td.Properties.CmdProperties[segmentContext.Flags]).To(Equal("force"))
-						Expect(td.Properties.CmdProperties[segmentContext.PreviousTelemetryStatus]).To(BeTrue())
-						Expect(td.Properties.CmdProperties[segmentContext.TelemetryStatus]).To(BeFalse())
+					When("setting value for preference "+tt.prefParam, func() {
+						BeforeEach(func() {
+							if !tt.clearText {
+								Expect(tt.differentValueIfAnonymized).ShouldNot(Equal(tt.value),
+									"test not written as expected. Values should be different for preference parameters declared as anonymized.")
+							}
+							helper.Cmd("odo", "preference", "set", tt.prefParam, tt.value, "--force").ShouldPass()
+						})
+
+						It(fmt.Sprintf("should track parameter that is set along with its %s value", form), func() {
+							td := helper.GetTelemetryDebugData()
+							Expect(td.Event).To(ContainSubstring("odo preference set"))
+							Expect(td.Properties.Success).To(BeTrue())
+							Expect(td.Properties.Error).To(BeEmpty())
+							Expect(td.Properties.ErrorType).To(BeEmpty())
+							Expect(td.Properties.CmdProperties[segmentContext.Flags]).To(Equal("force"))
+							Expect(td.Properties.CmdProperties[segmentContext.PreferenceParameter]).To(Equal(strings.ToLower(tt.prefParam)))
+							Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).ShouldNot(BeEmpty())
+							if tt.clearText {
+								Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).Should(Equal(tt.value))
+							} else {
+								Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).ShouldNot(Equal(tt.value))
+							}
+						})
+
+						if !tt.clearText {
+							It("should anonymize values set such that same strings have same hash", func() {
+								td := helper.GetTelemetryDebugData()
+								Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).ShouldNot(BeEmpty())
+								pref1Val, ok := td.Properties.CmdProperties[segmentContext.PreferenceValue].(string)
+								Expect(ok).To(BeTrue(), fmt.Sprintf("value recorded in telemetry for preference %q is expected to be a string", tt.prefParam))
+
+								helper.ClearTelemetryFile()
+
+								helper.Cmd("odo", "preference", "set", tt.prefParam, tt.value, "--force").ShouldPass()
+								td = helper.GetTelemetryDebugData()
+								Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).ShouldNot(BeEmpty())
+								pref2Val, ok := td.Properties.CmdProperties[segmentContext.PreferenceValue].(string)
+								Expect(ok).To(BeTrue(), fmt.Sprintf("value recorded in telemetry for preference %q is expected to be a string", tt.prefParam))
+
+								Expect(pref1Val).To(Equal(pref2Val))
+							})
+
+							It("should anonymize values set such that different strings will not have same hash", func() {
+								td := helper.GetTelemetryDebugData()
+								Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).ShouldNot(BeEmpty())
+								pref1Val, ok := td.Properties.CmdProperties[segmentContext.PreferenceValue].(string)
+								Expect(ok).To(BeTrue(), fmt.Sprintf("value recorded in telemetry for preference %q is expected to be a string", tt.prefParam))
+
+								helper.ClearTelemetryFile()
+
+								helper.Cmd("odo", "preference", "set", tt.prefParam, tt.differentValueIfAnonymized, "--force").ShouldPass()
+								td = helper.GetTelemetryDebugData()
+								Expect(td.Properties.CmdProperties[segmentContext.PreferenceValue]).ShouldNot(BeEmpty())
+								pref2Val, ok := td.Properties.CmdProperties[segmentContext.PreferenceValue].(string)
+								Expect(ok).To(BeTrue(), fmt.Sprintf("value recorded in telemetry for preference %q is expected to be a string", tt.prefParam))
+
+								Expect(pref1Val).ToNot(Equal(pref2Val))
+							})
+						}
 					})
-				})
-			})
+				}
 
-			When("Telemetry is disabled in preferences", func() {
-				BeforeEach(func() {
-					prefClient := helper.EnableTelemetryDebug()
-					err := prefClient.SetConfiguration(preference.ConsentTelemetrySetting, "false")
-					Expect(err).ShouldNot(HaveOccurred())
-					Expect(os.Unsetenv(segment.TrackingConsentEnv)).NotTo(HaveOccurred())
-				})
+				When("telemetry is enabled in preferences", func() {
+					BeforeEach(func() {
+						Expect(os.Unsetenv(segment.TrackingConsentEnv)).NotTo(HaveOccurred())
+						Expect(prefClient.SetConfiguration(preference.ConsentTelemetrySetting, "true")).ShouldNot(HaveOccurred())
+					})
 
-				AfterEach(func() {
-					helper.ResetTelemetry()
+					When("setting ConsentTelemetry to false", func() {
+						BeforeEach(func() {
+							helper.Cmd("odo", "preference", "set", "ConsentTelemetry", "false", "--force").ShouldPass()
+						})
+
+						// https://github.com/redhat-developer/odo/issues/6790
+						It("should record the odo-preference-set command in telemetry", func() {
+							td := helper.GetTelemetryDebugData()
+							Expect(td.Event).To(ContainSubstring("odo preference set"))
+							Expect(td.Properties.Success).To(BeTrue())
+							Expect(td.Properties.Error).To(BeEmpty())
+							Expect(td.Properties.ErrorType).To(BeEmpty())
+							Expect(td.Properties.CmdProperties[segmentContext.Flags]).To(Equal("force"))
+							Expect(td.Properties.CmdProperties[segmentContext.PreviousTelemetryStatus]).To(BeTrue())
+							Expect(td.Properties.CmdProperties[segmentContext.TelemetryStatus]).To(BeFalse())
+						})
+					})
 				})
 
-				When("setting ConsentTelemetry to true", func() {
+				When("Telemetry is disabled in preferences", func() {
 					BeforeEach(func() {
-						helper.Cmd("odo", "preference", "set", "ConsentTelemetry", "true", "--force").ShouldPass()
+						Expect(os.Unsetenv(segment.TrackingConsentEnv)).NotTo(HaveOccurred())
+						Expect(prefClient.SetConfiguration(preference.ConsentTelemetrySetting, "false")).ShouldNot(HaveOccurred())
 					})
 
-					// https://github.com/redhat-developer/odo/issues/6790
-					It("should record the odo-preference-set command in telemetry", func() {
-						td := helper.GetTelemetryDebugData()
-						Expect(td.Event).To(ContainSubstring("odo preference set"))
-						Expect(td.Properties.Success).To(BeTrue())
-						Expect(td.Properties.Error).To(BeEmpty())
-						Expect(td.Properties.ErrorType).To(BeEmpty())
-						Expect(td.Properties.CmdProperties[segmentContext.Flags]).To(Equal("force"))
-						Expect(td.Properties.CmdProperties[segmentContext.PreviousTelemetryStatus]).To(BeFalse())
-						Expect(td.Properties.CmdProperties[segmentContext.TelemetryStatus]).To(BeTrue())
+					When("setting ConsentTelemetry to true", func() {
+						BeforeEach(func() {
+							helper.Cmd("odo", "preference", "set", "ConsentTelemetry", "true", "--force").ShouldPass()
+						})
+
+						// https://github.com/redhat-developer/odo/issues/6790
+						It("should record the odo-preference-set command in telemetry", func() {
+							td := helper.GetTelemetryDebugData()
+							Expect(td.Event).To(ContainSubstring("odo preference set"))
+							Expect(td.Properties.Success).To(BeTrue())
+							Expect(td.Properties.Error).To(BeEmpty())
+							Expect(td.Properties.ErrorType).To(BeEmpty())
+							Expect(td.Properties.CmdProperties[segmentContext.Flags]).To(Equal("force"))
+							Expect(td.Properties.CmdProperties[segmentContext.PreviousTelemetryStatus]).To(BeFalse())
+							Expect(td.Properties.CmdProperties[segmentContext.TelemetryStatus]).To(BeTrue())
+						})
 					})
 				})
 			})