Make sure event is recorded whenever telemetry preference is changed

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() {