Make sure event is recorded whenever telemetry preference is changed (#…

…6842)

* Add integration test highlighting the expectations

* Use the cancellable context derived from 'cmd.Context' where needed

* Introduce new 'wasTelemetryEnabled' telemetry property

This is recorded before any command is run.
And it will help determine if telemetry was
changed (e.g., from enabled to disabled),
so that we can record the event in this case.

* Send telemetry if it was enabled previously or if it is currently enabled

This is done by reading the actual telemetry setting
(either from env vars or the preferences file)
at the moment we want to record the telemetry event.
This covers cases where a command ('odo preference set ConsentTelemetry')
or an environment variable ('ODO_TRACKING_CONSENT')
updated the telemetry consent for example.

pkg/odo/cli/telemetry/telemetry.go

@@ -47,7 +47,16 @@ func (o *TelemetryOptions) Validate(ctx context.Context) (err error) {
 }
 
 func (o *TelemetryOptions) Run(ctx context.Context) (err error) {
-	if !scontext.GetTelemetryStatus(ctx) {
+	// Telemetry is sent only if it was enabled previously (GetPreviousTelemetryStatus) or if it is currently enabled (GetTelemetryStatus).
+	// For example, if it was enabled previously, and user disables telemetry, we still want the event disabling it to be recorded.
+	// And if it was disabled, and now it is enabled, we want to track this event as well.
+	var wasTelemetryEnabled bool
+	val, ok := o.telemetryData.Properties.CmdProperties[scontext.PreviousTelemetryStatus]
+	if ok {
+		wasTelemetryEnabled = val.(bool)
+	}
+	if !(wasTelemetryEnabled || segment.IsTelemetryEnabled(o.clientset.PreferenceClient, envcontext.GetEnvConfig(ctx))) {
+		klog.V(2).Infof("Telemetry not enabled!")
 		return nil
 	}
 
@@ -89,6 +98,6 @@ func NewCmdTelemetry(name string) *cobra.Command {
 			return genericclioptions.GenericRun(o, cmd, args)
 		},
 	}
-	clientset.Add(telemetryCmd)
+	clientset.Add(telemetryCmd, clientset.PREFERENCE)
 	return telemetryCmd
 }

pkg/odo/genericclioptions/runnable.go

@@ -16,14 +16,13 @@ import (
 	"github.com/devfile/library/v2/pkg/devfile/parser"
 
 	"github.com/redhat-developer/odo/pkg/machineoutput"
+	"github.com/redhat-developer/odo/pkg/version"
 
 	"github.com/redhat-developer/odo/pkg/odo/cmdline"
 	"github.com/redhat-developer/odo/pkg/odo/commonflags"
 	"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
 	commonutil "github.com/redhat-developer/odo/pkg/util"
 
-	"github.com/redhat-developer/odo/pkg/version"
-
 	"gopkg.in/AlecAivazis/survey.v1"
 
 	"github.com/redhat-developer/odo/pkg/odo/cli/feature"
@@ -158,17 +157,17 @@ func GenericRun(o Runnable, cmd *cobra.Command, args []string) error {
 	}
 
 	// We can dereference as there is a default value defined for this config field
-	err = scontext.SetCaller(cmd.Context(), envConfig.TelemetryCaller)
+	err = scontext.SetCaller(ctx, envConfig.TelemetryCaller)
 	if err != nil {
 		klog.V(3).Infof("error handling caller property for telemetry: %v", err)
 		err = nil
 	}
 
-	scontext.SetFlags(cmd.Context(), cmd.Flags())
+	scontext.SetFlags(ctx, cmd.Flags())
 	// set value for telemetry status in context so that we do not need to call IsTelemetryEnabled every time to check its status
-	scontext.SetTelemetryStatus(cmd.Context(), segment.IsTelemetryEnabled(userConfig, envConfig))
+	scontext.SetPreviousTelemetryStatus(ctx, segment.IsTelemetryEnabled(userConfig, envConfig))
 
-	scontext.SetExperimentalMode(cmd.Context(), envConfig.OdoExperimentalMode)
+	scontext.SetExperimentalMode(ctx, envConfig.OdoExperimentalMode)
 
 	// Send data to telemetry in case of user interrupt
 	captureSignals := []os.Signal{syscall.SIGHUP, syscall.SIGTERM, syscall.SIGQUIT, os.Interrupt}
@@ -180,7 +179,7 @@ func GenericRun(o Runnable, cmd *cobra.Command, args []string) error {
 				log.Errorf("error handling interrupt signal : %v", err)
 			}
 		}
