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,14 @@
 package cmd
 
 import (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -26,7 +34,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 +56,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 +272,170 @@ 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.ContainerRemoteBuildKey.Bool(true)
+		require.Equal(t, "container.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(key, ...) / attribute.Bool(key, ...) etc. — whether
+// the key is a string literal or a named constant, and whether the package is
+// imported under its default name or an alias. 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).
+//   - 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 key and value. 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":          {},
+		"IntSlice":     {},
+		"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)
+
+		fset := token.NewFileSet()
+		file, parseErr := parser.ParseFile(fset, path, nil, 0)
+		if parseErr != nil {
+			return nil // skip unparseable files
+		}
+
+		// Resolve the local name bound to go.opentelemetry.io/otel/attribute in
+		// this file. Matching the selector base literally against "attribute"
+		// would miss an aliased import (e.g. otelattr "...otel/attribute") and
+		// could also misfire on an unrelated local identifier named "attribute".
+		// If the file does not import the package, it cannot construct a raw
+		// attribute, so there is nothing to scan.
+		attrPkgName := ""
+		for _, imp := range file.Imports {
+			importPath, uErr := strconv.Unquote(imp.Path.Value)
+			if uErr != nil || importPath != "go.opentelemetry.io/otel/attribute" {
+				continue
+			}
+			if imp.Name != nil {
+				attrPkgName = imp.Name.Name // explicit alias
+			} else {
+				attrPkgName = "attribute" // default package name
+			}
+			break
+		}
+		// A blank ("_") or dot (".") import cannot produce a "pkg.Constructor"
+		// selector, so there is nothing this AST check can match on.
+		if attrPkgName == "" || attrPkgName == "_" || attrPkgName == "." {
+			return nil
+		}
+
+		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 != attrPkgName {
+				return true
+			}
+
+			if _, isConstructor := rawAttributeConstructors[sel.Sel.Name]; !isConstructor {
+				return true
+			}
+
+			// Every direct attribute constructor call in product code bypasses
+			// the fields.AttributeKey registry, so the GDPR classifier can never
+			// see it — whether the key is a string literal or a named constant.
+			// Both are violations; surface the literal key when present for a
+			// friendlier message.
+			keyDesc := "..."
+			if lit, ok := call.Args[0].(*ast.BasicLit); ok && lit.Kind == token.STRING {
+				keyDesc = lit.Value
+			}
+
+			pos := fset.Position(call.Pos())
+			violations = append(violations, fmt.Sprintf(
+				"  %s:%d: %s.%s(%s, ...)", rel, pos.Line, attrPkgName, sel.Sel.Name, keyDesc))
+			return true
+		})
+
+		return nil
+	})
+	require.NoError(t, err)
+
+	if len(violations) > 0 {
+		t.Errorf(
+			"Found %d raw telemetry attribute(s) constructed directly.\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,15 @@ var (
 		Purpose:        FeatureInsight,
 		IsMeasurement:  true,
 	}
+
+	// ContainerRemoteBuildKey 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).
+	ContainerRemoteBuildKey = AttributeKey{
+		Key:            attribute.Key("container.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.ContainerRemoteBuildKey.Bool(serviceConfig.Docker.RemoteBuild),
 	)
 
 	var remoteImage string