Repository: code-ready/crc
Pull request #2000 (2021-02-15 09:42:47)

Distuiguish first start from others call of crc start 
With this, we will better understand the duration of the first crc start. 

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

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2000#" 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-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/code-ready/crc/pull/2000#pullrequestreview-591222457" title="Approved">cfergeau</a>*, *<a href="https://github.com/code-ready/crc/pull/2000#" 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":[]} -->
openshift-ci-robot: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/cmd/crc/cmd/config/get.go b/cmd/crc/cmd/config/get.go
index 6a31d3ad2..c2f35b807 100644
--- a/cmd/crc/cmd/config/get.go
+++ b/cmd/crc/cmd/config/get.go
@@ -25,7 +25,7 @@ func configGetCmd(config config.Storage) *cobra.Command {
 				return fmt.Errorf("Configuration property '%s' does not exist", key)
 			}
 
-			telemetry.SetContextProperty(cmd.Context(), "key", args[0])
+			telemetry.SetConfigurationKey(cmd.Context(), args[0])
 
 			if v.IsDefault {
 				return fmt.Errorf("Configuration property '%s' is not set. Default value is '%s'", key, v.AsString())
diff --git a/cmd/crc/cmd/config/set.go b/cmd/crc/cmd/config/set.go
index cec3e8469..c7930ba67 100644
--- a/cmd/crc/cmd/config/set.go
+++ b/cmd/crc/cmd/config/set.go
@@ -24,7 +24,7 @@ CONFIG-KEYS: ` + "\n\n" + configurableFields(config),
 				return err
 			}
 
-			telemetry.SetContextProperty(cmd.Context(), "key", args[0])
+			telemetry.SetConfigurationKey(cmd.Context(), args[0])
 
 			if setMessage != "" {
 				fmt.Println(setMessage)
diff --git a/cmd/crc/cmd/config/unset.go b/cmd/crc/cmd/config/unset.go
index e356d8590..4bb949c01 100644
--- a/cmd/crc/cmd/config/unset.go
+++ b/cmd/crc/cmd/config/unset.go
@@ -23,7 +23,7 @@ func configUnsetCmd(config config.Storage) *cobra.Command {
 				return err
 			}
 
-			telemetry.SetContextProperty(cmd.Context(), "key", args[0])
+			telemetry.SetConfigurationKey(cmd.Context(), args[0])
 
 			if unsetMessage != "" {
 				fmt.Println(unsetMessage)
diff --git a/cmd/crc/cmd/start.go b/cmd/crc/cmd/start.go
index 4927f9264..661c73635 100644
--- a/cmd/crc/cmd/start.go
+++ b/cmd/crc/cmd/start.go
@@ -17,7 +17,6 @@ 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/code-ready/crc/pkg/os/shell"
@@ -63,10 +62,6 @@ func runStart(ctx context.Context) (*machine.StartResult, error) {
 
 	checkIfNewVersionAvailable(config.Get(cmdConfig.DisableUpdateCheck).AsBool())
 
-	telemetry.SetContextProperty(ctx, cmdConfig.CPUs, config.Get(cmdConfig.CPUs).AsInt())
-	telemetry.SetContextProperty(ctx, cmdConfig.Memory, uint64(config.Get(cmdConfig.Memory).AsInt())*1024*1024)
-	telemetry.SetContextProperty(ctx, cmdConfig.DiskSize, uint64(config.Get(cmdConfig.DiskSize).AsInt())*1024*1024*1024)
-
 	startConfig := machine.StartConfig{
 		BundlePath: config.Get(cmdConfig.Bundle).AsString(),
 		Memory:     config.Get(cmdConfig.Memory).AsInt(),
@@ -85,7 +80,7 @@ func runStart(ctx context.Context) (*machine.StartResult, error) {
 		}
 	}
 
-	return client.Start(startConfig)
+	return client.Start(ctx, startConfig)
 }
 
 func renderStartResult(result *machine.StartResult, err error) error {
diff --git a/pkg/crc/api/adapter.go b/pkg/crc/api/adapter.go
index 68dc350ab..37cf6fb9f 100644
--- a/pkg/crc/api/adapter.go
+++ b/pkg/crc/api/adapter.go
@@ -1,6 +1,8 @@
 package api
 
 import (
+	"context"
+
 	"github.com/code-ready/crc/pkg/crc/logging"
 	"github.com/code-ready/crc/pkg/crc/machine"
 )
@@ -86,7 +88,7 @@ func (a *Adapter) GetConsoleURL() ConsoleResult {
 }
 
 func (a *Adapter) Start(startConfig machine.StartConfig) StartResult {
-	res, err := a.Underlying.Start(startConfig)
+	res, err := a.Underlying.Start(context.Background(), startConfig)
 	if err != nil {
 		logging.Error(err)
 		return StartResult{
diff --git a/pkg/crc/machine/client.go b/pkg/crc/machine/client.go
index c7d98e091..b3770047f 100644
--- a/pkg/crc/machine/client.go
+++ b/pkg/crc/machine/client.go
@@ -1,6 +1,8 @@
 package machine
 
 import (
+	"context"
+
 	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/network"
@@ -15,7 +17,7 @@ type Client interface {
 	GetConsoleURL() (*ConsoleResult, error)
 	IP() (string, error)
 	PowerOff() error
-	Start(startConfig StartConfig) (*StartResult, error)
+	Start(ctx context.Context, startConfig StartConfig) (*StartResult, error)
 	Status() (*ClusterStatusResult, error)
 	Stop() (state.State, error)
 	IsRunning() (bool, error)
diff --git a/pkg/crc/machine/fakemachine/client.go b/pkg/crc/machine/fakemachine/client.go
index c64d3189a..64387aece 100644
--- a/pkg/crc/machine/fakemachine/client.go
+++ b/pkg/crc/machine/fakemachine/client.go
@@ -1,6 +1,7 @@
 package fakemachine
 
 import (
+	"context"
 	"errors"
 
 	"github.com/code-ready/crc/pkg/crc/machine"
@@ -67,7 +68,7 @@ func (c *Client) PowerOff() error {
 	return nil
 }
 
-func (c *Client) Start(startConfig machine.StartConfig) (*machine.StartResult, error) {
+func (c *Client) Start(ctx context.Context, startConfig machine.StartConfig) (*machine.StartResult, error) {
 	if c.Failing {
 		return nil, errors.New("Failed to start")
 	}
diff --git a/pkg/crc/machine/start.go b/pkg/crc/machine/start.go
index 540ef40dc..bd08f6829 100644
--- a/pkg/crc/machine/start.go
+++ b/pkg/crc/machine/start.go
@@ -1,6 +1,7 @@
 package machine
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -23,6 +24,7 @@ import (
 	"github.com/code-ready/crc/pkg/crc/ssh"
 	crcssh "github.com/code-ready/crc/pkg/crc/ssh"
 	"github.com/code-ready/crc/pkg/crc/systemd"
+	"github.com/code-ready/crc/pkg/crc/telemetry"
 	"github.com/code-ready/crc/pkg/libmachine"
 	"github.com/code-ready/crc/pkg/libmachine/host"
 	crcos "github.com/code-ready/crc/pkg/os"
@@ -87,7 +89,11 @@ func (client *client) updateVMConfig(startConfig StartConfig, api libmachine.API
 	return nil
 }
 
-func (client *client) Start(startConfig StartConfig) (*StartResult, error) {
+func (client *client) Start(ctx context.Context, startConfig StartConfig) (*StartResult, error) {
+	telemetry.SetCPUs(ctx, startConfig.CPUs)
+	telemetry.SetMemory(ctx, uint64(startConfig.Memory)*1024*1024)
+	telemetry.SetDiskSize(ctx, uint64(startConfig.DiskSize)*1024*1024*1024)
+
 	if err := client.validateStartConfig(startConfig); err != nil {
 		return nil, err
 	}
@@ -103,7 +109,10 @@ func (client *client) Start(startConfig StartConfig) (*StartResult, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "Cannot determine if VM exists")
 	}
+
 	if !exists {
+		telemetry.SetStartType(ctx, telemetry.CreationStartType)
+
 		// Ask early for pull secret if it hasn't been requested yet
 		_, err = startConfig.PullSecret.Value()
 		if err != nil {
@@ -166,6 +175,8 @@ func (client *client) Start(startConfig StartConfig) (*StartResult, error) {
 			if err != nil {
 				return nil, errors.Wrap(err, "Cannot create cluster configuration")
 			}
+
+			telemetry.SetStartType(ctx, telemetry.AlreadyRunningStartType)
 			return &StartResult{
 				Status:         vmState,
 				ClusterConfig:  *clusterConfig,
@@ -173,6 +184,8 @@ func (client *client) Start(startConfig StartConfig) (*StartResult, error) {
 			}, nil
 		}
 
+		telemetry.SetStartType(ctx, telemetry.StartStartType)
+
 		logging.Infof("Starting CodeReady Containers VM for OpenShift %s...", crcBundleMetadata.GetOpenshiftVersion())
 
 		if err := client.updateVMConfig(startConfig, libMachineAPIClient, host); err != nil {
diff --git a/pkg/crc/telemetry/telemetry.go b/pkg/crc/telemetry/telemetry.go
index f0a0ad841..c39799d82 100644
--- a/pkg/crc/telemetry/telemetry.go
+++ b/pkg/crc/telemetry/telemetry.go
@@ -50,9 +50,37 @@ func GetContextProperties(ctx context.Context) map[string]interface{} {
 	return properties.values()
 }
 
-func SetContextProperty(ctx context.Context, key string, value interface{}) {
+func setContextProperty(ctx context.Context, key string, value interface{}) {
 	properties := propertiesFromContext(ctx)
 	if properties != nil {
 		properties.set(key, value)
 	}
 }
+
+func SetCPUs(ctx context.Context, value int) {
+	setContextProperty(ctx, "cpus", value)
+}
+
+func SetMemory(ctx context.Context, value uint64) {
+	setContextProperty(ctx, "memory", value)
+}
+
+func SetDiskSize(ctx context.Context, value uint64) {
+	setContextProperty(ctx, "disk-size", value)
+}
+
+func SetConfigurationKey(ctx context.Context, value string) {
+	setContextProperty(ctx, "key", value)
+}
+
+func SetStartType(ctx context.Context, value StartType) {
+	setContextProperty(ctx, "start-type", value)
+}
+
+type StartType string
+
+const (
+	AlreadyRunningStartType StartType = "already-running"
+	CreationStartType       StartType = "creation"
+	StartStartType          StartType = "start"
+)


Test patch:
diff --git a/pkg/crc/segment/segment_test.go b/pkg/crc/segment/segment_test.go
index 59b6ff053..d315fbb7e 100644
--- a/pkg/crc/segment/segment_test.go
+++ b/pkg/crc/segment/segment_test.go
@@ -155,7 +155,7 @@ func TestClientUploadWithContext(t *testing.T) {
 	require.NoError(t, err)
 
 	ctx := telemetry.NewContext(context.Background())
-	telemetry.SetContextProperty(ctx, "cpus", 6)
+	telemetry.SetCPUs(ctx, 6)
 	require.NoError(t, c.Upload(ctx, "start", time.Minute, nil))
 	require.NoError(t, c.Close())