Repository: code-ready/crc
Pull request #1885 (2021-01-25 08:16:36)

telemetry: send additional data (cpus, memory, network mode) 
This uses context to pass the data. This is required as we cannot easily extend or change the signature of cobra.Command RunE.

openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1885#" title="Author self-approved">guillaumerose</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [guillaumerose]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci[bot]: @guillaumerose: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 90a1fd2ca193b16f4943c771d926a71fc5aed39b | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1885/pull-ci-code-ready-crc-master-e2e-crc/1353814817512624128) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1885). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:guillaumerose).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
guillaumerose: /retest
openshift-ci[bot]: @guillaumerose: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/e2e-crc | 90a1fd2ca193b16f4943c771d926a71fc5aed39b | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/code-ready_crc/1885/pull-ci-code-ready-crc-master-e2e-crc/1354021598549512192) | `/test e2e-crc`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=code-ready&repo=crc&pr=1885). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:guillaumerose).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/1885#pullrequestreview-580269666" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/1885#" title="Author self-approved">guillaumerose</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=code-ready%2Fcrc).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/code-ready/crc/blob/master/OWNERS)~~ [cfergeau,guillaumerose]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/Makefile b/Makefile
index fa391df49..7894588ef 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 SHELL := /bin/bash
 
-BUNDLE_VERSION = 4.6.12
+BUNDLE_VERSION = 4.6.13
 BUNDLE_EXTENSION = crcbundle
 CRC_VERSION = 1.21.0
 COMMIT_SHA=$(shell git rev-parse --short HEAD)
diff --git a/centos_ci.sh b/centos_ci.sh
index f223021ec..3c1d02f43 100644
--- a/centos_ci.sh
+++ b/centos_ci.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 
 # bundle location
-BUNDLE_VERSION=4.6.12
+BUNDLE_VERSION=4.6.13
 BUNDLE=crc_libvirt_$BUNDLE_VERSION.crcbundle
 GO_VERSION=1.14.13
 
diff --git a/cmd/crc/cmd/config/get.go b/cmd/crc/cmd/config/get.go
index 9f694eb79..6a31d3ad2 100644
--- a/cmd/crc/cmd/config/get.go
+++ b/cmd/crc/cmd/config/get.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/code-ready/crc/pkg/crc/config"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/spf13/cobra"
 )
 
@@ -18,15 +19,19 @@ func configGetCmd(config config.Storage) *cobra.Command {
 				return errors.New("Please provide a configuration property to get")
 			}
 			key := args[0]
+
 			v := config.Get(key)
-			switch {
-			case v.Invalid:
+			if v.Invalid {
 				return fmt.Errorf("Configuration property '%s' does not exist", key)
-			case v.IsDefault:
+			}
+
+			telemetry.SetContextProperty(cmd.Context(), "key", args[0])
+
+			if v.IsDefault {
 				return fmt.Errorf("Configuration property '%s' is not set. Default value is '%s'", key, v.AsString())
-			default:
-				fmt.Println(key, ":", v.AsString())
+
 			}
+			fmt.Println(key, ":", v.AsString())
 			return nil
 		},
 	}
diff --git a/cmd/crc/cmd/config/set.go b/cmd/crc/cmd/config/set.go
index 0afc89d54..e13b40f76 100644
--- a/cmd/crc/cmd/config/set.go
+++ b/cmd/crc/cmd/config/set.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/code-ready/crc/pkg/crc/config"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/spf13/cobra"
 )
 
@@ -22,6 +23,8 @@ func configSetCmd(config config.Storage) *cobra.Command {
 				return err
 			}
 
+			telemetry.SetContextProperty(cmd.Context(), "key", args[0])
+
 			if setMessage != "" {
 				fmt.Println(setMessage)
 			}
diff --git a/cmd/crc/cmd/config/unset.go b/cmd/crc/cmd/config/unset.go
index 222c20595..e356d8590 100644
--- a/cmd/crc/cmd/config/unset.go
+++ b/cmd/crc/cmd/config/unset.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/code-ready/crc/pkg/crc/config"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/spf13/cobra"
 )
 
