Skip to content

Commit

Permalink
some refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
h4ck3rk3y committed Nov 7, 2023
1 parent e863b21 commit 3af63fd
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@ type StarlarkRunConfig struct {
DryRun bool
Parallelism int32
ExperimentalFeatureFlags []kurtosis_core_rpc_api_bindings.KurtosisFeatureFlag
// Deprecated: This field isn't used anymore
CloudInstanceId string
// Deprecated: This field isn't used anymore
CloudUserId string
ImageDownload kurtosis_core_rpc_api_bindings.ImageDownloadMode
CloudInstanceId string
CloudUserId string
ImageDownload kurtosis_core_rpc_api_bindings.ImageDownloadMode
}

type starlarkRunConfigOption func(*StarlarkRunConfig)
Expand Down
4 changes: 2 additions & 2 deletions cli/cli/helpers/engine_manager/engine_existence_guarantor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/Masterminds/semver/v3"
"github.com/kurtosis-tech/kurtosis/api/golang/engine/lib/kurtosis_context"
"github.com/kurtosis-tech/kurtosis/cli/cli/command_str_consts"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_client_factory"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_cloud_user_instance_id_helper"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_user_id_store"
"github.com/kurtosis-tech/kurtosis/cli/cli/kurtosis_config/resolved_config"
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface"
Expand Down Expand Up @@ -148,7 +148,7 @@ func (guarantor *engineExistenceGuarantor) VisitStopped() error {
return stacktrace.Propagate(err, "An error occurred getting metrics user id")
}

maybeCloudUserId, maybeCloudInstanceId := metrics_client_factory.GetMaybeCloudUserAndInstanceID()
maybeCloudUserId, maybeCloudInstanceId := metrics_cloud_user_instance_id_helper.GetMaybeCloudUserAndInstanceID()

