diff --git a/.nextchanges/bundles/immutable-snapshot-telemetry.md b/.nextchanges/bundles/immutable-snapshot-telemetry.md new file mode 100644 index 00000000000..3c88eddb16f --- /dev/null +++ b/.nextchanges/bundles/immutable-snapshot-telemetry.md @@ -0,0 +1 @@ +* Add deploy telemetry for immutable snapshots: whether a deploy used a snapshot and how long its upload took. ([#7168](https://github.com/databricks/cli/pull/7168)) diff --git a/acceptance/bundle/resource_deps/job_tasks/output.txt b/acceptance/bundle/resource_deps/job_tasks/output.txt index f8c14742d70..806433661ed 100644 --- a/acceptance/bundle/resource_deps/job_tasks/output.txt +++ b/acceptance/bundle/resource_deps/job_tasks/output.txt @@ -18,6 +18,7 @@ has_classic_interactive_compute false has_classic_job_compute false has_python_wheel_task true has_serverless_compute true +immutable_snapshot_used false local.cache.attempt true local.cache.miss true permissions_section_set false diff --git a/acceptance/bundle/resource_deps/resources_var/output.txt b/acceptance/bundle/resource_deps/resources_var/output.txt index 5538dc49ab2..98af2cec84e 100644 --- a/acceptance/bundle/resource_deps/resources_var/output.txt +++ b/acceptance/bundle/resource_deps/resources_var/output.txt @@ -47,6 +47,7 @@ experimental.use_legacy_run_as false has_classic_interactive_compute false has_classic_job_compute false has_serverless_compute false +immutable_snapshot_used false local.cache.attempt true local.cache.hit true permissions_section_set false diff --git a/acceptance/bundle/resource_deps/tf_path_only_error/out.telemetry.txt b/acceptance/bundle/resource_deps/tf_path_only_error/out.telemetry.txt index f9aa960d00f..9557c4da3c9 100644 --- a/acceptance/bundle/resource_deps/tf_path_only_error/out.telemetry.txt +++ b/acceptance/bundle/resource_deps/tf_path_only_error/out.telemetry.txt @@ -1,5 +1,6 @@ experimental.use_legacy_run_as false has_tf_only_references true +immutable_snapshot_used false local.cache.attempt true local.cache.hit true presets_name_prefix_is_set false diff --git a/acceptance/bundle/telemetry/deploy-app-lifecycle-started/output.txt b/acceptance/bundle/telemetry/deploy-app-lifecycle-started/output.txt index b70ab61ef9b..4362ebba960 100644 --- a/acceptance/bundle/telemetry/deploy-app-lifecycle-started/output.txt +++ b/acceptance/bundle/telemetry/deploy-app-lifecycle-started/output.txt @@ -93,6 +93,10 @@ Resources: 1 created, 0 changed, 0 deleted, 0 unchanged "key": "deployment_history_enabled", "value": false }, + { + "key": "immutable_snapshot_used", + "value": false + }, { "key": "app_lifecycle_started", "value": true diff --git a/acceptance/bundle/telemetry/deploy-compute-type/output.txt b/acceptance/bundle/telemetry/deploy-compute-type/output.txt index 453fffeaae2..cd5666d3415 100644 --- a/acceptance/bundle/telemetry/deploy-compute-type/output.txt +++ b/acceptance/bundle/telemetry/deploy-compute-type/output.txt @@ -33,6 +33,8 @@ has_serverless_compute true has_serverless_compute true has_spark_python_task true has_spark_python_task true +immutable_snapshot_used false +immutable_snapshot_used false local.cache.attempt true local.cache.attempt true local.cache.hit true diff --git a/acceptance/bundle/telemetry/deploy-experimental/output.txt b/acceptance/bundle/telemetry/deploy-experimental/output.txt index baa7f5a4721..dcd99009312 100644 --- a/acceptance/bundle/telemetry/deploy-experimental/output.txt +++ b/acceptance/bundle/telemetry/deploy-experimental/output.txt @@ -20,6 +20,7 @@ has_classic_interactive_compute true has_classic_job_compute false has_serverless_compute false has_spark_python_task true +immutable_snapshot_used false local.cache.attempt true local.cache.miss true permissions_section_set false diff --git a/acceptance/bundle/telemetry/deploy-immutable-snapshot/databricks.yml b/acceptance/bundle/telemetry/deploy-immutable-snapshot/databricks.yml new file mode 100644 index 00000000000..c6b7cb5b004 --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-immutable-snapshot/databricks.yml @@ -0,0 +1,20 @@ +bundle: + name: test-bundle + +experimental: + immutable_folder: true + +resources: + jobs: + foo: + name: foo + tasks: + - task_key: task1 + spark_python_task: + python_file: ./src/main.py + environment_key: env + + environments: + - environment_key: env + spec: + environment_version: "4" diff --git a/acceptance/bundle/telemetry/deploy-immutable-snapshot/out.test.toml b/acceptance/bundle/telemetry/deploy-immutable-snapshot/out.test.toml new file mode 100644 index 00000000000..19b5b15d11f --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-immutable-snapshot/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/telemetry/deploy-immutable-snapshot/output.txt b/acceptance/bundle/telemetry/deploy-immutable-snapshot/output.txt new file mode 100644 index 00000000000..c8d5bdbc49d --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-immutable-snapshot/output.txt @@ -0,0 +1,26 @@ + +=== First deploy uploads the snapshot + +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.foo +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +>>> print_telemetry_bool_values +immutable_snapshot_used true + +>>> print_snapshot_upload_entries +1 + +=== Second deploy finds the snapshot already uploaded + +>>> [CLI] bundle deploy +Files: 0 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 2 unchanged + +>>> print_telemetry_bool_values +immutable_snapshot_used true + +>>> print_snapshot_upload_entries +0 diff --git a/acceptance/bundle/telemetry/deploy-immutable-snapshot/script b/acceptance/bundle/telemetry/deploy-immutable-snapshot/script new file mode 100644 index 00000000000..ade8746ac6c --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-immutable-snapshot/script @@ -0,0 +1,17 @@ +# The snapshot upload duration is reported next to the files.(upload) mutator time. Its value +# varies, so only assert how many times it is reported. +print_snapshot_upload_entries() { + jq -r 'select(.path? == "/telemetry-ext") | (.body.protoLogs // [])[] | fromjson | (.entry // .) | .databricks_cli_log.bundle_deploy_event.experimental.bundle_mutator_execution_time_ms // [] | map(select(.key == "snapshot.upload" and .value >= 0)) | length' "$OUT_REQUESTS" +} + +title "First deploy uploads the snapshot\n" +trace $CLI bundle deploy +trace print_telemetry_bool_values | grep immutable_snapshot_used +trace print_snapshot_upload_entries +rm "$OUT_REQUESTS" + +title "Second deploy finds the snapshot already uploaded\n" +trace $CLI bundle deploy +trace print_telemetry_bool_values | grep immutable_snapshot_used +trace print_snapshot_upload_entries +rm "$OUT_REQUESTS" diff --git a/acceptance/bundle/telemetry/deploy-immutable-snapshot/src/main.py b/acceptance/bundle/telemetry/deploy-immutable-snapshot/src/main.py new file mode 100644 index 00000000000..11b15b1a458 --- /dev/null +++ b/acceptance/bundle/telemetry/deploy-immutable-snapshot/src/main.py @@ -0,0 +1 @@ +print("hello") diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt b/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt index f71dbd423dc..a7eb8f29142 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt +++ b/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt @@ -15,6 +15,7 @@ experimental.use_legacy_run_as false has_classic_interactive_compute false has_classic_job_compute false has_serverless_compute false +immutable_snapshot_used false local.cache.attempt true local.cache.miss true permissions_section_set false diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt index ae9172ced5b..9067822c457 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt +++ b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt @@ -15,6 +15,7 @@ experimental.use_legacy_run_as false has_classic_interactive_compute false has_classic_job_compute false has_serverless_compute false +immutable_snapshot_used false local.cache.attempt true local.cache.miss true permissions_section_set false diff --git a/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt b/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt index 3e3f4d9d66e..1f42024a762 100644 --- a/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt +++ b/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt @@ -35,6 +35,8 @@ has_classic_job_compute false has_classic_job_compute false has_serverless_compute false has_serverless_compute false +immutable_snapshot_used false +immutable_snapshot_used false local.cache.attempt true local.cache.attempt true local.cache.hit true diff --git a/acceptance/bundle/telemetry/deploy/out.telemetry.txt b/acceptance/bundle/telemetry/deploy/out.telemetry.txt index 45a366836f1..cb92e434e30 100644 --- a/acceptance/bundle/telemetry/deploy/out.telemetry.txt +++ b/acceptance/bundle/telemetry/deploy/out.telemetry.txt @@ -125,6 +125,10 @@ { "key": "deployment_history_enabled", "value": false + }, + { + "key": "immutable_snapshot_used", + "value": false } ], "bundle_mode": "TYPE_UNSPECIFIED", diff --git a/bundle/direct/bundle_apply.go b/bundle/direct/bundle_apply.go index a673a3239c8..add38bf99f6 100644 --- a/bundle/direct/bundle_apply.go +++ b/bundle/direct/bundle_apply.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "time" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/deployplan" @@ -158,6 +159,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa // // Success is recorded by the state writes inside Deploy, so a recreate reports // each of its steps. + start := time.Now() err = d.Deploy(ctx, &b.StateDB, sv.Value, action, entry) if err != nil { // Empty for a create that never got an ID, and for a recreate whose delete @@ -167,6 +169,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa logdiag.LogError(ctx, fmt.Errorf("%s: %w", errorPrefix, err)) return false } + b.DeployDurations.Store(resourceKey, time.Since(start)) // Reported before the remote-state refresh below: the resource is already // deployed at this point, so the line is accurate even if the refresh fails. diff --git a/bundle/direct/pkg.go b/bundle/direct/pkg.go index ea7ae1e51b6..6cfee30dc3b 100644 --- a/bundle/direct/pkg.go +++ b/bundle/direct/pkg.go @@ -50,6 +50,11 @@ type DeploymentBundle struct { Plan *deployplan.Plan RemoteStateCache sync.Map StateCache structvar.Cache + + // DeployDurations maps a resource key to the time.Duration its create, update or + // recreate took. Only successfully deployed resources are recorded; Apply runs + // nodes in parallel, hence the sync.Map. + DeployDurations sync.Map } // SetRemoteState updates the remote state with type validation and marks as fresh. diff --git a/bundle/metrics/metrics.go b/bundle/metrics/metrics.go index 05d663f4244..f350b3b91fb 100644 --- a/bundle/metrics/metrics.go +++ b/bundle/metrics/metrics.go @@ -12,6 +12,19 @@ const ( SqlWarehouseLifecycleStarted = "sql_warehouse_lifecycle_started" SelectUsed = "select_used" + // Whether the deploy registered an immutable snapshot. Derived from the snapshot + // resource, not from experimental.immutable_folder: the flag may change, and the + // resource is what the deploy actually uploads. + ImmutableSnapshotUsed = "immutable_snapshot_used" + + // Key under which the snapshot upload duration is reported in + // bundle_mutator_execution_time_ms, next to files.(upload) so the two can be + // compared. Covers creating the snapshot resource (the upload plus recording it in + // state), not building the zip, which is snapshot.(snapshotUpload). Absent when no + // snapshot was uploaded. It lacks the parentheses of a mutator name because it is + // not a mutator. + SnapshotUploadTime = "snapshot.upload" + // Outcome of an automatic migration of terraform state to the direct engine. // - migrate_error: state conversion itself errored. // - migrate_commit_error: the state was converted, but committing it diff --git a/bundle/phases/telemetry.go b/bundle/phases/telemetry.go index 99464f3ef24..062d31324a9 100644 --- a/bundle/phases/telemetry.go +++ b/bundle/phases/telemetry.go @@ -7,6 +7,7 @@ import ( "reflect" "slices" "strings" + "time" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" @@ -199,6 +200,13 @@ func aiRuntimeTaskMetrics(jobs map[string]*resources.Job) (present, scheduled, m func LogDeployTelemetry(ctx context.Context, b *bundle.Bundle, errMsg string) { errMsg = telemetry.ScrubErrorMessage(errMsg) b.Metrics.SetBoolValue(metrics.DeploymentHistoryEnabled, b.DeploymentBundle.StateDB.IsDeploymentMetadataService()) + b.Metrics.SetBoolValue(metrics.ImmutableSnapshotUsed, len(b.Config.Resources.Snapshots) > 0) + if d, ok := b.DeploymentBundle.DeployDurations.Load(resources.SnapshotKey); ok { + b.Metrics.ExecutionTimes = append(b.Metrics.ExecutionTimes, protos.IntMapEntry{ + Key: metrics.SnapshotUploadTime, + Value: d.(time.Duration).Milliseconds(), + }) + } resourcesCount := int64(0) _, err := dyn.MapByPattern(b.Config.Value(), dyn.NewPattern(dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey()), func(p dyn.Path, v dyn.Value) (dyn.Value, error) {