From 9274e5c1bb6e57b78c2692f130157e67cef60504 Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 11:03:06 +0000 Subject: [PATCH 1/7] updated metrics library --- cli/cli/go.mod | 2 +- cli/cli/go.sum | 4 ++-- core/server/go.mod | 2 +- core/server/go.sum | 4 ++-- engine/server/go.mod | 2 +- engine/server/go.sum | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/cli/go.mod b/cli/cli/go.mod index 4bb95882ba..08a68dc814 100644 --- a/cli/cli/go.mod +++ b/cli/cli/go.mod @@ -20,7 +20,7 @@ require ( github.com/kurtosis-tech/kurtosis/container-engine-lib v0.0.0 // local dependency github.com/kurtosis-tech/kurtosis/engine/launcher v0.0.0 // local dependency github.com/kurtosis-tech/kurtosis/kurtosis_version v0.0.0 // Local dependency generated during build - github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e + github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570 github.com/kurtosis-tech/stacktrace v0.0.0-20211028211901-1c67a77b5409 github.com/manifoldco/promptui v0.9.0 github.com/mattn/go-isatty v0.0.14 diff --git a/cli/cli/go.sum b/cli/cli/go.sum index e6583fc1db..8b12f15a06 100644 --- a/cli/cli/go.sum +++ b/cli/cli/go.sum @@ -291,10 +291,10 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kurtosis-tech/free-ip-addr-tracker-lib v0.0.0-20211106222342-1f73d028840d h1:IWFwJfg2EhA/9Anv+VyG3LSTEZ/TZL30tUaVCFrzWK0= github.com/kurtosis-tech/free-ip-addr-tracker-lib v0.0.0-20211106222342-1f73d028840d/go.mod h1:1PoW/0l4K80JRSj7A13aONsnaTdTM+dM36DIfDbYmVs= -github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230214135833-e362c2ee8600 h1:8GZVRAr16u7WCy/5xNsjZIgEcslh02cHCa3AzBSa7ME= -github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230214135833-e362c2ee8600/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e h1:VSoT53oeS53dGr28LtVVbwihdcRHQShsufbQjyHm/5M= github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= +github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570 h1:2d8FxmA36Heb0uNziPof0Tqi70VhHr7H1cfoYY6IB9M= +github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= github.com/kurtosis-tech/stacktrace v0.0.0-20211028211901-1c67a77b5409 h1:YQTATifMUwZEtZYb0LVA7DK2pj8s71iY8rzweuUQ5+g= github.com/kurtosis-tech/stacktrace v0.0.0-20211028211901-1c67a77b5409/go.mod h1:y5weVs5d9wXXHcDA1awRxkIhhHC1xxYJN8a7aXnE6S8= github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= diff --git a/core/server/go.mod b/core/server/go.mod index 4dce179fbe..ba8a8223f5 100644 --- a/core/server/go.mod +++ b/core/server/go.mod @@ -29,7 +29,7 @@ require ( github.com/cenkalti/backoff/v4 v4.2.0 github.com/go-git/go-git/v5 v5.4.2 github.com/itchyny/gojq v0.12.9 - github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e + github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570 github.com/mholt/archiver v3.1.1+incompatible github.com/pkg/errors v0.9.1 go.etcd.io/bbolt v1.3.6 diff --git a/core/server/go.sum b/core/server/go.sum index 64a210888e..7ecbceaf3e 100644 --- a/core/server/go.sum +++ b/core/server/go.sum @@ -139,10 +139,10 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kurtosis-tech/free-ip-addr-tracker-lib v0.0.0-20211106222342-d3be9e82993e h1:CDNMjCNCvvCPSpoakxYZ3IHjBsxo9rLiMyjgvPYL87Q= github.com/kurtosis-tech/free-ip-addr-tracker-lib v0.0.0-20211106222342-d3be9e82993e/go.mod h1:1PoW/0l4K80JRSj7A13aONsnaTdTM+dM36DIfDbYmVs= -github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230214135833-e362c2ee8600 h1:8GZVRAr16u7WCy/5xNsjZIgEcslh02cHCa3AzBSa7ME= -github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230214135833-e362c2ee8600/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e h1:VSoT53oeS53dGr28LtVVbwihdcRHQShsufbQjyHm/5M= github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= +github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570 h1:2d8FxmA36Heb0uNziPof0Tqi70VhHr7H1cfoYY6IB9M= +github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= github.com/kurtosis-tech/minimal-grpc-server/golang v0.0.0-20211201000847-a204edc5a0b3 h1:Begoh17x/9rLWQLM64A8oINjoQk1DFKN2iwhQf1i0Rc= github.com/kurtosis-tech/minimal-grpc-server/golang v0.0.0-20211201000847-a204edc5a0b3/go.mod h1:Y0O+lt256iWpkdjDOB8r6csF96Pt3JVIFMbUuWIvr8o= github.com/kurtosis-tech/stacktrace v0.0.0-20211028211901-1c67a77b5409 h1:YQTATifMUwZEtZYb0LVA7DK2pj8s71iY8rzweuUQ5+g= diff --git a/engine/server/go.mod b/engine/server/go.mod index fbe80494c7..2266942c62 100644 --- a/engine/server/go.mod +++ b/engine/server/go.mod @@ -15,7 +15,7 @@ require ( github.com/kurtosis-tech/kurtosis/container-engine-lib v0.0.0 // local dependency github.com/kurtosis-tech/kurtosis/core/launcher v0.0.0 // local dependency github.com/kurtosis-tech/kurtosis/engine/launcher v0.0.0 - github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e + github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570 github.com/kurtosis-tech/minimal-grpc-server/golang v0.0.0-20211201000847-a204edc5a0b3 github.com/kurtosis-tech/stacktrace v0.0.0-20211028211901-1c67a77b5409 github.com/sirupsen/logrus v1.8.1 diff --git a/engine/server/go.sum b/engine/server/go.sum index 7102c7746b..0c8ca14794 100644 --- a/engine/server/go.sum +++ b/engine/server/go.sum @@ -100,10 +100,10 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kurtosis-tech/free-ip-addr-tracker-lib v0.0.0-20211106222342-1f73d028840d h1:IWFwJfg2EhA/9Anv+VyG3LSTEZ/TZL30tUaVCFrzWK0= github.com/kurtosis-tech/free-ip-addr-tracker-lib v0.0.0-20211106222342-1f73d028840d/go.mod h1:1PoW/0l4K80JRSj7A13aONsnaTdTM+dM36DIfDbYmVs= -github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230214135833-e362c2ee8600 h1:8GZVRAr16u7WCy/5xNsjZIgEcslh02cHCa3AzBSa7ME= -github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230214135833-e362c2ee8600/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e h1:VSoT53oeS53dGr28LtVVbwihdcRHQShsufbQjyHm/5M= github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230215143841-da53ad89218e/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= +github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570 h1:2d8FxmA36Heb0uNziPof0Tqi70VhHr7H1cfoYY6IB9M= +github.com/kurtosis-tech/metrics-library/golang v0.0.0-20230216101811-98b2623f2570/go.mod h1:tteWV+M47xMHxqCIPQmdmgPW80rhN8YfzrgRRWbQhOw= github.com/kurtosis-tech/minimal-grpc-server/golang v0.0.0-20211201000847-a204edc5a0b3 h1:Begoh17x/9rLWQLM64A8oINjoQk1DFKN2iwhQf1i0Rc= github.com/kurtosis-tech/minimal-grpc-server/golang v0.0.0-20211201000847-a204edc5a0b3/go.mod h1:Y0O+lt256iWpkdjDOB8r6csF96Pt3JVIFMbUuWIvr8o= github.com/kurtosis-tech/stacktrace v0.0.0-20211028211901-1c67a77b5409 h1:YQTATifMUwZEtZYb0LVA7DK2pj8s71iY8rzweuUQ5+g= From bcf56fe2932a0d479d98abfd71c032bf6f4852f3 Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 11:47:31 +0000 Subject: [PATCH 2/7] this seems a lot cleaner --- .../server/api_container_service.go | 2 +- .../startosis_engine/startosis_executor.go | 20 ++++++++++++++++--- .../startosis_executor_test.go | 19 +++++++++++++++++- .../startosis_engine/startosis_runner.go | 8 +++++--- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/core/server/api_container/server/api_container_service.go b/core/server/api_container/server/api_container_service.go index 2ab97e3bcd..a0c4f1e67f 100644 --- a/core/server/api_container/server/api_container_service.go +++ b/core/server/api_container/server/api_container_service.go @@ -672,7 +672,7 @@ func (apicService ApiContainerService) runStarlarkPackageSetup(packageId string, } func (apicService ApiContainerService) runStarlark(parallelism int, dryRun bool, packageId string, serializedStarlark string, serializedParams string, stream grpc.ServerStream) { - responseLineStream := apicService.startosisRunner.Run(stream.Context(), dryRun, parallelism, packageId, serializedStarlark, serializedParams) + responseLineStream := apicService.startosisRunner.Run(stream.Context(), dryRun, parallelism, packageId, serializedStarlark, serializedParams, apicService.metricsClient, apicService.serviceNetwork) for { select { case <-stream.Context().Done(): diff --git a/core/server/api_container/server/startosis_engine/startosis_executor.go b/core/server/api_container/server/startosis_engine/startosis_executor.go index 3252f85540..8ad99fb2a9 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor.go @@ -4,14 +4,19 @@ import ( "context" "github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings" "github.com/kurtosis-tech/kurtosis/api/golang/core/lib/binding_constructors" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction" + metrics_client "github.com/kurtosis-tech/metrics-library/golang/lib/client" "github.com/kurtosis-tech/stacktrace" + "github.com/sirupsen/logrus" "sync" ) const ( - progressMsg = "Execution in progress" - ParallelismParam = "PARALLELISM" + progressMsg = "Execution in progress" + ParallelismParam = "PARALLELISM" + executionFinishedSuccessfully = true + executionFailed = false ) type StartosisExecutor struct { @@ -35,7 +40,7 @@ func NewStartosisExecutor() *StartosisExecutor { // - A regular KurtosisInstruction that was successfully executed // - A KurtosisExecutionError if the execution failed // - A ProgressInfo to update the current "state" of the execution -func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, parallelism int, instructions []kurtosis_instruction.KurtosisInstruction, serializedScriptOutput string) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { +func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, parallelism int, instructions []kurtosis_instruction.KurtosisInstruction, serializedScriptOutput string, packageId string, metricsClient metrics_client.MetricsClient, serviceNetwork service_network.ServiceNetwork) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { executor.mutex.Lock() starlarkRunResponseLineStream := make(chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) ctxWithParallelism := context.WithValue(ctx, ParallelismParam, parallelism) @@ -60,6 +65,11 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par if err != nil { propagatedError := stacktrace.Propagate(err, "An error occurred executing instruction (number %d): \n%v", instructionNumber, instruction.String()) serializedError := binding_constructors.NewStarlarkExecutionError(propagatedError.Error()) + numServices := len(serviceNetwork.GetServiceNames()) + starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) + if err := metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFailed); err != nil { + logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) + } starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromExecutionError(serializedError) starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunFailureEvent() return @@ -71,7 +81,11 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par } // TODO(gb): we should run magic string replacement on the output + numServices := len(serviceNetwork.GetServiceNames()) starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) + if err := metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFinishedSuccessfully); err != nil { + logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) + } }() return starlarkRunResponseLineStream } diff --git a/core/server/api_container/server/startosis_engine/startosis_executor_test.go b/core/server/api_container/server/startosis_engine/startosis_executor_test.go index 662e5eeb20..0fe2252525 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor_test.go @@ -5,8 +5,12 @@ import ( "errors" "github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings" "github.com/kurtosis-tech/kurtosis/api/golang/core/lib/binding_constructors" + "github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_interface/objects/service" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction" "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/startosis_engine/kurtosis_instruction/mock_instruction" + metrics_client "github.com/kurtosis-tech/metrics-library/golang/lib/client" + "github.com/kurtosis-tech/metrics-library/golang/lib/source" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "strings" @@ -22,8 +26,15 @@ const ( noScriptOutputObject = "" noParallelism = 1 + + dummyPackageIdForTesting = "testing-package" ) +type doNothingMetricsClientCallback struct{} + +func (d doNothingMetricsClientCallback) Success() {} +func (d doNothingMetricsClientCallback) Failure(err error) {} + var ( dummyPosition = kurtosis_instruction.NewInstructionPosition(12, 1, "dummyFile") noInstructionArgsForTesting []*kurtosis_core_rpc_api_bindings.StarlarkInstructionArg @@ -139,7 +150,13 @@ func createMockInstruction(t *testing.T, instructionName string, executeSuccessf func executeSynchronously(t *testing.T, executor *StartosisExecutor, dryRun bool, instructions []kurtosis_instruction.KurtosisInstruction) (string, []*kurtosis_core_rpc_api_bindings.StarlarkInstruction, *kurtosis_core_rpc_api_bindings.StarlarkExecutionError) { scriptOutput := strings.Builder{} var serializedInstructions []*kurtosis_core_rpc_api_bindings.StarlarkInstruction - executionResponseLines := executor.Execute(context.Background(), dryRun, noParallelism, instructions, noScriptOutputObject) + mockServiceNetwork := service_network.NewMockServiceNetwork(t) + mockServiceNetwork.EXPECT().GetServiceNames().Times(1).Return(map[service.ServiceName]bool{}) + doNothingMetricsClient, metricsClientCloser, metricsClientCreationError := metrics_client.CreateMetricsClient(source.KurtosisCoreSource, "", "", "", false, false, doNothingMetricsClientCallback{}) + require.Nil(t, metricsClientCreationError) + defer metricsClientCloser() + + executionResponseLines := executor.Execute(context.Background(), dryRun, noParallelism, instructions, noScriptOutputObject, dummyPackageIdForTesting, doNothingMetricsClient, mockServiceNetwork) for executionResponseLine := range executionResponseLines { if executionResponseLine.GetError() != nil { return scriptOutput.String(), serializedInstructions, executionResponseLine.GetError().GetExecutionError() diff --git a/core/server/api_container/server/startosis_engine/startosis_runner.go b/core/server/api_container/server/startosis_engine/startosis_runner.go index f3d9353a24..77b90824ab 100644 --- a/core/server/api_container/server/startosis_engine/startosis_runner.go +++ b/core/server/api_container/server/startosis_engine/startosis_runner.go @@ -4,6 +4,8 @@ import ( "context" "github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings" "github.com/kurtosis-tech/kurtosis/api/golang/core/lib/binding_constructors" + "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network" + metrics_client "github.com/kurtosis-tech/metrics-library/golang/lib/client" "github.com/sirupsen/logrus" ) @@ -31,7 +33,7 @@ func NewStartosisRunner(interpreter *StartosisInterpreter, validator *StartosisV } } -func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism int, moduleId string, serializedStartosis string, serializedParams string) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { +func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism int, packageId string, serializedStartosis string, serializedParams string, metricsClient metrics_client.MetricsClient, serviceNetwork service_network.ServiceNetwork) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { // TODO(gb): add metric tracking maybe? starlarkRunResponseLines := make(chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) @@ -43,7 +45,7 @@ func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism startingInterpretationMsg, defaultCurrentStepNumber, defaultTotalStepsNumber) starlarkRunResponseLines <- progressInfo - serializedScriptOutput, instructionsList, interpretationError := runner.startosisInterpreter.Interpret(ctx, moduleId, serializedStartosis, serializedParams) + serializedScriptOutput, instructionsList, interpretationError := runner.startosisInterpreter.Interpret(ctx, packageId, serializedStartosis, serializedParams) if interpretationError != nil { starlarkRunResponseLines <- binding_constructors.NewStarlarkRunResponseLineFromInterpretationError(interpretationError) starlarkRunResponseLines <- binding_constructors.NewStarlarkRunResponseLineFromRunFailureEvent() @@ -69,7 +71,7 @@ func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism startingExecutionMsg, defaultCurrentStepNumber, totalNumberOfInstructions) starlarkRunResponseLines <- progressInfo - executionResponseLinesChan := runner.startosisExecutor.Execute(ctx, dryRun, parallelism, instructionsList, serializedScriptOutput) + executionResponseLinesChan := runner.startosisExecutor.Execute(ctx, dryRun, parallelism, instructionsList, serializedScriptOutput, packageId, metricsClient, serviceNetwork) if isRunFinished := forwardKurtosisResponseLineChannelUntilSourceIsClosed(executionResponseLinesChan, starlarkRunResponseLines); !isRunFinished { logrus.Warnf("Execution finished but no 'RunFinishedEvent' was received through the stream. This is unexpected as every execution should be terminal.") } From 6afc4610ca8a086cb6df082e11f4c2d803f1487d Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 11:55:49 +0000 Subject: [PATCH 3/7] thanks linter --- .../server/startosis_engine/startosis_executor_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_executor_test.go b/core/server/api_container/server/startosis_engine/startosis_executor_test.go index 0fe2252525..a7e30ee74b 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor_test.go @@ -154,7 +154,10 @@ func executeSynchronously(t *testing.T, executor *StartosisExecutor, dryRun bool mockServiceNetwork.EXPECT().GetServiceNames().Times(1).Return(map[service.ServiceName]bool{}) doNothingMetricsClient, metricsClientCloser, metricsClientCreationError := metrics_client.CreateMetricsClient(source.KurtosisCoreSource, "", "", "", false, false, doNothingMetricsClientCallback{}) require.Nil(t, metricsClientCreationError) - defer metricsClientCloser() + defer func() { + err := metricsClientCloser() + require.Nil(t, err) + }() executionResponseLines := executor.Execute(context.Background(), dryRun, noParallelism, instructions, noScriptOutputObject, dummyPackageIdForTesting, doNothingMetricsClient, mockServiceNetwork) for executionResponseLine := range executionResponseLines { From 69dcf646908977ec4c77ea3c3b5404433b20640a Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 12:03:48 +0000 Subject: [PATCH 4/7] some fixes --- .../server/startosis_engine/startosis_executor.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_executor.go b/core/server/api_container/server/startosis_engine/startosis_executor.go index 8ad99fb2a9..b296e7ac63 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor.go @@ -66,7 +66,6 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par propagatedError := stacktrace.Propagate(err, "An error occurred executing instruction (number %d): \n%v", instructionNumber, instruction.String()) serializedError := binding_constructors.NewStarlarkExecutionError(propagatedError.Error()) numServices := len(serviceNetwork.GetServiceNames()) - starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) if err := metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFailed); err != nil { logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) } @@ -82,10 +81,10 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par // TODO(gb): we should run magic string replacement on the output numServices := len(serviceNetwork.GetServiceNames()) - starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) if err := metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFinishedSuccessfully); err != nil { logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) } + starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) }() return starlarkRunResponseLineStream } From 921d3ec466519baa43bad905c412ae2212b08814 Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 12:08:00 +0000 Subject: [PATCH 5/7] use named constants --- .../server/startosis_engine/startosis_executor_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_executor_test.go b/core/server/api_container/server/startosis_engine/startosis_executor_test.go index a7e30ee74b..c874c87323 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor_test.go @@ -27,7 +27,12 @@ const ( noScriptOutputObject = "" noParallelism = 1 - dummyPackageIdForTesting = "testing-package" + dummyPackageIdForTesting = "testing-package" + sourceVersionForTesting = "" + metricsUserIdForTesting = "" + backendTypeForTesting = "" + dontSendMetrics = false + dontFlushEnqueuedMetricsOnEachEvent = false ) type doNothingMetricsClientCallback struct{} @@ -152,7 +157,7 @@ func executeSynchronously(t *testing.T, executor *StartosisExecutor, dryRun bool var serializedInstructions []*kurtosis_core_rpc_api_bindings.StarlarkInstruction mockServiceNetwork := service_network.NewMockServiceNetwork(t) mockServiceNetwork.EXPECT().GetServiceNames().Times(1).Return(map[service.ServiceName]bool{}) - doNothingMetricsClient, metricsClientCloser, metricsClientCreationError := metrics_client.CreateMetricsClient(source.KurtosisCoreSource, "", "", "", false, false, doNothingMetricsClientCallback{}) + doNothingMetricsClient, metricsClientCloser, metricsClientCreationError := metrics_client.CreateMetricsClient(source.KurtosisCoreSource, sourceVersionForTesting, metricsUserIdForTesting, backendTypeForTesting, dontSendMetrics, dontFlushEnqueuedMetricsOnEachEvent, doNothingMetricsClientCallback{}) require.Nil(t, metricsClientCreationError) defer func() { err := metricsClientCloser() From 9d5bd8283b7049d18f67b3c4b81ae240153e16f9 Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 14:40:34 +0000 Subject: [PATCH 6/7] refactor --- core/server/api_container/main.go | 2 +- .../server/api_container_service.go | 2 +- .../startosis_engine/startosis_executor.go | 20 ++++++---- .../startosis_executor_test.go | 38 +++++++++++++------ .../startosis_engine/startosis_runner.go | 6 +-- 5 files changed, 42 insertions(+), 26 deletions(-) diff --git a/core/server/api_container/main.go b/core/server/api_container/main.go index 3d17719b6f..ba7d8c4be6 100644 --- a/core/server/api_container/main.go +++ b/core/server/api_container/main.go @@ -201,7 +201,7 @@ func runMain() error { startosisRunner := startosis_engine.NewStartosisRunner( startosis_engine.NewStartosisInterpreter(serviceNetwork, gitPackageContentProvider, runtime_value_store.NewRuntimeValueStore()), startosis_engine.NewStartosisValidator(&kurtosisBackend, serviceNetwork, filesArtifactStore), - startosis_engine.NewStartosisExecutor()) + startosis_engine.NewStartosisExecutor(metricsClient, serviceNetwork)) //Creation of ApiContainerService apiContainerService, err := server.NewApiContainerService( diff --git a/core/server/api_container/server/api_container_service.go b/core/server/api_container/server/api_container_service.go index a0c4f1e67f..2ab97e3bcd 100644 --- a/core/server/api_container/server/api_container_service.go +++ b/core/server/api_container/server/api_container_service.go @@ -672,7 +672,7 @@ func (apicService ApiContainerService) runStarlarkPackageSetup(packageId string, } func (apicService ApiContainerService) runStarlark(parallelism int, dryRun bool, packageId string, serializedStarlark string, serializedParams string, stream grpc.ServerStream) { - responseLineStream := apicService.startosisRunner.Run(stream.Context(), dryRun, parallelism, packageId, serializedStarlark, serializedParams, apicService.metricsClient, apicService.serviceNetwork) + responseLineStream := apicService.startosisRunner.Run(stream.Context(), dryRun, parallelism, packageId, serializedStarlark, serializedParams) for { select { case <-stream.Context().Done(): diff --git a/core/server/api_container/server/startosis_engine/startosis_executor.go b/core/server/api_container/server/startosis_engine/startosis_executor.go index b296e7ac63..e9d5ab9b39 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor.go @@ -20,16 +20,20 @@ const ( ) type StartosisExecutor struct { - mutex *sync.Mutex + mutex *sync.Mutex + metricsClient metrics_client.MetricsClient + serviceNetwork service_network.ServiceNetwork } type ExecutionError struct { Error string } -func NewStartosisExecutor() *StartosisExecutor { +func NewStartosisExecutor(metricsClient metrics_client.MetricsClient, serviceNetwork service_network.ServiceNetwork) *StartosisExecutor { return &StartosisExecutor{ - mutex: &sync.Mutex{}, + mutex: &sync.Mutex{}, + metricsClient: metricsClient, + serviceNetwork: serviceNetwork, } } @@ -40,7 +44,7 @@ func NewStartosisExecutor() *StartosisExecutor { // - A regular KurtosisInstruction that was successfully executed // - A KurtosisExecutionError if the execution failed // - A ProgressInfo to update the current "state" of the execution -func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, parallelism int, instructions []kurtosis_instruction.KurtosisInstruction, serializedScriptOutput string, packageId string, metricsClient metrics_client.MetricsClient, serviceNetwork service_network.ServiceNetwork) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { +func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, parallelism int, instructions []kurtosis_instruction.KurtosisInstruction, serializedScriptOutput string, packageId string) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { executor.mutex.Lock() starlarkRunResponseLineStream := make(chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) ctxWithParallelism := context.WithValue(ctx, ParallelismParam, parallelism) @@ -65,8 +69,8 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par if err != nil { propagatedError := stacktrace.Propagate(err, "An error occurred executing instruction (number %d): \n%v", instructionNumber, instruction.String()) serializedError := binding_constructors.NewStarlarkExecutionError(propagatedError.Error()) - numServices := len(serviceNetwork.GetServiceNames()) - if err := metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFailed); err != nil { + numServices := len(executor.serviceNetwork.GetServiceNames()) + if err := executor.metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFailed); err != nil { logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) } starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromExecutionError(serializedError) @@ -80,8 +84,8 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par } // TODO(gb): we should run magic string replacement on the output - numServices := len(serviceNetwork.GetServiceNames()) - if err := metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFinishedSuccessfully); err != nil { + numServices := len(executor.serviceNetwork.GetServiceNames()) + if err := executor.metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFinishedSuccessfully); err != nil { logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) } starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) diff --git a/core/server/api_container/server/startosis_engine/startosis_executor_test.go b/core/server/api_container/server/startosis_engine/startosis_executor_test.go index c874c87323..17195ea56d 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor_test.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor_test.go @@ -46,7 +46,12 @@ var ( ) func TestExecuteKurtosisInstructions_ExecuteForReal_Success(t *testing.T) { - executor := NewStartosisExecutor() + + executor, metricsClientCloser := newMockStartosisExecutorForTesting(t) + defer func() { + err := metricsClientCloser() + require.Nil(t, err) + }() instruction1 := createMockInstruction(t, "instruction1", executeSuccessfully) instruction2 := createMockInstruction(t, "instruction2", executeSuccessfully) @@ -71,7 +76,11 @@ func TestExecuteKurtosisInstructions_ExecuteForReal_Success(t *testing.T) { } func TestExecuteKurtosisInstructions_ExecuteForReal_FailureHalfWay(t *testing.T) { - executor := NewStartosisExecutor() + executor, metricsClientCloser := newMockStartosisExecutorForTesting(t) + defer func() { + err := metricsClientCloser() + require.Nil(t, err) + }() instruction1 := createMockInstruction(t, "instruction1", executeSuccessfully) instruction2 := createMockInstruction(t, "instruction2", throwOnExecute) @@ -108,7 +117,11 @@ instruction2() } func TestExecuteKurtosisInstructions_DoDryRun(t *testing.T) { - executor := NewStartosisExecutor() + executor, metricsClientCloser := newMockStartosisExecutorForTesting(t) + defer func() { + err := metricsClientCloser() + require.Nil(t, err) + }() instruction1 := createMockInstruction(t, "instruction1", executeSuccessfully) instruction2 := createMockInstruction(t, "instruction2", executeSuccessfully) @@ -155,16 +168,8 @@ func createMockInstruction(t *testing.T, instructionName string, executeSuccessf func executeSynchronously(t *testing.T, executor *StartosisExecutor, dryRun bool, instructions []kurtosis_instruction.KurtosisInstruction) (string, []*kurtosis_core_rpc_api_bindings.StarlarkInstruction, *kurtosis_core_rpc_api_bindings.StarlarkExecutionError) { scriptOutput := strings.Builder{} var serializedInstructions []*kurtosis_core_rpc_api_bindings.StarlarkInstruction - mockServiceNetwork := service_network.NewMockServiceNetwork(t) - mockServiceNetwork.EXPECT().GetServiceNames().Times(1).Return(map[service.ServiceName]bool{}) - doNothingMetricsClient, metricsClientCloser, metricsClientCreationError := metrics_client.CreateMetricsClient(source.KurtosisCoreSource, sourceVersionForTesting, metricsUserIdForTesting, backendTypeForTesting, dontSendMetrics, dontFlushEnqueuedMetricsOnEachEvent, doNothingMetricsClientCallback{}) - require.Nil(t, metricsClientCreationError) - defer func() { - err := metricsClientCloser() - require.Nil(t, err) - }() - executionResponseLines := executor.Execute(context.Background(), dryRun, noParallelism, instructions, noScriptOutputObject, dummyPackageIdForTesting, doNothingMetricsClient, mockServiceNetwork) + executionResponseLines := executor.Execute(context.Background(), dryRun, noParallelism, instructions, noScriptOutputObject, dummyPackageIdForTesting) for executionResponseLine := range executionResponseLines { if executionResponseLine.GetError() != nil { return scriptOutput.String(), serializedInstructions, executionResponseLine.GetError().GetExecutionError() @@ -181,3 +186,12 @@ func executeSynchronously(t *testing.T, executor *StartosisExecutor, dryRun bool } return scriptOutput.String(), serializedInstructions, nil } + +func newMockStartosisExecutorForTesting(t *testing.T) (*StartosisExecutor, func() error) { + mockServiceNetwork := service_network.NewMockServiceNetwork(t) + mockServiceNetwork.EXPECT().GetServiceNames().Times(1).Return(map[service.ServiceName]bool{}) + doNothingMetricsClient, metricsClientCloser, metricsClientCreationError := metrics_client.CreateMetricsClient(source.KurtosisCoreSource, sourceVersionForTesting, metricsUserIdForTesting, backendTypeForTesting, dontSendMetrics, dontFlushEnqueuedMetricsOnEachEvent, doNothingMetricsClientCallback{}) + require.Nil(t, metricsClientCreationError) + executor := NewStartosisExecutor(doNothingMetricsClient, mockServiceNetwork) + return executor, metricsClientCloser +} diff --git a/core/server/api_container/server/startosis_engine/startosis_runner.go b/core/server/api_container/server/startosis_engine/startosis_runner.go index 77b90824ab..a72ef4d9c4 100644 --- a/core/server/api_container/server/startosis_engine/startosis_runner.go +++ b/core/server/api_container/server/startosis_engine/startosis_runner.go @@ -4,8 +4,6 @@ import ( "context" "github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings" "github.com/kurtosis-tech/kurtosis/api/golang/core/lib/binding_constructors" - "github.com/kurtosis-tech/kurtosis/core/server/api_container/server/service_network" - metrics_client "github.com/kurtosis-tech/metrics-library/golang/lib/client" "github.com/sirupsen/logrus" ) @@ -33,7 +31,7 @@ func NewStartosisRunner(interpreter *StartosisInterpreter, validator *StartosisV } } -func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism int, packageId string, serializedStartosis string, serializedParams string, metricsClient metrics_client.MetricsClient, serviceNetwork service_network.ServiceNetwork) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { +func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism int, packageId string, serializedStartosis string, serializedParams string) <-chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine { // TODO(gb): add metric tracking maybe? starlarkRunResponseLines := make(chan *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine) @@ -71,7 +69,7 @@ func (runner *StartosisRunner) Run(ctx context.Context, dryRun bool, parallelism startingExecutionMsg, defaultCurrentStepNumber, totalNumberOfInstructions) starlarkRunResponseLines <- progressInfo - executionResponseLinesChan := runner.startosisExecutor.Execute(ctx, dryRun, parallelism, instructionsList, serializedScriptOutput, packageId, metricsClient, serviceNetwork) + executionResponseLinesChan := runner.startosisExecutor.Execute(ctx, dryRun, parallelism, instructionsList, serializedScriptOutput, packageId) if isRunFinished := forwardKurtosisResponseLineChannelUntilSourceIsClosed(executionResponseLinesChan, starlarkRunResponseLines); !isRunFinished { logrus.Warnf("Execution finished but no 'RunFinishedEvent' was received through the stream. This is unexpected as every execution should be terminal.") } From c38d914bd0f444b2a520f65aaf4c995e4e16e312 Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Thu, 16 Feb 2023 14:55:29 +0000 Subject: [PATCH 7/7] moved around a comment --- .../api_container/server/startosis_engine/startosis_executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/server/api_container/server/startosis_engine/startosis_executor.go b/core/server/api_container/server/startosis_engine/startosis_executor.go index e9d5ab9b39..664d8c3a8a 100644 --- a/core/server/api_container/server/startosis_engine/startosis_executor.go +++ b/core/server/api_container/server/startosis_engine/startosis_executor.go @@ -83,11 +83,11 @@ func (executor *StartosisExecutor) Execute(ctx context.Context, dryRun bool, par } } - // TODO(gb): we should run magic string replacement on the output numServices := len(executor.serviceNetwork.GetServiceNames()) if err := executor.metricsClient.TrackKurtosisRunFinishedEvent(packageId, numServices, executionFinishedSuccessfully); err != nil { logrus.Errorf("An error occurred tracking kurtosis run finished event \n%s", err) } + // TODO(gb): we should run magic string replacement on the output starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromRunSuccessEvent(serializedScriptOutput) }() return starlarkRunResponseLineStream