Classify raw OpenTelemetry attributes so GDPR metadata is applied

cli/azd/AGENTS.md

@@ -279,20 +279,31 @@ public reference, and downstream Kusto/LENS consumers drift out of sync. Verify
 
 **1. Code**
 
-- **Field** — define an `AttributeKey` in `cli/azd/internal/tracing/fields/fields.go` (this file
-  holds the field/key definitions; within the same package `features.go` holds feature-name
-  attribute values and `domains.go` the Azure host-domain table). Every field MUST set a
-  `Classification` (e.g. `SystemMetadata`, `OrganizationalIdentifiableInformation`,
-  `EndUserPseudonymizedInformation`; never emit `CustomerContent`) and a `Purpose`
-  (`FeatureInsight` / `BusinessInsight` / `PerformanceAndHealth`).
+- **Field** — define an **exported, package-level** `AttributeKey` var in
+  `cli/azd/internal/tracing/fields/fields.go` (this file holds the field/key definitions; within the
+  same package `features.go` holds feature-name attribute values and `domains.go` the Azure
+  host-domain table). Every field MUST set a `Classification` (e.g. `SystemMetadata`,
+  `OrganizationalIdentifiableInformation`, `EndUserPseudonymizedInformation`; never emit
+  `CustomerContent`) and a `Purpose` (`FeatureInsight` / `BusinessInsight` /
+  `PerformanceAndHealth`); the classifier also reads the optional `Endpoint` and `IsMeasurement`
+  members.
 - **Event** — define a constant in `cli/azd/internal/tracing/events/events.go` following the
-  `prefix.noun.verb` naming convention.
+  `prefix.noun.verb` value convention. It must be an exported string `const` whose Go identifier
+  contains `Event` (end it with `Prefix` for a prefix-match group) so the classifier
+  discovers it.
 - **Emit** at the call site via `tracing.Start` (spans/events) plus `tracing.SetUsageAttributes`
-  or `span.SetAttributes` (attributes).
+  or `span.SetAttributes` (attributes). Always pass a `fields.AttributeKey` method
+  (e.g. `fields.MyKey.String(v)` / `.Bool(v)` / `.Int(v)`) — never a raw
+  `attribute.String("my.key", v)`. The GDPR classifier discovers fields by statically scanning
+  the `fields` package for exported `AttributeKey` vars; a raw literal key is invisible to it, so the
+  property reaches App Insights but its data-catalog row stays Unclassified / `Complete=false`. Enforced by
+  `TestNoRawTelemetryAttributes` (`cli/azd/cmd/telemetry_test.go`); dynamic
+  `ext.*` keys are the only sanctioned exception.
 - **Hash user-derived values** with `fields.StringHashed` / `fields.StringSliceHashed`
   (`cli/azd/internal/tracing/fields/key.go`). Hash anything that embeds a user-chosen name, path,
   repo URL, or project / env / service / layer identifier (e.g. `exegraph.step.name`, `hooks.name`).
-  Emit raw only for fixed enums or compile-time literals.
+  Emit raw only for fixed enums or compile-time literals (the key itself must
+  still be a `fields.AttributeKey`, per **Emit** above).
 
 **2. Documentation — keep all of these in sync**
 

cli/azd/cmd/auth_login.go

@@ -33,7 +33,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/github"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"go.opentelemetry.io/otel/attribute"
 )
 
 // The parent of the login command.
