From 520f06905b93f80040bc5ea45f5f752725f7dbbe Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 18 Feb 2025 16:30:50 +0100 Subject: [PATCH] use daemon --- acceptance/bin/wait_file | 27 ----------- acceptance/telemetry/dummy-error/output.txt | 5 -- acceptance/telemetry/dummy/output.txt | 5 -- .../out.requests.txt | 4 +- .../out.upload.txt | 0 .../telemetry-upload-fails/output.txt | 3 ++ .../script | 5 +- .../test.toml | 0 .../out.requests.txt | 4 +- .../out.send_telemetry.txt} | 0 .../telemetry-upload-succeeds/output.txt | 4 ++ .../script | 5 +- .../test.toml | 0 .../out.requests.txt | 0 .../{upload => upload-command}/out.upload.txt | 0 .../{upload => upload-command}/output.txt | 0 .../{upload => upload-command}/script | 0 .../{upload => upload-command}/stdin | 4 +- .../{upload => upload-command}/test.toml | 0 cmd/cmd.go | 2 - cmd/root/root.go | 46 ++++++++----------- cmd/selftest/selftest.go | 1 + .../dummy.go => selftest/send_telemetry.go} | 10 ++-- cmd/telemetry/telemetry.go | 16 ------- 24 files changed, 39 insertions(+), 102 deletions(-) delete mode 100755 acceptance/bin/wait_file delete mode 100644 acceptance/telemetry/dummy-error/output.txt delete mode 100644 acceptance/telemetry/dummy/output.txt rename acceptance/telemetry/{dummy-error => telemetry-upload-fails}/out.requests.txt (50%) rename acceptance/telemetry/{dummy-error => telemetry-upload-fails}/out.upload.txt (100%) create mode 100644 acceptance/telemetry/telemetry-upload-fails/output.txt rename acceptance/telemetry/{dummy-error => telemetry-upload-fails}/script (77%) rename acceptance/telemetry/{dummy-error => telemetry-upload-fails}/test.toml (100%) rename acceptance/telemetry/{dummy => telemetry-upload-succeeds}/out.requests.txt (50%) rename acceptance/telemetry/{dummy/out.upload.txt => telemetry-upload-succeeds/out.send_telemetry.txt} (100%) create mode 100644 acceptance/telemetry/telemetry-upload-succeeds/output.txt rename acceptance/telemetry/{dummy => telemetry-upload-succeeds}/script (66%) rename acceptance/telemetry/{dummy => telemetry-upload-succeeds}/test.toml (100%) rename acceptance/telemetry/{upload => upload-command}/out.requests.txt (100%) rename acceptance/telemetry/{upload => upload-command}/out.upload.txt (100%) rename acceptance/telemetry/{upload => upload-command}/output.txt (100%) rename acceptance/telemetry/{upload => upload-command}/script (100%) rename acceptance/telemetry/{upload => upload-command}/stdin (74%) rename acceptance/telemetry/{upload => upload-command}/test.toml (100%) rename cmd/{telemetry/dummy.go => selftest/send_telemetry.go} (70%) delete mode 100644 cmd/telemetry/telemetry.go diff --git a/acceptance/bin/wait_file b/acceptance/bin/wait_file deleted file mode 100755 index 5fa1ab69c..000000000 --- a/acceptance/bin/wait_file +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/bash - -wait_file() { - local file_path="$1" - local max_attempts=100 - local attempt=0 - - while [ $attempt -lt $max_attempts ]; do - if [ -e "$file_path" ]; then - echo "File $file_path exists" - return 0 - fi - sleep 0.1 - attempt=$((attempt + 1)) - done - - echo "Timeout: File $file_path did not appear within 10 seconds" - return 1 -} - -if [ $# -eq 0 ]; then - echo "Usage: $0 " - exit 1 -fi - -wait_file "$1" -exit $? diff --git a/acceptance/telemetry/dummy-error/output.txt b/acceptance/telemetry/dummy-error/output.txt deleted file mode 100644 index 4fe94d1ad..000000000 --- a/acceptance/telemetry/dummy-error/output.txt +++ /dev/null @@ -1,5 +0,0 @@ - ->>> [CLI] telemetry dummy -waiting for telemetry process to finish -File ./telemetry.pid exists -Process has ended diff --git a/acceptance/telemetry/dummy/output.txt b/acceptance/telemetry/dummy/output.txt deleted file mode 100644 index 4fe94d1ad..000000000 --- a/acceptance/telemetry/dummy/output.txt +++ /dev/null @@ -1,5 +0,0 @@ - ->>> [CLI] telemetry dummy -waiting for telemetry process to finish -File ./telemetry.pid exists -Process has ended diff --git a/acceptance/telemetry/dummy-error/out.requests.txt b/acceptance/telemetry/telemetry-upload-fails/out.requests.txt similarity index 50% rename from acceptance/telemetry/dummy-error/out.requests.txt rename to acceptance/telemetry/telemetry-upload-fails/out.requests.txt index a94ed3fd7..9b23f98a4 100644 --- a/acceptance/telemetry/dummy-error/out.requests.txt +++ b/acceptance/telemetry/telemetry-upload-fails/out.requests.txt @@ -5,8 +5,8 @@ "uploadTime": "UNIX_TIME_MILLIS", "items": [], "protoLogs": [ - "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"telemetry_dummy\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE1\"}}}}", - "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"telemetry_dummy\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE2\"}}}}" + "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"selftest_send-telemetry\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE1\"}}}}", + "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"selftest_send-telemetry\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE2\"}}}}" ] } } diff --git a/acceptance/telemetry/dummy-error/out.upload.txt b/acceptance/telemetry/telemetry-upload-fails/out.upload.txt similarity index 100% rename from acceptance/telemetry/dummy-error/out.upload.txt rename to acceptance/telemetry/telemetry-upload-fails/out.upload.txt diff --git a/acceptance/telemetry/telemetry-upload-fails/output.txt b/acceptance/telemetry/telemetry-upload-fails/output.txt new file mode 100644 index 000000000..5e94a34d3 --- /dev/null +++ b/acceptance/telemetry/telemetry-upload-fails/output.txt @@ -0,0 +1,3 @@ + +>>> [CLI] selftest send-telemetry +[wait_pid] process has ended diff --git a/acceptance/telemetry/dummy-error/script b/acceptance/telemetry/telemetry-upload-fails/script similarity index 77% rename from acceptance/telemetry/dummy-error/script rename to acceptance/telemetry/telemetry-upload-fails/script index 8f22b50d9..b305b366a 100644 --- a/acceptance/telemetry/dummy-error/script +++ b/acceptance/telemetry/telemetry-upload-fails/script @@ -3,12 +3,9 @@ export DATABRICKS_CLI_TELEMETRY_UPLOAD_LOGS_FILE=./out.upload.txt # This test ensures that the main CLI command does not error even if # telemetry upload fails. -trace $CLI telemetry dummy - -echo "waiting for telemetry process to finish" +trace $CLI selftest send-telemetry # Wait for the child telemetry process to finish -wait_file ./telemetry.pid wait_pid $(cat ./telemetry.pid) # cleanup the pid file diff --git a/acceptance/telemetry/dummy-error/test.toml b/acceptance/telemetry/telemetry-upload-fails/test.toml similarity index 100% rename from acceptance/telemetry/dummy-error/test.toml rename to acceptance/telemetry/telemetry-upload-fails/test.toml diff --git a/acceptance/telemetry/dummy/out.requests.txt b/acceptance/telemetry/telemetry-upload-succeeds/out.requests.txt similarity index 50% rename from acceptance/telemetry/dummy/out.requests.txt rename to acceptance/telemetry/telemetry-upload-succeeds/out.requests.txt index a94ed3fd7..9b23f98a4 100644 --- a/acceptance/telemetry/dummy/out.requests.txt +++ b/acceptance/telemetry/telemetry-upload-succeeds/out.requests.txt @@ -5,8 +5,8 @@ "uploadTime": "UNIX_TIME_MILLIS", "items": [], "protoLogs": [ - "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"telemetry_dummy\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE1\"}}}}", - "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"telemetry_dummy\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE2\"}}}}" + "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"selftest_send-telemetry\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE1\"}}}}", + "{\"frontend_log_event_id\":\"[UUID]\",\"entry\":{\"databricks_cli_log\":{\"execution_context\":{\"cmd_exec_id\":\"[UUID]\",\"version\":\"[DEV_VERSION]\",\"command\":\"selftest_send-telemetry\",\"operating_system\":\"OS\",\"execution_time_ms\":\"SMALL_INT\",\"exit_code\":0},\"cli_test_event\":{\"name\":\"VALUE2\"}}}}" ] } } diff --git a/acceptance/telemetry/dummy/out.upload.txt b/acceptance/telemetry/telemetry-upload-succeeds/out.send_telemetry.txt similarity index 100% rename from acceptance/telemetry/dummy/out.upload.txt rename to acceptance/telemetry/telemetry-upload-succeeds/out.send_telemetry.txt diff --git a/acceptance/telemetry/telemetry-upload-succeeds/output.txt b/acceptance/telemetry/telemetry-upload-succeeds/output.txt new file mode 100644 index 000000000..1a1f61588 --- /dev/null +++ b/acceptance/telemetry/telemetry-upload-succeeds/output.txt @@ -0,0 +1,4 @@ + +>>> [CLI] selftest send-telemetry +waiting for telemetry process to finish +[wait_pid] process has ended diff --git a/acceptance/telemetry/dummy/script b/acceptance/telemetry/telemetry-upload-succeeds/script similarity index 66% rename from acceptance/telemetry/dummy/script rename to acceptance/telemetry/telemetry-upload-succeeds/script index 0c0f52950..ccae6f9b2 100644 --- a/acceptance/telemetry/dummy/script +++ b/acceptance/telemetry/telemetry-upload-succeeds/script @@ -1,12 +1,11 @@ export DATABRICKS_CLI_TELEMETRY_PID_FILE=./telemetry.pid -export DATABRICKS_CLI_TELEMETRY_UPLOAD_LOGS_FILE=./out.upload.txt +export DATABRICKS_CLI_TELEMETRY_UPLOAD_LOGS_FILE=./out.send_telemetry.txt -trace $CLI telemetry dummy +trace $CLI selftest send-telemetry echo "waiting for telemetry process to finish" # Wait for the child telemetry process to finish -wait_file ./telemetry.pid wait_pid $(cat ./telemetry.pid) # cleanup the pid file diff --git a/acceptance/telemetry/dummy/test.toml b/acceptance/telemetry/telemetry-upload-succeeds/test.toml similarity index 100% rename from acceptance/telemetry/dummy/test.toml rename to acceptance/telemetry/telemetry-upload-succeeds/test.toml diff --git a/acceptance/telemetry/upload/out.requests.txt b/acceptance/telemetry/upload-command/out.requests.txt similarity index 100% rename from acceptance/telemetry/upload/out.requests.txt rename to acceptance/telemetry/upload-command/out.requests.txt diff --git a/acceptance/telemetry/upload/out.upload.txt b/acceptance/telemetry/upload-command/out.upload.txt similarity index 100% rename from acceptance/telemetry/upload/out.upload.txt rename to acceptance/telemetry/upload-command/out.upload.txt diff --git a/acceptance/telemetry/upload/output.txt b/acceptance/telemetry/upload-command/output.txt similarity index 100% rename from acceptance/telemetry/upload/output.txt rename to acceptance/telemetry/upload-command/output.txt diff --git a/acceptance/telemetry/upload/script b/acceptance/telemetry/upload-command/script similarity index 100% rename from acceptance/telemetry/upload/script rename to acceptance/telemetry/upload-command/script diff --git a/acceptance/telemetry/upload/stdin b/acceptance/telemetry/upload-command/stdin similarity index 74% rename from acceptance/telemetry/upload/stdin rename to acceptance/telemetry/upload-command/stdin index 6874ffaee..373e1545e 100644 --- a/acceptance/telemetry/upload/stdin +++ b/acceptance/telemetry/upload-command/stdin @@ -1,7 +1,7 @@ { "logs": [ { - "frontend_log_event_id": "BB79BB52-96F6-42C5-9E44-E63EEA84888D", + "frontend_log_event_id": "AAAAAAAA-AAAA-AAAA-AAAA-AAAAAAAAAAAA", "entry": { "databricks_cli_log": { "cli_test_event": { @@ -11,7 +11,7 @@ } }, { - "frontend_log_event_id": "A7F597B0-66D1-462D-824C-C5C706F232E8", + "frontend_log_event_id": "BBBBBBBB-BBBB-BBBB-BBBB-BBBBBBBBBBBB", "entry": { "databricks_cli_log": { "cli_test_event": { diff --git a/acceptance/telemetry/upload/test.toml b/acceptance/telemetry/upload-command/test.toml similarity index 100% rename from acceptance/telemetry/upload/test.toml rename to acceptance/telemetry/upload-command/test.toml diff --git a/cmd/cmd.go b/cmd/cmd.go index 00e9512d4..4f5337fd3 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -14,7 +14,6 @@ import ( "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/cmd/selftest" "github.com/databricks/cli/cmd/sync" - "github.com/databricks/cli/cmd/telemetry" "github.com/databricks/cli/cmd/version" "github.com/databricks/cli/cmd/workspace" "github.com/spf13/cobra" @@ -76,7 +75,6 @@ func New(ctx context.Context) *cobra.Command { cli.AddCommand(labs.New(ctx)) cli.AddCommand(sync.New()) cli.AddCommand(version.New()) - cli.AddCommand(telemetry.New()) cli.AddCommand(selftest.New()) return cli diff --git a/cmd/root/root.go b/cmd/root/root.go index 0d2489126..0a738822f 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -7,16 +7,15 @@ import ( "fmt" "log/slog" "os" - "os/exec" "runtime" "slices" - "strconv" "strings" "time" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/daemon" "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" @@ -198,47 +197,38 @@ func uploadTelemetry(ctx context.Context, cmdStr string, start, end time.Time, e Logs: logs, } - execPath, err := os.Executable() - if err != nil { - log.Debugf(ctx, "failed to get executable path: %s", err) - } - telemetryCmd := exec.Command(execPath, "telemetry", "upload") - telemetryCmd.Env = inheritEnvVars() + // Compute environment variables with the appropriate auth configuration. + env := inheritEnvVars() for k, v := range auth.Env(ConfigUsed(ctx)) { - telemetryCmd.Env = append(telemetryCmd.Env, fmt.Sprintf("%s=%s", k, v)) + env = append(env, fmt.Sprintf("%s=%s", k, v)) } - b, err := json.Marshal(in) - if err != nil { - log.Debugf(ctx, "failed to marshal telemetry logs: %s", err) - return + d := daemon.Daemon{ + Args: []string{"telemetry", "upload"}, + Env: env, + PidFilePath: os.Getenv(telemetry.PidFileEnvVar), + LogFile: os.Getenv(telemetry.UploadLogsFileEnvVar), } - stdin, err := telemetryCmd.StdinPipe() - if err != nil { - log.Debugf(ctx, "failed to create stdin pipe for telemetry worker: %s", err) - } - - err = telemetryCmd.Start() + err := d.Start() if err != nil { log.Debugf(ctx, "failed to start telemetry worker: %s", err) return } - if pidFilePath := env.Get(ctx, telemetry.PidFileEnvVar); pidFilePath != "" { - err = os.WriteFile(pidFilePath, []byte(strconv.Itoa(telemetryCmd.Process.Pid)), 0o644) - if err != nil { - log.Debugf(ctx, "failed to write telemetry worker PID file: %s", err) - } + // If the telemetry worker is started successfully, we write the logs to its stdin. + b, err := json.Marshal(in) + if err != nil { + log.Debugf(ctx, "failed to marshal telemetry logs: %s", err) + return } - - _, err = stdin.Write(b) + err = d.WriteInput(b) if err != nil { log.Debugf(ctx, "failed to write to telemetry worker: %s", err) } - err = stdin.Close() + err = d.Release() if err != nil { - log.Debugf(ctx, "failed to close stdin for telemetry worker: %s", err) + log.Debugf(ctx, "failed to release telemetry worker: %s", err) } } diff --git a/cmd/selftest/selftest.go b/cmd/selftest/selftest.go index 3291e0633..145909fb8 100644 --- a/cmd/selftest/selftest.go +++ b/cmd/selftest/selftest.go @@ -13,5 +13,6 @@ func New() *cobra.Command { cmd.AddCommand(newChildCommand()) cmd.AddCommand(newParentCommand()) + cmd.AddCommand(newSendTelemetry()) return cmd } diff --git a/cmd/telemetry/dummy.go b/cmd/selftest/send_telemetry.go similarity index 70% rename from cmd/telemetry/dummy.go rename to cmd/selftest/send_telemetry.go index 792cf5de8..b091058e7 100644 --- a/cmd/telemetry/dummy.go +++ b/cmd/selftest/send_telemetry.go @@ -1,4 +1,4 @@ -package telemetry +package selftest import ( "github.com/databricks/cli/cmd/root" @@ -7,12 +7,10 @@ import ( "github.com/spf13/cobra" ) -func newDummyCommand() *cobra.Command { +func newSendTelemetry() *cobra.Command { cmd := &cobra.Command{ - Use: "dummy", - Short: "log dummy telemetry events", - Long: "Fire a test telemetry event against the configured Databricks workspace.", - Hidden: true, + Use: "send-telemetry", + Short: "log some test telemetry events", PreRunE: root.MustWorkspaceClient, } diff --git a/cmd/telemetry/telemetry.go b/cmd/telemetry/telemetry.go deleted file mode 100644 index 888aa318f..000000000 --- a/cmd/telemetry/telemetry.go +++ /dev/null @@ -1,16 +0,0 @@ -package telemetry - -import ( - "github.com/spf13/cobra" -) - -func New() *cobra.Command { - cmd := &cobra.Command{ - Use: "telemetry", - Short: "", - Hidden: true, - } - - cmd.AddCommand(newDummyCommand()) - return cmd -}