Shorten the comments and changelog added by this change

Several comments restated the code or carried the reasoning behind an
earlier revision, which belongs in the commit messages. Each one now states
the API behaviour or decision the code depends on and stops there.

The changelog entry explained the mechanism at length; it now names the fix
and keeps the regression for unsupported endpoint types.

Author: radakam

.nextchanges/bundles/model-serving-telemetry-config-converges.md

@@ -1 +1 @@
-direct: Fixed `telemetry_config` on model serving endpoints never converging, and edits to it being planned and then silently dropped. The backend creates a telemetry profile from `table_names` and returns only the resulting `telemetry_profile_id`, so every `bundle plan` after a deploy reported an update, and the deploy that followed made no telemetry call at all. Wiring the telemetry API also means a `telemetry_config` on an endpoint that serves nothing or only external models now fails on a deploy that applies the field, with the error that API returns (create still drops the field) ([#6106](https://github.com/databricks/cli/pull/6106)).
+direct: Fixed model serving `telemetry_config` drift and applied planned telemetry updates. Unsupported endpoint types now fail when telemetry is applied; create may still succeed because it drops the field ([#6106](https://github.com/databricks/cli/pull/6106)).

acceptance/bundle/invariant/migrate/test.toml

@@ -38,6 +38,5 @@ EnvMatrixExclude.no_pydabs_1000_tasks = ["INPUT_CONFIG=job_pydabs_1000_tasks.yml
 # volume schema_name ("inconsistent final plan"). Covered by no_drift on direct.
 EnvMatrixExclude.no_volume_uppercase = ["INPUT_CONFIG=volume_uppercase_name.yml.tmpl"]
 
-# The TF provider types sampling_fraction as an integer, so the terraform seed
-# deploy truncates 0.5 to 0 and the plan never converges. Covered by no_drift.
+# Terraform types sampling_fraction as an integer and truncates 0.5; covered by no_drift.
 EnvMatrixExclude.no_model_serving_endpoint_telemetry = ["INPUT_CONFIG=model_serving_endpoint_telemetry.yml.tmpl"]

acceptance/bundle/invariant/test.toml

@@ -117,8 +117,7 @@ no_external_volume_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=volume_externa
 # it here to keep the cloud invariant runs from timing out. Still exercised locally.
 no_vector_search_index_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=vector_search_index.yml.tmpl"]
 
-# Unlike model_serving_endpoint.yml.tmpl, this config serves a real model, which
-# telemetry_config requires and the cloud test environment does not provision.
+# Telemetry requires a real model, which cloud invariant tests do not provision.
 no_model_serving_endpoint_telemetry_on_cloud = ["CONFIG_Cloud=true", "INPUT_CONFIG=model_serving_endpoint_telemetry.yml.tmpl"]
 
 # Fake SQL endpoint for local tests

acceptance/bundle/resources/model_serving_endpoints/drift/telemetry_config_unmanaged/test.toml

@@ -1,4 +1,4 @@
 RecordRequests = true
 
-# Direct engine only, where the classification lives.
+# Telemetry classification is direct-engine-specific.
 EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]

acceptance/bundle/resources/model_serving_endpoints/drift/telemetry_config_with_config_update/test.toml

@@ -1,4 +1,4 @@
 RecordRequests = true
 
-# Direct engine only, where the DoUpdate wait lives.
+# Telemetry updates are direct-engine-specific.
 EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]

bundle/direct/dresources/model_serving_endpoint.go

@@ -247,7 +247,7 @@ func (r *ResourceModelServingEndpoint) updateNotifications(ctx context.Context,
 	return nil
 }
 
-// updateTelemetryConfig applies telemetry_config; a nil config removes it.
+// updateTelemetryConfig removes telemetry when telemetryConfig is nil.
 func (r *ResourceModelServingEndpoint) updateTelemetryConfig(ctx context.Context, id string, telemetryConfig *serving.TelemetryConfig) error {
 	req := serving.PatchTelemetryConfigRequest{
 		Name:            id,
@@ -358,8 +358,7 @@ func (r *ResourceModelServingEndpoint) DoUpdate(ctx context.Context, id string,
 	}
 
 	if entry.Changes.HasChange(pathTelemetryConfig) {
-		// The telemetry API rejects an endpoint still applying an update, and
-		// WaitAfterUpdate runs too late to order the earlier calls with this patch.
+		// The telemetry API rejects endpoints with an update in progress.
 		_, err = r.waitForEndpointReady(ctx, id)
 		if err != nil {
 			return nil, err

bundle/direct/dresources/resources.yml

@@ -285,7 +285,7 @@ resources:
       # Accepted on write but not returned by GET.
       - field: config.served_entities[*].burst_scaling_enabled
         reason: input_only
-      # Consumed to build a profile and never returned; inference_table_config round-trips so it is not listed.
+      # table_names creates a profile and is not returned; inference_table_config round-trips.
       - field: telemetry_config.table_names
         reason: input_only
       # Write-only secrets: the backend stores them and returns the reference field, not the plaintext.
@@ -319,7 +319,7 @@ resources:
       - field: rate_limits
         reason: not_implemented
     backend_defaults:
-      # Subtree, not only telemetry_profile_id: a remote-only config is one change on the parent path.
+      # Remote-only telemetry is reported as a change at this parent path.
       - field: telemetry_config
 
       # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/serving/resource_model_serving.go#L383

libs/testserver/serving_endpoints.go

@@ -35,8 +35,7 @@ func servedEntitiesInputToOutput(input []serving.ServedEntityInput) []serving.Se
 	return entities
 }
 
-// applyTelemetryConfig mirrors the backend: table_names is consumed and not echoed;
-// naming neither table_names nor telemetry_profile_id discards the config.
+// applyTelemetryConfig consumes table_names and discards configs that identify no profile.
 func applyTelemetryConfig(previous, config *serving.TelemetryConfig) *serving.TelemetryConfig {
 	if config == nil {
 		return nil
@@ -62,8 +61,7 @@ func applyTelemetryConfig(previous, config *serving.TelemetryConfig) *serving.Te
 	return &applied
 }
 
-// telemetrySupported is true only for custom served models; otherwise it returns the
-// endpoint type the backend names when rejecting (NO_CONFIG / EXTERNAL_MODELS).
+// telemetrySupported returns the unsupported endpoint type when telemetry cannot be applied.
 func telemetrySupported(endpoint serving.ServingEndpointDetailed) (string, bool) {
 	if endpoint.Config == nil || len(endpoint.Config.ServedEntities) == 0 {
 		return "NO_CONFIG", false
@@ -276,7 +274,7 @@ func (s *FakeWorkspace) ServingEndpointCreate(req Request) Response {
 		ForceSendFields: append(createReq.ForceSendFields, "PermissionLevel", "RouteOptimized", "Description"),
 	}
 
-	// Unlike the telemetry API, create drops telemetry it cannot apply instead of failing.
+	// Create drops unsupported telemetry, while the telemetry API rejects it.
 	if _, ok := telemetrySupported(endpoint); !ok {
 		endpoint.TelemetryConfig = nil
 	}
@@ -288,7 +286,7 @@ func (s *FakeWorkspace) ServingEndpointCreate(req Request) Response {
 	}
 }
 
-// ServingEndpointGet reports an in-progress update once, then settles so a poller converges.
+// ServingEndpointGet reports an in-progress update once before settling it.
 func (s *FakeWorkspace) ServingEndpointGet(name string) Response {
 	defer s.LockUnlock()()
 
@@ -313,6 +311,7 @@ func (s *FakeWorkspace) ServingEndpointGet(name string) Response {
 	return Response{Body: endpoint}
 }
 
+// endpointUpdating reports whether a config update is in progress.
 func endpointUpdating(endpoint serving.ServingEndpointDetailed) bool {
 	return endpoint.State != nil && endpoint.State.ConfigUpdate == serving.EndpointStateConfigUpdateInProgress
 }
@@ -364,7 +363,7 @@ func (s *FakeWorkspace) ServingEndpointUpdate(req Request, name string) Response
 
 	endpoint.Config = config
 	endpoint.LastUpdatedTimestamp = nowMilli()
-	// Leave IN_PROGRESS until the next GET settles it; a same-pass telemetry PATCH must see that.
+	// Keep the update in progress until GET observes it.
 	endpoint.State = &serving.EndpointState{
 		ConfigUpdate: serving.EndpointStateConfigUpdateInProgress,
 		Ready:        serving.EndpointStateReadyNotReady,
@@ -446,6 +445,7 @@ func (s *FakeWorkspace) ServingEndpointUpdateNotifications(req Request, name str
 	}
 }
 
+// ServingEndpointPatchTelemetryConfig applies telemetry after validating endpoint state.
 func (s *FakeWorkspace) ServingEndpointPatchTelemetryConfig(req Request, name string) Response {
 	defer s.LockUnlock()()
 
@@ -476,7 +476,7 @@ func (s *FakeWorkspace) ServingEndpointPatchTelemetryConfig(req Request, name st
 		}
 	}
 
-	// The telemetry API refuses to run while an earlier update is still applying.
+	// The telemetry API returns 409 while another update is in progress.
 	if endpointUpdating(endpoint) {
 		return Response{
 			StatusCode: 409,