@@ -378,11 +377,11 @@ func (la *loginAction) Run(ctx context.Context) (*actions.ActionResult, error) {
 	if !isServicePrincipalOrMI {
 		if _, err := la.authManager.LogInDetails(ctx); !errors.Is(err, auth.ErrNoCurrentUser) {
 			if err := la.authManager.CleanAllAuthCache(); err != nil {
-				tracing.SetUsageAttributes(attribute.String("auth.cache_clear_failed", "auth"))
+				tracing.SetUsageAttributes(fields.AuthCacheClearFailedKey.String("auth"))
 				return nil, fmt.Errorf("clearing auth cache: %w", err)
 			}
 			if err := la.accountSubManager.ClearSubscriptions(ctx); err != nil {
-				tracing.SetUsageAttributes(attribute.String("auth.cache_clear_failed", "subscriptions"))
+				tracing.SetUsageAttributes(fields.AuthCacheClearFailedKey.String("subscriptions"))
 				return nil, fmt.Errorf("clearing subscriptions cache: %w", err)
 			}
 		}

cli/azd/cmd/telemetry_test.go

@@ -4,6 +4,13 @@
 package cmd
 
 import (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -26,7 +33,8 @@ func TestTelemetryEventConstants(t *testing.T) {
 // NOTE: This test validates field definitions, not command-level instrumentation.
 // Command-level coverage is enforced via the documented allowlist in
 // TestCommandTelemetryCoverageAllowlist (below) and the feature-telemetry-matrix.md.
-// Full AST-based scanning of SetUsageAttributes calls is a future enhancement.
+// Raw attribute.* string-literal keys in telemetry sinks are additionally
+// rejected by TestNoRawTelemetryAttributes (below).
 func TestTelemetryFieldConstants(t *testing.T) {
 	t.Parallel()
 	// Auth command telemetry fields
@@ -47,6 +55,15 @@ func TestTelemetryFieldConstants(t *testing.T) {
 			kv := fields.AuthMethodKey.String(method)
 			require.NotEmpty(t, kv.Value.AsString())
 		}
+
+		// Cache-clear failure indicator (fixed enum, emitted on `auth login`).
+		kvCache := fields.AuthCacheClearFailedKey.String("auth")
+		require.Equal(t, "auth.cache_clear_failed", string(kvCache.Key))
+		require.Equal(t, "auth", kvCache.Value.AsString())
+		for _, which := range []string{"auth", "subscriptions"} {
+			kv := fields.AuthCacheClearFailedKey.String(which)
+			require.NotEmpty(t, kv.Value.AsString())
+		}
 	})
 
 	// Env command telemetry fields
@@ -254,6 +271,147 @@ func TestTelemetryFieldConstants(t *testing.T) {
 			require.NotEmpty(t, kv.Value.AsString())
 		}
 	})
+
+	// Container publish telemetry fields
+	t.Run("ContainerFields", func(t *testing.T) {
+		t.Parallel()
+		kv := fields.ContainerPublishRemoteBuildKey.Bool(true)
+		require.Equal(t, "container.publish.remotebuild", string(kv.Key))
+		require.Equal(t, true, kv.Value.AsBool())
+	})
+
+	// AKS service target telemetry fields
+	t.Run("AksFields", func(t *testing.T) {
+		t.Parallel()
+		kv := fields.AksSkipReasonKey.String("cluster_not_provisioned")
+		require.Equal(t, "skip.reason", string(kv.Key))
+		require.Equal(t, "cluster_not_provisioned", kv.Value.AsString())
+	})
+}
+
+// TestNoRawTelemetryAttributes enforces that product code never emits telemetry
+// via raw attribute.String("literal", ...) / attribute.Bool("literal", ...) etc.
+// Every telemetry attribute must be declared as a fields.AttributeKey (with a
+// Classification and Purpose) and emitted through it, e.g.
+// fields.SomeKey.String(value). This keeps the telemetry schema discoverable and
+// classifiable for the GDPR metadata pipeline (azure-dev issue #1803).
+//
+// Legitimately excluded from the scan:
+//   - *_test.go files (test fixtures build raw attributes on purpose).
+//   - internal/tracing/... — the tracing/baggage plumbing that the
+//     fields.AttributeKey abstraction is itself built on top of.
+//   - extensions/... — independent extension modules with their own schema.
+func TestNoRawTelemetryAttributes(t *testing.T) {
+	t.Parallel()
+
+	// rawAttributeConstructors are the go.opentelemetry.io/otel/attribute helpers
+	// that build a KeyValue from a string-literal key. Using them directly in
+	// product code bypasses the fields.AttributeKey registry, so the GDPR metadata
+	// exporter (which discovers only exported AttributeKey vars) can never classify
+	// the resulting property. See docs/specs/metrics-audit/telemetry-schema.md.
+	rawAttributeConstructors := map[string]struct{}{
+		"String":       {},
+		"Bool":         {},
+		"Int":          {},
+		"Int64":        {},
+		"Float64":      {},
+		"Stringer":     {},
+		"StringSlice":  {},
+		"BoolSlice":    {},
+		"Int64Slice":   {},
+		"Float64Slice": {},
+	}
+
+	// The test runs with its package directory (cli/azd/cmd) as the working
+	// directory, so the module root is one level up.
+	azdRoot, err := filepath.Abs("..")
+	require.NoError(t, err)
+
+	var violations []string
+
+	err = filepath.Walk(azdRoot, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			switch filepath.Base(path) {
+			case "vendor", "extensions", "testdata", "node_modules", ".git":
+				return filepath.SkipDir
+			}
+			return nil
+		}
+
+		if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
+			return nil
+		}
+
+		rel, relErr := filepath.Rel(azdRoot, path)
+		if relErr != nil {
+			rel = path
+		}
+		rel = filepath.ToSlash(rel)
+
+		// The tracing/baggage plumbing is the sanctioned home for raw attribute
+		// construction; the fields.AttributeKey abstraction is built on it.
+		if strings.HasPrefix(rel, "internal/tracing/") {
+			return nil
+		}
+
+		fset := token.NewFileSet()
+		file, parseErr := parser.ParseFile(fset, path, nil, 0)
+		if parseErr != nil {
+			return nil // skip unparseable files
+		}
+
+		ast.Inspect(file, func(n ast.Node) bool {
+			call, ok := n.(*ast.CallExpr)
+			if !ok || len(call.Args) == 0 {
+				return true
+			}
+
+			sel, ok := call.Fun.(*ast.SelectorExpr)
+			if !ok {
+				return true
+			}
+
+			pkgIdent, ok := sel.X.(*ast.Ident)
+			if !ok || pkgIdent.Name != "attribute" {
+				return true
+			}
+
+			if _, isConstructor := rawAttributeConstructors[sel.Sel.Name]; !isConstructor {
+				return true
+			}
+
+			// Only flag string-literal keys. A non-literal key (e.g. a dynamic
+			// extension field) is a separate, intentional pattern.
+			lit, ok := call.Args[0].(*ast.BasicLit)
+			if !ok || lit.Kind != token.STRING {
+				return true
+			}
+
+			pos := fset.Position(call.Pos())
+			violations = append(violations, fmt.Sprintf(
+				"  %s:%d: attribute.%s(%s, ...)", rel, pos.Line, sel.Sel.Name, lit.Value))
+			return true
+		})
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	if len(violations) > 0 {
+		t.Errorf(
+			"Found %d raw telemetry attribute(s) using a string-literal key.\n"+
+				"Declare an exported fields.AttributeKey (with Classification and Purpose) in\n"+
+				"internal/tracing/fields/fields.go and emit via it, e.g. fields.MyKey.String(v),\n"+
+				"so the property is discoverable and classifiable by the GDPR metadata pipeline.\n\n"+
+				"Raw attributes:\n%s",
+			len(violations),
+			strings.Join(violations, "\n"),
+		)
+	}
 }
 
 // TestCommandTelemetryCoverage ensures every user-facing command is explicitly categorized