@@ -21,6 +22,9 @@ func configUnsetCmd(config config.Storage) *cobra.Command {
 			if err != nil {
 				return err
 			}
+
+			telemetry.SetContextProperty(cmd.Context(), "key", args[0])
+
 			if unsetMessage != "" {
 				fmt.Println(unsetMessage)
 			}
diff --git a/cmd/crc/cmd/root.go b/cmd/crc/cmd/root.go
index 98267b396..77d07a014 100644
--- a/cmd/crc/cmd/root.go
+++ b/cmd/crc/cmd/root.go
@@ -1,12 +1,15 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"strings"
 	"time"
 
+	"github.com/code-ready/crc/pkg/crc/telemetry"
+
 	cmdConfig "github.com/code-ready/crc/cmd/crc/cmd/config"
 	crcConfig "github.com/code-ready/crc/pkg/crc/config"
 	"github.com/code-ready/crc/pkg/crc/constants"
@@ -85,7 +88,7 @@ func runRoot() {
 func Execute() {
 	attachMiddleware([]string{}, rootCmd)
 
-	if err := rootCmd.Execute(); err != nil {
+	if err := rootCmd.ExecuteContext(telemetry.NewContext(context.Background())); err != nil {
 		runPostrun()
 		_, _ = fmt.Fprintln(os.Stderr, err.Error())
 		os.Exit(1)
@@ -174,7 +177,7 @@ func executeWithLogging(fullCmd string, input func(cmd *cobra.Command, args []st
 		logging.Debugf("Running '%s'", fullCmd)
 		startTime := time.Now()
 		err := input(cmd, args)
-		if serr := segmentClient.Upload(fullCmd, time.Since(startTime), err); serr != nil {
+		if serr := segmentClient.Upload(cmd.Context(), fullCmd, time.Since(startTime), err); serr != nil {
 			logging.Debugf("Cannot send data to telemetry: %v", serr)
 		}
 		return err
diff --git a/cmd/crc/cmd/start.go b/cmd/crc/cmd/start.go
index f48df266c..c4185e188 100644
--- a/cmd/crc/cmd/start.go
+++ b/cmd/crc/cmd/start.go
@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -16,6 +17,7 @@ import (
 	"github.com/code-ready/crc/pkg/crc/logging"
 	"github.com/code-ready/crc/pkg/crc/machine"
 	"github.com/code-ready/crc/pkg/crc/preflight"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/code-ready/crc/pkg/crc/validation"
 	crcversion "github.com/code-ready/crc/pkg/crc/version"
 	"github.com/spf13/cobra"
@@ -46,14 +48,14 @@ var startCmd = &cobra.Command{
 		if err := viper.BindFlagSet(cmd.Flags()); err != nil {
 			return err
 		}
-		if err := renderStartResult(runStart(args)); err != nil {
+		if err := renderStartResult(runStart(cmd.Context())); err != nil {
 			return err
 		}
 		return nil
 	},
 }
 
-func runStart(arguments []string) (*machine.StartResult, error) {
+func runStart(ctx context.Context) (*machine.StartResult, error) {
 	if err := validateStartFlags(); err != nil {
 		return nil, err
 	}
@@ -64,6 +66,10 @@ func runStart(arguments []string) (*machine.StartResult, error) {
 		return nil, err
 	}
 
+	for _, key := range []string{cmdConfig.CPUs, cmdConfig.Memory, cmdConfig.DiskSize} {
+		telemetry.SetContextProperty(ctx, key, config.Get(key).AsInt())
+	}
+
 	startConfig := machine.StartConfig{
 		BundlePath: config.Get(cmdConfig.Bundle).AsString(),
 		Memory:     config.Get(cmdConfig.Memory).AsInt(),
diff --git a/pkg/crc/machine/hyperkit/constants.go b/pkg/crc/machine/hyperkit/constants.go
index 9b2b3a2d8..a3cfe45f8 100644
--- a/pkg/crc/machine/hyperkit/constants.go
+++ b/pkg/crc/machine/hyperkit/constants.go
@@ -6,7 +6,7 @@ import "fmt"
 
 const (
 	MachineDriverCommand = "crc-driver-hyperkit"
-	MachineDriverVersion = "0.12.10"
+	MachineDriverVersion = "0.12.11"
 	HyperKitCommand      = "hyperkit"
 	HyperKitVersion      = "v0.20200224-44-gb54460"
 )
diff --git a/pkg/crc/machine/libvirt/constants.go b/pkg/crc/machine/libvirt/constants.go
index 27d970ea3..bb9e80723 100644
--- a/pkg/crc/machine/libvirt/constants.go
+++ b/pkg/crc/machine/libvirt/constants.go
@@ -16,7 +16,7 @@ const (
 
 const (
 	MachineDriverCommand = "crc-driver-libvirt"
-	MachineDriverVersion = "0.12.13"
+	MachineDriverVersion = "0.12.14"
 )
 
 var (
diff --git a/pkg/crc/segment/segment.go b/pkg/crc/segment/segment.go
index 723efd6bd..0e0f133c2 100644
--- a/pkg/crc/segment/segment.go
+++ b/pkg/crc/segment/segment.go
@@ -1,6 +1,7 @@
 package segment
 
 import (
+	"context"
 	"io/ioutil"
 	"net"
 	"os"
@@ -12,6 +13,7 @@ import (
 	crcConfig "github.com/code-ready/crc/pkg/crc/config"
 	"github.com/code-ready/crc/pkg/crc/constants"
 	"github.com/code-ready/crc/pkg/crc/logging"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/code-ready/crc/pkg/crc/version"
 	"github.com/pborman/uuid"
 	"github.com/segmentio/analytics-go"
@@ -54,7 +56,7 @@ func (c *Client) Close() error {
 	return c.segmentClient.Close()
 }
 
-func (c *Client) Upload(action string, duration time.Duration, err error) error {
+func (c *Client) Upload(ctx context.Context, action string, duration time.Duration, err error) error {
 	if c.config.Get(config.ConsentTelemetry).AsString() != "yes" {
 		return nil
 	}
@@ -66,13 +68,17 @@ func (c *Client) Upload(action string, duration time.Duration, err error) error
 
 	if err := c.segmentClient.Enqueue(analytics.Identify{
 		AnonymousId: anonymousID,
-		Traits:      traits(),
+		Traits:      addConfigTraits(c.config, traits()),
 	}); err != nil {
 		return err
 	}
 
-	properties := analytics.NewProperties().
-		Set("version", version.GetCRCVersion()).
+	properties := analytics.NewProperties()
+	for k, v := range telemetry.GetContextProperties(ctx) {
+		properties = properties.Set(k, v)
+	}
+
+	properties = properties.Set("version", version.GetCRCVersion()).
 		Set("success", err == nil).
 		Set("duration", duration.Milliseconds())
 	if err != nil {
@@ -86,6 +92,18 @@ func (c *Client) Upload(action string, duration time.Duration, err error) error
 	})
 }
 
+func addConfigTraits(c *crcConfig.Config, in analytics.Traits) analytics.Traits {
+	return in.
+		Set("proxy", isProxyUsed(c)).
+		Set(config.NetworkMode, c.Get(config.NetworkMode).AsString()).
+		Set(config.EnableClusterMonitoring, c.Get(config.EnableClusterMonitoring).AsBool()).
+		Set(config.ExperimentalFeatures, c.Get(config.ExperimentalFeatures).AsBool())
+}
+
+func isProxyUsed(c *crcConfig.Config) bool {
+	return c.Get(config.HTTPProxy).AsString() != "" || c.Get(config.HTTPSProxy).AsString() != ""
+}
+
 func getUserIdentity(telemetryFilePath string) (string, error) {
 	var id []byte
 	if err := os.MkdirAll(filepath.Dir(telemetryFilePath), 0750); err != nil {
diff --git a/pkg/crc/telemetry/telemetry.go b/pkg/crc/telemetry/telemetry.go
new file mode 100644
index 000000000..f0a0ad841
--- /dev/null
+++ b/pkg/crc/telemetry/telemetry.go
@@ -0,0 +1,58 @@
+package telemetry
+
+import (
+	"context"
+	"sync"
+)
+
+type contextKey struct{}
+
+var key = contextKey{}
+
+type Properties struct {
+	lock    sync.Mutex
+	storage map[string]interface{}
+}
+
+func (p *Properties) set(name string, value interface{}) {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+	p.storage[name] = value
+}
+
+func (p *Properties) values() map[string]interface{} {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+	ret := make(map[string]interface{})
+	for k, v := range p.storage {
+		ret[k] = v
+	}
+	return ret
+}
+
+func propertiesFromContext(ctx context.Context) *Properties {
+	value := ctx.Value(key)
+	if cast, ok := value.(*Properties); ok {
+		return cast
+	}
+	return nil
+}
+
+func NewContext(ctx context.Context) context.Context {
+	return context.WithValue(ctx, key, &Properties{storage: make(map[string]interface{})})
+}
+
+func GetContextProperties(ctx context.Context) map[string]interface{} {
+	properties := propertiesFromContext(ctx)
+	if properties == nil {
+		return make(map[string]interface{})
+	}
+	return properties.values()
+}
+
+func SetContextProperty(ctx context.Context, key string, value interface{}) {
+	properties := propertiesFromContext(ctx)
+	if properties != nil {
+		properties.set(key, value)
+	}
+}


Test patch:
diff --git a/pkg/crc/segment/segment_test.go b/pkg/crc/segment/segment_test.go
index 4b3f4df49..af65b96f3 100644
--- a/pkg/crc/segment/segment_test.go
+++ b/pkg/crc/segment/segment_test.go
@@ -1,6 +1,7 @@
 package segment
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"io/ioutil"
@@ -15,6 +16,7 @@ import (
 	cmdConfig "github.com/code-ready/crc/cmd/crc/cmd/config"
 	crcConfig "github.com/code-ready/crc/pkg/crc/config"
 	"github.com/code-ready/crc/pkg/crc/logging"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/code-ready/crc/pkg/crc/version"
 	"github.com/stretchr/testify/require"
 )
@@ -24,11 +26,13 @@ type segmentResponse struct {
 		AnonymousID string `json:"anonymousId"`
 		MessageID   string `json:"messageId"`
 		Traits      struct {
-			OS string `json:"os"`
+			OS                   string `json:"os"`
+			ExperimentalFeatures bool   `json:"enable-experimental-features"`
 		} `json:"traits"`
 		Properties struct {
 			Error   string `json:"error"`
 			Version string `json:"version"`
+			CPUs    int    `json:"cpus"`
 		} `json:"properties"`
 		Type string `json:"type"`
 	} `json:"batch"`
@@ -59,6 +63,9 @@ func newTestConfig(value string) (*crcConfig.Config, error) {
 	if _, err := config.Set(cmdConfig.ConsentTelemetry, value); err != nil {
 		return nil, err
 	}
+	if _, err := config.Set(cmdConfig.ExperimentalFeatures, true); err != nil {
+		return nil, err
+	}
 	return config, nil
 }
 
@@ -77,7 +84,7 @@ func TestClientUploadWithConsent(t *testing.T) {
 	c, err := newCustomClient(config, filepath.Join(dir, "telemetry"), server.URL)
 	require.NoError(t, err)
 
-	require.NoError(t, c.Upload("start", time.Minute, errors.New("an error occurred")))
+	require.NoError(t, c.Upload(context.Background(), "start", time.Minute, errors.New("an error occurred")))
 	require.NoError(t, c.Close())
 
 	select {
@@ -86,6 +93,7 @@ func TestClientUploadWithConsent(t *testing.T) {
 		require.NoError(t, json.Unmarshal(x, &s))
 		require.Equal(t, s.Batch[0].Type, "identify")
 		require.Equal(t, s.Batch[0].Traits.OS, runtime.GOOS)
+		require.Equal(t, s.Batch[0].Traits.ExperimentalFeatures, true)
 		require.Equal(t, s.Batch[1].Type, "track")
 		require.Equal(t, s.Batch[1].Properties.Error, "an error occurred")
 		require.Equal(t, s.Batch[1].Properties.Version, version.GetCRCVersion())
@@ -94,6 +102,36 @@ func TestClientUploadWithConsent(t *testing.T) {
 	}
 }
 
+func TestClientUploadWithContext(t *testing.T) {
+	body, server := mockServer()
+	defer server.Close()
+	defer close(body)
+
+	dir, err := ioutil.TempDir("", "cfg")
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	config, err := newTestConfig("yes")
+	require.NoError(t, err)
+
+	c, err := newCustomClient(config, filepath.Join(dir, "telemetry"), server.URL)
+	require.NoError(t, err)
+
+	ctx := telemetry.NewContext(context.Background())
+	telemetry.SetContextProperty(ctx, "cpus", 6)
+	require.NoError(t, c.Upload(ctx, "start", time.Minute, nil))
+	require.NoError(t, c.Close())
+
+	select {
+	case x := <-body:
+		s := segmentResponse{}
+		require.NoError(t, json.Unmarshal(x, &s))
+		require.Equal(t, s.Batch[1].Properties.CPUs, 6)
+	default:
+		require.Fail(t, "server should receive data")
+	}
+}
+
 func TestClientUploadWithOutConsent(t *testing.T) {
 	body, server := mockServer()
 	defer server.Close()
@@ -109,7 +147,7 @@ func TestClientUploadWithOutConsent(t *testing.T) {
 	c, err := newCustomClient(config, filepath.Join(dir, "telemetry"), server.URL)
 	require.NoError(t, err)
 
-	require.NoError(t, c.Upload("start", time.Second, errors.New("an error occurred")))
+	require.NoError(t, c.Upload(context.Background(), "start", time.Second, errors.New("an error occurred")))
 	require.NoError(t, c.Close())
 
 	select {