var engineLaunchErr error
if guarantor.imageVersionTag == defaultEngineImageVersionTag {
Expand Down
22 changes: 3 additions & 19 deletions cli/cli/helpers/metrics_client_factory/metrics_client_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package metrics_client_factory
import (
"github.com/kurtosis-tech/kurtosis/cli/cli/defaults"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/do_nothing_metrics_client_callback"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_cloud_user_instance_id_helper"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_user_id_store"
"github.com/kurtosis-tech/kurtosis/cli/cli/kurtosis_cluster_setting"
"github.com/kurtosis-tech/kurtosis/cli/cli/kurtosis_config"
"github.com/kurtosis-tech/kurtosis/cli/cli/kurtosis_config/resolved_config"
"github.com/kurtosis-tech/kurtosis/contexts-config-store/store"
"github.com/kurtosis-tech/kurtosis/kurtosis_version"
"github.com/kurtosis-tech/kurtosis/metrics-library/golang/lib/analytics_logger"
"github.com/kurtosis-tech/kurtosis/metrics-library/golang/lib/metrics_client"
Expand All @@ -32,7 +32,7 @@ func GetMetricsClient() (metrics_client.MetricsClient, func() error, error) {
return nil, nil, stacktrace.NewError("An error occurred while determining whether configuration already exists")
}

maybeCloudUserId, maybeCloudInstanceId := GetMaybeCloudUserAndInstanceID()
maybeCloudUserId, maybeCloudInstanceId := metrics_cloud_user_instance_id_helper.GetMaybeCloudUserAndInstanceID()

var sendUserMetrics bool
if hasConfig {
Expand Down Expand Up @@ -75,7 +75,7 @@ func GetSegmentClient() (metrics_client.MetricsClient, func() error, error) {
// this is force set to true in order to get the segment client
sendUserMetrics := true

maybeCloudUserId, maybeCloudInstanceId := GetMaybeCloudUserAndInstanceID()
maybeCloudUserId, maybeCloudInstanceId := metrics_cloud_user_instance_id_helper.GetMaybeCloudUserAndInstanceID()

logger := logrus.StandardLogger()
metricsClient, metricsClientCloseFunc, err := metrics_client.CreateMetricsClient(
Expand All @@ -97,22 +97,6 @@ func GetSegmentClient() (metrics_client.MetricsClient, func() error, error) {
return metricsClient, metricsClientCloseFunc, nil
}

func GetMaybeCloudUserAndInstanceID() (metrics_client.CloudUserID, metrics_client.CloudInstanceID) {
cloudUserId := ""
cloudInstanceId := ""
currentContext, err := store.GetContextsConfigStore().GetCurrentContext()
if err != nil {
logrus.Debugf("Could not retrieve the current context. Kurtosis will assume context is local and not pass empty values to the metrics client")
logrus.Debugf("Error was: %v", err.Error())
} else {
if store.IsRemote(currentContext) {
cloudUserId = currentContext.GetRemoteContextV0().GetCloudUserId()
cloudInstanceId = currentContext.GetRemoteContextV0().GetCloudInstanceId()
}
}
return metrics_client.CloudUserID(cloudUserId), metrics_client.CloudInstanceID(cloudInstanceId)
}

func getMetricsUserIdAndClusterType() (string, string, error) {
clusterSettingStore := kurtosis_cluster_setting.GetKurtosisClusterSettingStore()
isClusterSet, err := clusterSettingStore.HasClusterSetting()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package metrics_cloud_user_instance_id_helper

import (
"github.com/kurtosis-tech/kurtosis/contexts-config-store/store"
"github.com/kurtosis-tech/kurtosis/metrics-library/golang/lib/metrics_client"
"github.com/sirupsen/logrus"
)

func GetMaybeCloudUserAndInstanceID() (metrics_client.CloudUserID, metrics_client.CloudInstanceID) {
cloudUserId := ""
cloudInstanceId := ""
currentContext, err := store.GetContextsConfigStore().GetCurrentContext()
if err != nil {
logrus.Debugf("Could not retrieve the current context")
logrus.Debugf("Error was: %v", err.Error())
} else {
if store.IsRemote(currentContext) {
cloudUserId = currentContext.GetRemoteContextV0().GetCloudUserId()
cloudInstanceId = currentContext.GetRemoteContextV0().GetCloudInstanceId()
}
}
return metrics_client.CloudUserID(cloudUserId), metrics_client.CloudInstanceID(cloudInstanceId)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package user_send_metrics_election

import (
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_cloud_user_instance_id_helper"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_user_id_store"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/user_send_metrics_election/user_metrics_election_event_backlog"
"github.com/kurtosis-tech/kurtosis/cli/cli/kurtosis_cluster_setting"
Expand Down Expand Up @@ -41,6 +42,8 @@ func SendAnyBackloggedUserMetricsElectionEvent() error {
}
}

maybeCloudUserID, maybeCloudInstanceID := metrics_cloud_user_instance_id_helper.GetMaybeCloudUserAndInstanceID()

if hasBackloggedEvent {
metricsUserIdStore := metrics_user_id_store.GetMetricsUserIDStore()

Expand All @@ -64,6 +67,8 @@ func SendAnyBackloggedUserMetricsElectionEvent() error {
metricsClientCallback,
analytics_logger.ConvertLogrusLoggerToAnalyticsLogger(logger),
metrics_client.IsCI(),
maybeCloudUserID,
maybeCloudInstanceID,
),
)
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion cli/cli/kurtosis_config/email_collector/email_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package email_collector

import (
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/do_nothing_metrics_client_callback"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_cloud_user_instance_id_helper"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/metrics_user_id_store"
"github.com/kurtosis-tech/kurtosis/cli/cli/helpers/prompt_displayer"
"github.com/kurtosis-tech/kurtosis/cli/cli/kurtosis_config/resolved_config"
Expand Down Expand Up @@ -41,6 +42,8 @@ func logUserEmailAddressAsMetric(userEmail string) {
}
logger := logrus.StandardLogger()

maybeCloudUserID, maybeCloudInstanceID := metrics_cloud_user_instance_id_helper.GetMaybeCloudUserAndInstanceID()

metricsClient, metricsClientCloseFunc, err := metrics_client.CreateMetricsClient(
metrics_client.NewMetricsClientCreatorOption(
source.KurtosisCLISource,
Expand All @@ -53,7 +56,9 @@ func logUserEmailAddressAsMetric(userEmail string) {
flushQueueOnEachEvent,
do_nothing_metrics_client_callback.NewDoNothingMetricsClientCallback(),
analytics_logger.ConvertLogrusLoggerToAnalyticsLogger(logger),
metrics_client.IsCI()),
metrics_client.IsCI(),
maybeCloudUserID,
maybeCloudInstanceID),
)
if err != nil {
logrus.Debugf("tried creating a metrics client but failed with error:\n%v", err)
Expand Down
15 changes: 15 additions & 0 deletions core/launcher/args/api_container_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ type APIContainerArgs struct {
CloudInstanceID metrics_client.CloudInstanceID `json:"cloud_instance_id"`
}

var skipValidation = []string{"cloud_user_id", "cloud_instance_id"}

func (args *APIContainerArgs) UnmarshalJSON(data []byte) error {
// create a mirror type to avoid unmarshalling infinitely https://stackoverflow.com/questions/52433467/how-to-call-json-unmarshal-inside-unmarshaljson-without-causing-stack-overflow
type APIContainerArgsMirror APIContainerArgs
Expand Down Expand Up @@ -138,6 +140,10 @@ func (args APIContainerArgs) validate() error {
field := reflectValType.Field(i)
jsonFieldName := field.Tag.Get(jsonFieldTag)

if contains(skipValidation, jsonFieldName) {
continue
}

// Ensure no empty strings
strVal := reflectVal.Field(i).String()
if strings.TrimSpace(strVal) == "" {
Expand All @@ -146,3 +152,12 @@ func (args APIContainerArgs) validate() error {
}
return nil
}

func contains(slice []string, value string) bool {
for _, item := range slice {
if item == value {
return true
}
}
return false
}
15 changes: 15 additions & 0 deletions engine/launcher/args/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ type EngineServerArgs struct {
CloudInstanceID metrics_client.CloudInstanceID `json:"cloud_instance_id"`
}

var skipValidation = []string{"cloud_user_id", "cloud_instance_id"}

func (args *EngineServerArgs) UnmarshalJSON(data []byte) error {
// create a mirror type to avoid unmarshalling infinitely https://stackoverflow.com/questions/52433467/how-to-call-json-unmarshal-inside-unmarshaljson-without-causing-stack-overflow
type EngineServerArgsMirror EngineServerArgs
Expand Down Expand Up @@ -139,6 +141,10 @@ func (args EngineServerArgs) validate() error {
field := reflectValType.Field(i)
jsonFieldName := field.Tag.Get(jsonFieldTag)

if contains(skipValidation, jsonFieldName) {
continue
}

// Ensure no empty strings
strVal := reflectVal.Field(i).String()
if strings.TrimSpace(strVal) == "" {
Expand All @@ -147,3 +153,12 @@ func (args EngineServerArgs) validate() error {
}
return nil
}

func contains(slice []string, value string) bool {
for _, item := range slice {
if item == value {
return true
}
}
return false
}
8 changes: 8 additions & 0 deletions engine/server/engine/enclave_manager/enclave_creator.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ func (creator *EnclaveCreator) CreateEnclave(
metricsUserID,
didUserAcceptSendingMetrics,
isCI,
cloudUserID,
cloudInstanceID,
)

if err != nil {
Expand Down Expand Up @@ -169,6 +171,8 @@ func (creator *EnclaveCreator) launchApiContainer(
metricsUserID string,
didUserAcceptSendingMetrics bool,
isCI bool,
cloudUserID metrics_client.CloudUserID,
cloudInstanceID metrics_client.CloudInstanceID,
) (
resultApiContainer *api_container.APIContainer,
resultErr error,
Expand All @@ -189,6 +193,8 @@ func (creator *EnclaveCreator) launchApiContainer(
metricsUserID,
didUserAcceptSendingMetrics,
isCI,
cloudUserID,
cloudInstanceID,
)
if err != nil {
return nil, stacktrace.Propagate(err, "Expected to be able to launch api container for enclave '%v' with custom version '%v', but an error occurred", enclaveUuid, apiContainerImageVersionTag)
Expand All @@ -206,6 +212,8 @@ func (creator *EnclaveCreator) launchApiContainer(
metricsUserID,
didUserAcceptSendingMetrics,
isCI,
cloudUserID,
cloudInstanceID,
)
if err != nil {
return nil, stacktrace.Propagate(err, "Expected to be able to launch api container for enclave '%v' with the default version, but an error occurred", enclaveUuid)
Expand Down
2 changes: 1 addition & 1 deletion engine/server/engine/enclave_manager/enclave_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func CreateEnclaveManager(

// The enclave pool feature is only available for Kubernetes so far
if kurtosisBackendType == args.KurtosisBackendType_Kubernetes {
enclavePool, err = CreateEnclavePool(kurtosisBackend, enclaveCreator, poolSize, engineVersion, enclaveEnvVars, metricsUserID, didUserAcceptSendingMetrics, isCI)
enclavePool, err = CreateEnclavePool(kurtosisBackend, enclaveCreator, poolSize, engineVersion, enclaveEnvVars, metricsUserID, didUserAcceptSendingMetrics, isCI, cloudUserID, cloudInstanceID)
if err != nil {
return nil, stacktrace.Propagate(err, "An error occurred creating enclave pool with pool-size '%v' and engine version '%v'", poolSize, engineVersion)
}
Expand Down
4 changes: 4 additions & 0 deletions engine/server/engine/enclave_manager/enclave_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ func CreateEnclavePool(
metricsUserID string,
didUserAcceptSendingMetrics bool,
isCI bool,
cloudUserID metrics_client.CloudUserID,
cloudInstanceID metrics_client.CloudInstanceID,

) (*EnclavePool, error) {

Expand Down Expand Up @@ -98,6 +100,8 @@ func CreateEnclavePool(
metricsUserID: metricsUserID,
didUserAcceptSendingMetrics: didUserAcceptSendingMetrics,
isCI: isCI,
cloudUserID: cloudUserID,
cloudInstanceID: cloudInstanceID,
}

go enclavePool.run(ctxWithCancel)
Expand Down

0 comments on commit 3af63fd

Please sign in to comment.