cli/azd/internal/tracing/fields/fields.go

@@ -360,6 +360,17 @@ var (
 		Classification: SystemMetadata,
 		Purpose:        FeatureInsight,
 	}
+
+	// AuthCacheClearFailedKey records which cache failed to clear during the
+	// re-login cleanup that runs before a fresh login. It is a fixed enum
+	// (not user-derived), so it is emitted raw (not hashed). Emitted on the
+	// `auth login` usage event.
+	// Values: "auth" (credential cache), "subscriptions" (subscription cache).
+	AuthCacheClearFailedKey = AttributeKey{
+		Key:            attribute.Key("auth.cache_clear_failed"),
+		Classification: SystemMetadata,
+		Purpose:        PerformanceAndHealth,
+	}
 )
 
 // Environment command related fields
@@ -1053,6 +1064,19 @@ var (
 	}
 )
 
+// AKS service target related fields
+var (
+	// AksSkipReasonKey records why AKS postprovision Kubernetes context setup
+	// was skipped, as a bounded, low-cardinality code (never raw error text).
+	// Emitted on the `aks.postprovision.skip` event.
+	// Values: "cluster_not_provisioned".
+	AksSkipReasonKey = AttributeKey{
+		Key:            attribute.Key("skip.reason"),
+		Classification: SystemMetadata,
+		Purpose:        FeatureInsight,
+	}
+)
+
 // Mcp related fields
 var (
 	// The name of the MCP client.
@@ -1129,6 +1153,17 @@ var (
 		Purpose:        FeatureInsight,
 		IsMeasurement:  true,
 	}
+
+	// ContainerPublishRemoteBuildKey records whether the container image was
+	// built remotely (ACR build) rather than locally for a container publish.
+	// It is a boolean (fixed cardinality), so it is emitted raw (not hashed).
+	// Emitted on the `container.publish` event. This is distinct from the
+	// `container.remotebuild` event, which marks the Azure-side remote build.
+	ContainerPublishRemoteBuildKey = AttributeKey{
+		Key:            attribute.Key("container.publish.remotebuild"),
+		Classification: SystemMetadata,
+		Purpose:        FeatureInsight,
+	}
 )
 
 // JSON-RPC related fields