-		scontext.SetSignal(cmd.Context(), receivedSignal)
+		scontext.SetSignal(ctx, receivedSignal)
 		startTelemetry(cmd, err, startTime)
 	})
 
@@ -292,34 +291,47 @@ func GenericRun(o Runnable, cmd *cobra.Command, args []string) error {
 // startTelemetry uploads the data to segment if user has consented to usage data collection and the command is not telemetry
 // TODO: move this function to a more suitable place, preferably pkg/segment
 func startTelemetry(cmd *cobra.Command, err error, startTime time.Time) {
-	if scontext.GetTelemetryStatus(cmd.Context()) && !strings.Contains(cmd.CommandPath(), "telemetry") {
-		uploadData := &segment.TelemetryData{
-			Event: cmd.CommandPath(),
-			Properties: segment.TelemetryProperties{
-				Duration:      time.Since(startTime).Milliseconds(),
-				Success:       err == nil,
-				Tty:           segment.RunningInTerminal(),
-				Version:       fmt.Sprintf("odo %v (%v)", version.VERSION, version.GITCOMMIT),
-				CmdProperties: scontext.GetContextProperties(cmd.Context()),
-			},
-		}
-		if err != nil {
-			uploadData.Properties.Error = segment.SetError(err)
-			uploadData.Properties.ErrorType = segment.ErrorType(err)
-		}
-		data, err1 := json.Marshal(uploadData)
-		if err1 != nil {
-			klog.V(4).Infof("Failed to marshall telemetry data. %q", err1.Error())
-		}
-		command := exec.Command(os.Args[0], "telemetry", string(data))
-		if err1 = command.Start(); err1 != nil {
-			klog.V(4).Infof("Failed to start the telemetry process. Error: %q", err1.Error())
-			return
-		}
-		if err1 = command.Process.Release(); err1 != nil {
-			klog.V(4).Infof("Failed to release the process. %q", err1.Error())
-			return
-		}
+	if strings.Contains(cmd.CommandPath(), "telemetry") {
+		return
+	}
+	ctx := cmd.Context()
+	// Re-read the preferences, so that we can get the real settings in case a command updated the preferences file
+	currentUserConfig, prefErr := preference.NewClient(ctx)
+	if prefErr != nil {
+		klog.V(2).Infof("unable to build preferences client to get telemetry consent status: %v", prefErr)
+		return
+	}
+	isTelemetryEnabled := segment.IsTelemetryEnabled(currentUserConfig, envcontext.GetEnvConfig(ctx))
+	if !(scontext.GetPreviousTelemetryStatus(ctx) || isTelemetryEnabled) {
+		return
+	}
+	scontext.SetTelemetryStatus(ctx, isTelemetryEnabled)
+	uploadData := &segment.TelemetryData{
+		Event: cmd.CommandPath(),
+		Properties: segment.TelemetryProperties{
+			Duration:      time.Since(startTime).Milliseconds(),
+			Success:       err == nil,
+			Tty:           segment.RunningInTerminal(),
+			Version:       fmt.Sprintf("odo %v (%v)", version.VERSION, version.GITCOMMIT),
+			CmdProperties: scontext.GetContextProperties(ctx),
+		},
+	}
+	if err != nil {
+		uploadData.Properties.Error = segment.SetError(err)
+		uploadData.Properties.ErrorType = segment.ErrorType(err)
+	}
+	data, err1 := json.Marshal(uploadData)
+	if err1 != nil {
+		klog.V(4).Infof("Failed to marshall telemetry data. %q", err1.Error())
+	}
+	command := exec.Command(os.Args[0], "telemetry", string(data))
+	if err1 = command.Start(); err1 != nil {
+		klog.V(4).Infof("Failed to start the telemetry process. Error: %q", err1.Error())
+		return
+	}
+	if err1 = command.Process.Release(); err1 != nil {
+		klog.V(4).Infof("Failed to release the process. %q", err1.Error())
+		return
 	}
 }
 

pkg/segment/context/context.go

@@ -20,19 +20,20 @@ import (
 )
 
 const (
-	Caller           = "caller"
-	ComponentType    = "componentType"
-	ClusterType      = "clusterType"
-	TelemetryStatus  = "isTelemetryEnabled"
-	DevfileName      = "devfileName"
-	Language         = "language"
-	ProjectType      = "projectType"
-	NOTFOUND         = "not-found"
-	InteractiveMode  = "interactive"
-	ExperimentalMode = "experimental"
-	Flags            = "flags"
-	Platform         = "platform"
-	PlatformVersion  = "platformVersion"
+	Caller                  = "caller"
+	ComponentType           = "componentType"
+	ClusterType             = "clusterType"
+	PreviousTelemetryStatus = "wasTelemetryEnabled"
+	TelemetryStatus         = "isTelemetryEnabled"
+	DevfileName             = "devfileName"
+	Language                = "language"
+	ProjectType             = "projectType"
+	NOTFOUND                = "not-found"
+	InteractiveMode         = "interactive"
+	ExperimentalMode        = "experimental"
+	Flags                   = "flags"
+	Platform                = "platform"
+	PlatformVersion         = "platformVersion"
 )
 
 const (
@@ -158,7 +159,12 @@ func setPlatformPodman(ctx context.Context, client podman.Client) {
 	setContextProperty(ctx, PlatformVersion, version.Client.Version)
 }
 
-// SetTelemetryStatus sets telemetry status before a command is run
+// SetPreviousTelemetryStatus sets telemetry status before a command is run
+func SetPreviousTelemetryStatus(ctx context.Context, isEnabled bool) {
+	setContextProperty(ctx, PreviousTelemetryStatus, isEnabled)
+}
+
+// SetTelemetryStatus sets telemetry status after a command is run
 func SetTelemetryStatus(ctx context.Context, isEnabled bool) {
 	setContextProperty(ctx, TelemetryStatus, isEnabled)
 }
@@ -222,7 +228,16 @@ func SetCaller(ctx context.Context, caller string) error {
 	return err
 }
 
-// GetTelemetryStatus gets the telemetry status that is set before a command is run
+// GetPreviousTelemetryStatus gets the telemetry status that was seen before a command is run
+func GetPreviousTelemetryStatus(ctx context.Context) bool {
+	wasEnabled, ok := GetContextProperties(ctx)[PreviousTelemetryStatus]
+	if ok {
+		return wasEnabled.(bool)
+	}
+	return false
+}
+
+// GetTelemetryStatus gets the current telemetry status that is set after a command is run
 func GetTelemetryStatus(ctx context.Context) bool {
 	isEnabled, ok := GetContextProperties(ctx)[TelemetryStatus]
 	if ok {

tests/helper/helper_telemetry.go

@@ -24,7 +24,7 @@ func setDebugTelemetryFile(value string) error {
 
 // EnableTelemetryDebug creates a temp file to use for debugging telemetry.
 // it also sets up envs and cfg for the same
-func EnableTelemetryDebug() {
+func EnableTelemetryDebug() preference.Client {
 	Expect(os.Setenv(segment.TrackingConsentEnv, "yes")).NotTo(HaveOccurred())
 
 	ctx := context.Background()
@@ -39,6 +39,8 @@ func EnableTelemetryDebug() {
 	Expect(err).NotTo(HaveOccurred())
 	Expect(setDebugTelemetryFile(tempFile.Name())).NotTo(HaveOccurred())
 	Expect(tempFile.Close()).NotTo(HaveOccurred())
+
+	return cfg
 }
 
 func GetDebugTelemetryFile() string {

tests/integration/cmd_pref_config_test.go

@@ -9,6 +9,9 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/tidwall/gjson"
 
+	"github.com/redhat-developer/odo/pkg/preference"
+	"github.com/redhat-developer/odo/pkg/segment"
+	segmentContext "github.com/redhat-developer/odo/pkg/segment/context"
 	"github.com/redhat-developer/odo/tests/helper"
 )
 
@@ -200,6 +203,68 @@ OdoSettings:
 					Expect(output).ToNot(ContainSubstring(promptMessageSubString))
 				})
 			})
+
+			When("Telemetry is enabled in preferences", func() {
+				BeforeEach(func() {
+					prefClient := helper.EnableTelemetryDebug()
+					err := prefClient.SetConfiguration(preference.ConsentTelemetrySetting, "true")
+					Expect(err).ShouldNot(HaveOccurred())
+					Expect(os.Unsetenv(segment.TrackingConsentEnv)).NotTo(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("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())
+				})
+
+				AfterEach(func() {
+					helper.ResetTelemetry()
+				})
+
+				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())
+					})
+				})
+			})
 		})
 	}
 	When("DevfileRegistriesList CRD is installed on cluster", func() {