cli/azd/pkg/project/container_helper.go

@@ -41,7 +41,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/pack"
 	"github.com/benbjohnson/clock"
 	"github.com/sethvargo/go-retry"
-	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -625,7 +624,7 @@ func (ch *ContainerHelper) Publish(
 	ctx, span := tracing.Start(ctx, events.ContainerPublishEvent)
 	defer func() { span.EndWithStatus(err) }()
 	span.SetAttributes(
-		attribute.Bool("container.remotebuild", serviceConfig.Docker.RemoteBuild),
+		fields.ContainerPublishRemoteBuildKey.Bool(serviceConfig.Docker.RemoteBuild),
 	)
 
 	var remoteImage string

cli/azd/pkg/project/service_target_aks.go

@@ -17,6 +17,7 @@ import (
 	"github.com/azure/azure-dev/cli/azd/internal/mapper"
 	"github.com/azure/azure-dev/cli/azd/internal/tracing"
 	"github.com/azure/azure-dev/cli/azd/internal/tracing/events"
+	"github.com/azure/azure-dev/cli/azd/internal/tracing/fields"
 	"github.com/azure/azure-dev/cli/azd/pkg/alpha"
 	"github.com/azure/azure-dev/cli/azd/pkg/async"
 	"github.com/azure/azure-dev/cli/azd/pkg/azapi"
@@ -32,7 +33,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/tools"
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/kubectl"
 	"github.com/sethvargo/go-retry"
-	"go.opentelemetry.io/otel/attribute"
 )
 
 const (
@@ -944,8 +944,7 @@ func (t *aksTarget) setK8sContext(
 	// (credentials, RBAC, namespace) are real failures that should surface
 	// even during postprovision.
 	if targetResource.ResourceName() == "" && eventName == postProvisionEvent {
-		return t.skipPostprovisionK8sSetup(
-			ctx, fmt.Errorf("AKS cluster resource not yet provisioned"))
+		return t.skipPostprovisionK8sSetup(ctx, aksSkipClusterNotProvisioned)
 	}
 
 	defaultNamespace := t.getK8sNamespace(serviceConfig)
@@ -973,13 +972,21 @@ func (t *aksTarget) setK8sContext(
 	return nil
 }
 
+// aksSkipClusterNotProvisioned is the bounded skip.reason code emitted on
+// events.AksPostprovisionSkipEvent when the AKS cluster resource does not exist
+// yet (delayed provisioning during multi-phase workflows) and Kubernetes
+// context setup is deferred until a deployment is performed. Reason codes must
+// stay a small, compile-time enum — never raw or user-derived text. Keep in
+// sync with docs/reference/telemetry-data.md.
+const aksSkipClusterNotProvisioned = "cluster_not_provisioned"
+
 // skipPostprovisionK8sSetup logs a warning and returns nil so that
-// postprovision continues even when Kubernetes context setup fails.
+// postprovision continues even when Kubernetes context setup is skipped.
 // The context will be configured later when a deployment is performed.
 // Context cancellation/timeouts are always propagated.
 func (t *aksTarget) skipPostprovisionK8sSetup(
 	ctx context.Context,
-	reason error,
+	reason string,
 ) error {
 	// Propagate context cancellation — the user (or system) asked
 	// to stop; swallowing that would be incorrect.
@@ -988,11 +995,11 @@ func (t *aksTarget) skipPostprovisionK8sSetup(
 	}
 
 	_, span := tracing.Start(ctx, events.AksPostprovisionSkipEvent)
-	span.SetAttributes(attribute.String("skip.reason", reason.Error()))
+	span.SetAttributes(fields.AksSkipReasonKey.String(reason))
 	span.End()
 
 	log.Printf(
-		"skipping k8s context setup during postprovision: %v", reason)
+		"skipping k8s context setup during postprovision: %s", reason)
 	t.console.Message(ctx, output.WithWarningFormat(
 		"AKS cluster not available yet, skipping Kubernetes "+
 			"context setup. It will be configured when the "+