diff --git a/cli/azd/AGENTS.md b/cli/azd/AGENTS.md index 42c48967c41..b601e8e0688 100644 --- a/cli/azd/AGENTS.md +++ b/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** diff --git a/cli/azd/cmd/auth_login.go b/cli/azd/cmd/auth_login.go index 853d6a239c8..fcaca2f1cee 100644 --- a/cli/azd/cmd/auth_login.go +++ b/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) } } diff --git a/cli/azd/cmd/telemetry_test.go b/cli/azd/cmd/telemetry_test.go index 380c21fb88f..e727ab477aa 100644 --- a/cli/azd/cmd/telemetry_test.go +++ b/cli/azd/cmd/telemetry_test.go @@ -4,8 +4,16 @@ package cmd import ( + "fmt" + "go/ast" + "go/token" + "go/types" + "path/filepath" + "strings" "testing" + "golang.org/x/tools/go/packages" + "github.com/stretchr/testify/require" "github.com/azure/azure-dev/cli/azd/internal" @@ -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,732 @@ 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 a raw attribute constructor — attribute.String(key, ...), attribute.Bool, +// the chained attribute.Key(key).String(...) form, or a KeyValue-producing method +// called on any value of type attribute.Key. 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). +// +// The scan is type-aware (go/types via go/packages) rather than purely +// syntactic. Type information is required for soundness: the sanctioned +// fields.SomeKey.String(v) is a promoted method whose receiver is the classified +// fields.AttributeKey struct, which is AST-indistinguishable from a bare +// attribute.Key value's method call. Only the resolved types tell the two named +// types apart, and they also let the guard follow keys reached through import +// aliases, dot imports, struct fields, or function results. +// +// Legitimately excluded from the scan: +// - *_test.go files (test fixtures build raw attributes on purpose): Tests is +// false, so go/packages does not load them. +// - Nested modules (extensions/*, test/evals, test data samples) have their own +// go.mod and are not matched by the "./..." pattern. Extension telemetry is +// out of scope by design, not merely by mechanics: extensions are separate +// modules whose attributes (the "ext.*" namespace) are reviewed together with +// the extension that reports them, per docs/specs/metrics-audit/ +// privacy-review-checklist.md, rather than against the core fields catalog. +func TestNoRawTelemetryAttributes(t *testing.T) { + t.Parallel() + + // 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) + + cfg := &packages.Config{ + Mode: packages.NeedName | packages.NeedFiles | packages.NeedSyntax | + packages.NeedTypes | packages.NeedTypesInfo | packages.NeedImports, + Dir: azdRoot, + Tests: false, + } + pkgs, err := packages.Load(cfg, "./...") + require.NoError(t, err) + require.NotEmpty(t, pkgs, "no packages loaded from the cli/azd module") + + var ( + violations []string + loadErrors []string + ) + for _, pkg := range pkgs { + for _, e := range pkg.Errors { + loadErrors = append(loadErrors, fmt.Sprintf(" %s: %s", pkg.PkgPath, e.Error())) + } + if pkg.TypesInfo == nil { + continue + } + for _, file := range pkg.Syntax { + filename := pkg.Fset.Position(file.Pos()).Filename + rel, relErr := filepath.Rel(azdRoot, filename) + if relErr != nil { + rel = filename + } + rel = filepath.ToSlash(rel) + violations = append(violations, scanFileForRawAttributes(pkg.Fset, file, pkg.TypesInfo, pkg.PkgPath, rel)...) + } + } + + // A package that failed to type-check would silently hide violations, so a + // load error is a failure rather than a false pass. + require.Empty(t, loadErrors, "packages failed to load/type-check:\n%s", strings.Join(loadErrors, "\n")) + + 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"), + ) + } +} + +// rawAttributePkgPath is the import path of the OpenTelemetry attribute package +// whose constructors and Key methods bypass the fields.AttributeKey registry. +const rawAttributePkgPath = "go.opentelemetry.io/otel/attribute" + +// isRawAttributeKeyType reports whether t is exactly the +// go.opentelemetry.io/otel/attribute.Key named type. A struct that merely embeds +// it — such as the sanctioned fields.AttributeKey — is a different named type and +// returns false, which is what keeps the guard from flagging fields.SomeKey.String. +func isRawAttributeKeyType(t types.Type) bool { + named, ok := t.(*types.Named) + if !ok { + return false + } + obj := named.Obj() + return obj != nil && obj.Pkg() != nil && + obj.Pkg().Path() == rawAttributePkgPath && obj.Name() == "Key" +} + +// isRawAttributeKeyValueType reports whether t is exactly the +// go.opentelemetry.io/otel/attribute.KeyValue named struct. Constructing one +// directly with a key literal bypasses both the attribute constructors and the +// fields registry, so the guard inspects these composite literals too. +func isRawAttributeKeyValueType(t types.Type) bool { + named, ok := t.(*types.Named) + if !ok { + return false + } + obj := named.Obj() + return obj != nil && obj.Pkg() != nil && + obj.Pkg().Path() == rawAttributePkgPath && obj.Name() == "KeyValue" +} + +// fieldsPkgPath is the import path of the package that defines the sanctioned +// classified attribute wrapper, fields.AttributeKey. +const fieldsPkgPath = "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" + +// isFieldsAttributeKeyType reports whether t is exactly the sanctioned +// fields.AttributeKey named type. That wrapper is the only type the metadata +// classifier discovers and reads Classification/Purpose/Endpoint from, so it is +// the only receiver on which a KeyValue-producing attribute.Key method is +// allowed. A bare attribute.Key, or any other struct that merely embeds +// attribute.Key, produces a key the classifier cannot see and is a violation. +func isFieldsAttributeKeyType(t types.Type) bool { + named, ok := t.(*types.Named) + if !ok { + return false + } + obj := named.Obj() + return obj != nil && obj.Pkg() != nil && + obj.Pkg().Path() == fieldsPkgPath && obj.Name() == "AttributeKey" +} + +// isAttributeKeyBuilderMethod reports whether m is a method defined on +// attribute.Key. The check is on the method's defining receiver type, so it +// matches whether the method is invoked directly on an attribute.Key value or +// promoted through an embedding struct, and it never matches an unrelated +// String()/Stringer() method on some other type. Callers additionally restrict +// to the KeyValue-producing names (rawAttributeConstructors) so attribute.Key's +// non-builder methods (e.g. Defined) are not flagged. +func isAttributeKeyBuilderMethod(m *types.Func) bool { + sig, ok := m.Type().(*types.Signature) + if !ok || sig.Recv() == nil { + return false + } + return isRawAttributeKeyType(sig.Recv().Type()) +} + +// scanFileForRawAttributes returns the raw-telemetry-attribute violations in a +// single type-checked Go file. info must be the go/types information for the +// file's package; pkgPath is that package's import path (used to exempt the +// fields package from the construction rule below); rel is the display path used +// in messages. Classifying calls by the resolved type of the callee and receiver +// — rather than by syntactic shape — makes the guard sound across import aliases, +// dot imports, and keys reached through parameters, struct fields, or function +// results. It is shared by TestNoRawTelemetryAttributes (which walks the module) +// and the fixture test TestRawTelemetryAttributeScanner, so the guard's contract +// is itself tested. +func scanFileForRawAttributes(fset *token.FileSet, file *ast.File, info *types.Info, pkgPath, rel string) []string { + // rawAttributeConstructors are the go.opentelemetry.io/otel/attribute helpers + // (and the identically named attribute.Key methods) that build a KeyValue from + // a key and a 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": {}, + } + + // literalKeyExpr renders a constant key expression for a friendlier message, + // preferring its resolved constant string value (covers both "k" and + // attribute.Key("k") forms), and falling back to "...". + literalKeyExpr := func(expr ast.Expr) string { + if tv, ok := info.Types[expr]; ok && tv.Value != nil { + return tv.Value.String() + } + return "..." + } + + // isRawAttributeConstructorFunc reports whether obj is a package-level function + // of the attribute package that builds a KeyValue from a key and value (e.g. + // attribute.String). Resolving via the types object makes this independent of + // how the package was imported: default name, alias, or dot import. + isRawAttributeConstructorFunc := func(obj types.Object) bool { + fn, ok := obj.(*types.Func) + if !ok || fn.Pkg() == nil || fn.Pkg().Path() != rawAttributePkgPath { + return false + } + sig, ok := fn.Type().(*types.Signature) + if !ok || sig.Recv() != nil { + return false + } + _, isCtor := rawAttributeConstructors[fn.Name()] + return isCtor + } + + // isReemittedKeyValueKey reports whether expr is `.Key` where has type + // attribute.KeyValue — i.e. a method call like kv.Key.String(v) merely re-emits + // the key of a KeyValue that was already built (and, at its build site, already + // subject to this guard). The telemetry baggage plumbing in + // internal/tracing legitimately rebuilds caller-supplied KeyValues with merged + // values this way; it introduces no new key literal, so it is not a violation. + isReemittedKeyValueKey := func(expr ast.Expr) bool { + sel, ok := expr.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Key" { + return false + } + named, ok := info.TypeOf(sel.X).(*types.Named) + if !ok { + return false + } + obj := named.Obj() + return obj != nil && obj.Pkg() != nil && + obj.Pkg().Path() == rawAttributePkgPath && obj.Name() == "KeyValue" + } + + // constantKey reports whether expr evaluates to a compile-time constant. A + // KeyValue whose Key is a constant introduces a fixed key literal, whereas a + // non-constant Key (a variable, a k.Key field access, a function result) + // merely forwards an existing key. This is what distinguishes a new raw key + // from the fields/baggage plumbing that only re-emits caller-supplied keys. + constantKey := func(expr ast.Expr) bool { + tv, ok := info.Types[expr] + return ok && tv.Value != nil + } + + // fabricatesRawKey reports whether the Key expression of an attribute.KeyValue + // literal mints a new key rather than forwarding an existing one. A + // compile-time constant (a "literal" or attribute.Key("literal")) is a fixed + // key literal. An explicit attribute.Key(x) conversion also fabricates a key — + // even when x is only known at run time — because it turns an arbitrary string + // into a key the classifier never sees; the sole sanctioned way to build a + // dynamic key is fields.ExtensionUsageAttribute. Forwarding forms (a bare + // attribute.Key variable, or a k.Key field access) are neither constant nor a + // conversion, so the fields/baggage plumbing that re-emits caller-supplied keys + // is left alone. + fabricatesRawKey := func(expr ast.Expr) bool { + if constantKey(expr) { + return true + } + call, ok := expr.(*ast.CallExpr) + if !ok { + return false + } + tv, ok := info.Types[call.Fun] + return ok && tv.IsType() && isRawAttributeKeyType(tv.Type) + } + + // keyValueLiteralKey returns the expression assigned to the Key field of an + // attribute.KeyValue composite literal, handling both keyed + // (KeyValue{Key: ...}) and positional (KeyValue{k, v}) forms. It returns nil + // when no key element is present (the zero value). + keyValueLiteralKey := func(cl *ast.CompositeLit) ast.Expr { + if len(cl.Elts) == 0 { + return nil + } + if _, keyed := cl.Elts[0].(*ast.KeyValueExpr); keyed { + for _, elt := range cl.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + continue + } + if id, ok := kv.Key.(*ast.Ident); ok && id.Name == "Key" { + return kv.Value + } + } + return nil + } + // Positional: Key is the first field of attribute.KeyValue. + return cl.Elts[0] + } + + // rawAttributeBuilderSelection reports whether sel selects a KeyValue-producing + // builder (String/Bool/…) defined on the raw attribute.Key type, on a receiver + // that is NOT the classified fields.AttributeKey. It matches both a method + // value (recvExpr.String) and a method expression (attribute.Key.String) so the + // guard also covers a call written in method-expression form and a builder + // captured as a function value (builder := attribute.Key.String). A method + // value that only re-emits an existing KeyValue's key (kv.Key.String) is not a + // new key and returns false. + rawAttributeBuilderSelection := func(sel *ast.SelectorExpr) bool { + selection := info.Selections[sel] + if selection == nil { + return false + } + kind := selection.Kind() + if kind != types.MethodVal && kind != types.MethodExpr { + return false + } + m, ok := selection.Obj().(*types.Func) + if !ok { + return false + } + if _, isCtor := rawAttributeConstructors[m.Name()]; !isCtor || !isAttributeKeyBuilderMethod(m) { + return false + } + if isFieldsAttributeKeyType(selection.Recv()) { + return false + } + if kind == types.MethodVal && isReemittedKeyValueKey(sel.X) { + return false + } + return true + } + + var violations []string + ast.Inspect(file, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.CompositeLit: + pos := fset.Position(node.Pos()) + clType := info.TypeOf(node) + // Constructing a fields.AttributeKey outside the fields package + // fabricates a key the classifier never sees. The GDPR scanner discovers + // only the exported package-level AttributeKey vars declared in the + // fields package, so a locally built one — e.g. + // fields.AttributeKey{Key: attribute.Key("x")} — carries an uncatalogued + // key even though its type is fields.AttributeKey, and the method branch + // below would (correctly, to keep registered keys passed through + // parameters valid) treat that receiver type as sanctioned. The only + // sanctioned ways to obtain an AttributeKey in product code are to + // reference a registered fields.* var or to call + // fields.ExtensionUsageAttribute; both live in the fields package, which + // is why that package is exempt here. + if pkgPath != fieldsPkgPath && isFieldsAttributeKeyType(clType) { + violations = append(violations, fmt.Sprintf( + " %s:%d: fields.AttributeKey{...} constructed outside the fields "+ + "package (its key is not in the classifier catalog; reference a "+ + "registered fields.* key or fields.ExtensionUsageAttribute)", rel, pos.Line)) + return true + } + // A raw attribute.KeyValue struct literal whose Key is fabricated + // introduces an unclassified key directly (e.g. + // attribute.KeyValue{Key: attribute.Key("raw.key"), Value: ...}, or the + // same with a run-time attribute.Key(x) conversion), which + // span.SetAttributes would emit without ever passing through a + // fields.AttributeKey. The plumbing that legitimately builds KeyValues + // (fields.StringHashed and baggage re-emission) sets Key from an existing + // key expression (k.Key, a ranged variable), which is neither a constant + // nor a conversion and is therefore left alone. + if isRawAttributeKeyValueType(clType) { + if keyExpr := keyValueLiteralKey(node); keyExpr != nil && fabricatesRawKey(keyExpr) { + violations = append(violations, fmt.Sprintf( + " %s:%d: attribute.KeyValue{Key: %s, ...} introduces an unclassified "+ + "key literal (build it from a fields.AttributeKey instead)", + rel, pos.Line, literalKeyExpr(keyExpr))) + } + } + return true + case *ast.SelectorExpr: + if rawAttributeBuilderSelection(node) { + pos := fset.Position(node.Pos()) + violations = append(violations, fmt.Sprintf( + " %s:%d: attribute.Key builder .%s used on a non-fields.AttributeKey "+ + "receiver (produces an unclassified key)", rel, pos.Line, node.Sel.Name)) + } + return true + case *ast.Ident: + // A reference to a package-level attribute constructor — attribute.String, + // an aliased import of it, or a dot-imported String — bypasses the + // fields.AttributeKey registry. Resolving the identifier's object flags + // every form: the selector's Sel in attribute.String(...), a bare + // dot-imported String(...), and, crucially, the constructor captured as a + // function value (builder := attribute.String; builder("raw.key", v)), + // which a call-position-only check would miss. attribute.Key's methods are + // handled by the *ast.SelectorExpr case above and are excluded here because + // isRawAttributeConstructorFunc rejects any func with a receiver. + if isRawAttributeConstructorFunc(info.Uses[node]) { + pos := fset.Position(node.Pos()) + violations = append(violations, fmt.Sprintf( + " %s:%d: %s is a raw attribute constructor (build telemetry from a "+ + "fields.AttributeKey instead)", rel, pos.Line, node.Name)) + } + return true + } + return true + }) + + return violations +} + +// TestRawTelemetryAttributeScanner is a fixture test for the type-aware guard +// used by TestNoRawTelemetryAttributes. It pins the contract: raw attribute +// constructors (default, aliased, dot-imported, and captured as a function +// value), constant keys, the chained attribute.Key(k).String(v) form, KeyValue- +// producing attribute.Key builders reached as a method value (through a +// parameter, local, struct field, or function result) or as a method expression +// (attribute.Key.String, including when captured as a function value), a locally +// constructed fields.AttributeKey, and a raw attribute.KeyValue literal whose Key +// is fabricated (a constant, or a run-time attribute.Key(x) conversion) are all +// flagged; while the sanctioned promoted-method call on the classified +// fields.AttributeKey, fields.ExtensionUsageAttribute, an attribute.KeyValue that +// only re-emits an existing (forwarded) key, a bare attribute.Key(k) +// conversion (which does not build a KeyValue), and non-KeyValue uses of a key +// (e.g. a map lookup) are not. Fixtures are type-checked against the real +// attribute and fields packages via go/packages, so the guard runs with the same +// type information it uses on the module. +func TestRawTelemetryAttributeScanner(t *testing.T) { + t.Parallel() + + root, err := filepath.Abs("..") + require.NoError(t, err) + + cases := []struct { + name string + src string + wantViolation bool + }{ + { + name: "literal key", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.String("raw.key", "v") +`, + wantViolation: true, + }, + { + name: "constant key", + src: `package p +import "go.opentelemetry.io/otel/attribute" +const k = "raw.key" +var _ = attribute.String(k, "v") +`, + wantViolation: true, + }, + { + name: "aliased import", + src: `package p +import otelattr "go.opentelemetry.io/otel/attribute" +var _ = otelattr.Bool("raw.key", true) +`, + wantViolation: true, + }, + { + name: "dot-imported constructor", + src: `package p +import . "go.opentelemetry.io/otel/attribute" +var _ = String("raw.key", "v") +`, + wantViolation: true, + }, + { + name: "constructor captured as function value", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var builder = attribute.String +var _ = builder("raw.key", "v") +`, + wantViolation: true, + }, + { + name: "int slice constructor", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.IntSlice("raw.key", []int{1}) +`, + wantViolation: true, + }, + { + name: "chained key method", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.Key("raw.key").String("v") +`, + wantViolation: true, + }, + { + name: "raw attribute.KeyValue struct literal", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.KeyValue{Key: attribute.Key("raw.key"), Value: attribute.StringValue("v")} +`, + wantViolation: true, + }, + { + name: "raw attribute.KeyValue positional literal", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.KeyValue{attribute.Key("raw.key"), attribute.StringValue("v")} +`, + wantViolation: true, + }, + { + name: "attribute.KeyValue with run-time key conversion", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func f(runtimeKey string) attribute.KeyValue { + return attribute.KeyValue{Key: attribute.Key(runtimeKey), Value: attribute.StringValue("v")} +} +`, + wantViolation: true, + }, + { + name: "attribute.KeyValue re-emitting an existing key", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func f(k attribute.Key, v attribute.Value) attribute.KeyValue { + return attribute.KeyValue{Key: k, Value: v} +} +`, + wantViolation: false, + }, + { + name: "attribute.Key builder method expression call", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.Key.String(attribute.Key("raw.key"), "v") +`, + wantViolation: true, + }, + { + name: "attribute.Key builder captured as function value", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var builder = attribute.Key.String +var _ = builder(attribute.Key("raw.key"), "v") +`, + wantViolation: true, + }, + { + name: "method on key-typed parameter", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func f(k attribute.Key) { _ = k.String("v") } +`, + wantViolation: true, + }, + { + name: "method on locally built key value", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func f() { k := attribute.Key("raw.key"); _ = k.String("v") } +`, + wantViolation: true, + }, + { + name: "method on inferred key declaration", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var k = attribute.Key("raw.key") +var _ = k.String("v") +`, + wantViolation: true, + }, + { + name: "method on struct-field key", + src: `package p +import "go.opentelemetry.io/otel/attribute" +type holder struct{ key attribute.Key } +func f(h holder) { _ = h.key.String("v") } +`, + wantViolation: true, + }, + { + name: "method on function-result key", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func mk() attribute.Key { return attribute.Key("raw.key") } +func f() { _ = mk().String("v") } +`, + wantViolation: true, + }, + { + name: "method via embedded Key field of a struct", + src: `package p +import "go.opentelemetry.io/otel/attribute" +type wrapper struct{ attribute.Key } +func f(c wrapper) { _ = c.Key.Bool(true) } +`, + wantViolation: true, + }, + { + name: "reemit method on KeyValue key field", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func f(kv attribute.KeyValue) { _ = kv.Key.String("v") } +`, + wantViolation: false, + }, + { + name: "bare key conversion without value", + src: `package p +import "go.opentelemetry.io/otel/attribute" +var _ = attribute.Key("raw.key") +`, + wantViolation: false, + }, + { + name: "promoted method on non-fields embedding struct", + src: `package p +import "go.opentelemetry.io/otel/attribute" +type wrapper struct{ attribute.Key } +var c wrapper +var _ = c.String("v") +`, + wantViolation: true, + }, + { + name: "promoted method on classified fields.AttributeKey", + src: `package p +import "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" +var _ = fields.ServiceNameKey.String("v") +`, + wantViolation: false, + }, + { + name: "dynamic sanctioned extension usage attribute", + src: `package p +import "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" +var _ = fields.ExtensionUsageAttribute("foo").String("v") +`, + wantViolation: false, + }, + { + name: "locally constructed unregistered fields.AttributeKey emitted inline", + src: `package p +import "go.opentelemetry.io/otel/attribute" +import "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" +var _ = fields.AttributeKey{Key: attribute.Key("raw.key")}.String("v") +`, + wantViolation: true, + }, + { + name: "unregistered fields.AttributeKey via local variable", + src: `package p +import "go.opentelemetry.io/otel/attribute" +import "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" +func f() { + k := fields.AttributeKey{Key: attribute.Key("raw.key")} + _ = k.Bool(true) +} +`, + wantViolation: true, + }, + { + name: "shadowed classified key across scopes", + src: `package p +import "go.opentelemetry.io/otel/attribute" +import "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" +func a(k attribute.Key) { _ = k } +func b(k fields.AttributeKey) { _ = k.Bool(true) } +`, + wantViolation: false, + }, + { + name: "map lookup on key-typed parameter", + src: `package p +import "go.opentelemetry.io/otel/attribute" +func f(m map[attribute.Key]int, k attribute.Key) int { return m[k] } +`, + wantViolation: false, + }, + { + name: "file without the attribute import", + src: `package p +var _ = 1 +`, + wantViolation: false, + }, + } + + // Type-check every fixture against the real attribute package via an in-memory + // overlay. The virtual files live under a directory that does not exist on + // disk, so they neither collide with the module walk nor require cleanup. + overlay := make(map[string][]byte, len(cases)) + patterns := make([]string, 0, len(cases)) + pathToCase := make(map[string]int, len(cases)) + for i, tc := range cases { + p := filepath.Join(root, "cmd", fmt.Sprintf("zz_rawscan_fixture_%02d", i), "fixture.go") + overlay[p] = []byte(tc.src) + patterns = append(patterns, "file="+p) + pathToCase[filepath.ToSlash(p)] = i + } + + cfg := &packages.Config{ + Mode: packages.NeedName | packages.NeedFiles | packages.NeedSyntax | + packages.NeedTypes | packages.NeedTypesInfo | packages.NeedImports, + Dir: root, + Overlay: overlay, + } + pkgs, err := packages.Load(cfg, patterns...) + require.NoError(t, err) + + got := make([]bool, len(cases)) + checked := make([]bool, len(cases)) + for _, pkg := range pkgs { + require.Empty(t, pkg.Errors, "fixture package %s failed to type-check", pkg.PkgPath) + if pkg.TypesInfo == nil { + continue + } + for _, file := range pkg.Syntax { + fname := filepath.ToSlash(pkg.Fset.Position(file.Pos()).Filename) + idx, ok := pathToCase[fname] + if !ok { + continue + } + got[idx] = len(scanFileForRawAttributes(pkg.Fset, file, pkg.TypesInfo, pkg.PkgPath, cases[idx].name)) > 0 + checked[idx] = true + } + } + + for i, tc := range cases { + require.True(t, checked[i], "fixture %q was not loaded/type-checked", tc.name) + require.Equal(t, tc.wantViolation, got[i], "fixture %q: unexpected violation result", tc.name) + } } // TestCommandTelemetryCoverage ensures every user-facing command is explicitly categorized diff --git a/cli/azd/go.mod b/cli/azd/go.mod index bb372f4d03a..89094102181 100644 --- a/cli/azd/go.mod +++ b/cli/azd/go.mod @@ -86,6 +86,7 @@ require ( golang.org/x/sys v0.46.0 golang.org/x/term v0.44.0 golang.org/x/time v0.9.0 + golang.org/x/tools v0.45.0 google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 google.golang.org/grpc v1.82.1 google.golang.org/protobuf v1.36.11 @@ -148,6 +149,7 @@ require ( go.opentelemetry.io/proto/otlp v1.10.0 // indirect golang.org/x/crypto v0.53.0 // indirect golang.org/x/exp v0.0.0-20250911091902-df9299821621 // indirect + golang.org/x/mod v0.36.0 // indirect golang.org/x/net v0.56.0 // indirect golang.org/x/text v0.38.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 // indirect diff --git a/cli/azd/go.sum b/cli/azd/go.sum index 49853c170fa..fa66976b57b 100644 --- a/cli/azd/go.sum +++ b/cli/azd/go.sum @@ -356,6 +356,8 @@ golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsi golang.org/x/exp v0.0.0-20250911091902-df9299821621 h1:2id6c1/gto0kaHYyrixvknJ8tUK/Qs5IsmBtrc+FtgU= golang.org/x/exp v0.0.0-20250911091902-df9299821621/go.mod h1:TwQYMMnGpvZyc+JpB/UAuTNIsVJifOlSkrZkhcvpVUk= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= +golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= @@ -395,6 +397,8 @@ golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8= +golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= diff --git a/cli/azd/internal/tracing/fields/fields.go b/cli/azd/internal/tracing/fields/fields.go index 161e3cbd4fd..52c38d769d0 100644 --- a/cli/azd/internal/tracing/fields/fields.go +++ b/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 diff --git a/cli/azd/pkg/azapi/webapp.go b/cli/azd/pkg/azapi/webapp.go index aed9a4d708c..86550e495df 100644 --- a/cli/azd/pkg/azapi/webapp.go +++ b/cli/azd/pkg/azapi/webapp.go @@ -189,7 +189,7 @@ func (cli *AzureClient) DeployAppServiceZip( } isLinux := isLinuxWebApp(app) - span.SetAttributes(fields.DeployLinuxKey.Key.Bool(isLinux)) + span.SetAttributes(fields.DeployLinuxKey.Bool(isLinux)) // Deployment Status API only support linux web app for now if isLinux && !skipStatusCheck && !isAppStopped(app) { @@ -200,7 +200,7 @@ func (cli *AzureClient) DeployAppServiceZip( // entire zip deploy when the build fails, giving the SCM time to stabilize. const maxBuildRetries = 2 for attempt := range maxBuildRetries + 1 { - span.SetAttributes(fields.DeployAttemptKey.Key.Int(attempt + 1)) + span.SetAttributes(fields.DeployAttemptKey.Int(attempt + 1)) if attempt > 0 { // Exponential backoff: 5s, 10s between retries to avoid hammering diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index 52c6b2d941e..95085ca68cb 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/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" ) @@ -623,10 +622,15 @@ func (ch *ContainerHelper) Publish( options *PublishOptions, ) (_ *ServicePublishResult, err error) { ctx, span := tracing.Start(ctx, events.ContainerPublishEvent) - defer func() { span.EndWithStatus(err) }() - span.SetAttributes( - attribute.Bool("container.remotebuild", serviceConfig.Docker.RemoteBuild), - ) + // Record whether the image was actually built remotely. It starts from the + // configured preference and is corrected below when a remote build fails and + // we fall back to a local build, so the attribute reflects the method used + // rather than the one requested (matching the field's documented meaning). + remoteBuildUsed := serviceConfig.Docker.RemoteBuild + defer func() { + span.SetAttributes(fields.ContainerRemoteBuildKey.Bool(remoteBuildUsed)) + span.EndWithStatus(err) + }() var remoteImage string @@ -651,6 +655,7 @@ func (ch *ContainerHelper) Publish( "Remote build failed: %s\nFalling back to local Docker build.", err), HidePrefix: false, }) + remoteBuildUsed = false remoteImage, err = ch.publishLocalImage( ctx, serviceConfig, serviceContext, env, progress, imageOverride) } diff --git a/cli/azd/pkg/project/service_target_aks.go b/cli/azd/pkg/project/service_target_aks.go index bd85ec71a1c..e2a1a3d718d 100644 --- a/cli/azd/pkg/project/service_target_aks.go +++ b/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 "+ diff --git a/docs/guides/feature-telemetry.md b/docs/guides/feature-telemetry.md index cc06b6eae7c..b76993667bf 100644 --- a/docs/guides/feature-telemetry.md +++ b/docs/guides/feature-telemetry.md @@ -43,16 +43,29 @@ const ( > `events.GetCommandEventName(...)`. You only need to define explicit event constants for > non-command operations (sub-spans, background work, etc.). +> [!IMPORTANT] +> The GDPR classifier discovers events by statically scanning the `events` package for **exported +> string constants whose Go identifier contains `Event`** (e.g. `MyFeatureEvent`). A constant that +> omits `Event` from its identifier is silently skipped and never classified, even if it is emitted. +> End the identifier with `Prefix` (e.g. `MyFeatureEventPrefix`) to register a prefix group that +> classifies every event name starting with that prefix. See +> [Telemetry Schema → Event discovery contract](../specs/metrics-audit/telemetry-schema.md#event-discovery-contract). + ## Step 2: Define Your Fields **File:** `cli/azd/internal/tracing/fields/fields.go` -Add `AttributeKey` variables for any new properties your feature emits. Every field must have: +Add **exported, package-level** `AttributeKey` variables for any new properties your feature emits +(the GDPR classifier only discovers exported `AttributeKey` vars declared in the `fields` package — +see [Field discovery contract](../specs/metrics-audit/telemetry-schema.md#field-attribute-discovery-contract)). +Every field must have: 1. **A key name** — descriptive, dot-separated, lowercase 2. **A classification** — what kind of data is this (see [Data Classifications](#data-classifications)) 3. **A purpose** — why are we collecting it (see [Purposes](#purposes)) -4. **`IsMeasurement: true`** if the value is numeric (goes to `Measurements` column, not `Properties`) +4. **`IsMeasurement: true`** if the value is numeric (goes to the `Measurements` column, not `Properties`) +5. **`Endpoint`** (optional) — an identifier-type tag to set when the value is a known endpoint + identifier; leave unset otherwise ```go // In fields.go — add your field keys @@ -106,6 +119,16 @@ tracing.SetUsageAttributes( ## Step 3: Instrument Your Code +> [!IMPORTANT] +> **Always emit through a `fields.AttributeKey` method — never a raw `attribute.*` call with a literal key.** +> Use `fields.MyFeatureStrategyKey.String(v)`, not `attribute.String("myfeature.strategy", v)`. +> The GDPR classification tool discovers fields by statically scanning `fields.go` for exported +> `AttributeKey` variables and reading their `Classification`/`Purpose`. A raw `attribute.*` call +> with a literal key is invisible to that scan, so the property still flows to App Insights but its +> data-catalog row stays **Unclassified / `Complete=false`**. The `TestNoRawTelemetryAttributes` guard +> (`cli/azd/cmd/telemetry_test.go`) fails the build on raw string-literal keys in +> product code. Dynamic, non-fixed keys (e.g. the `ext.*` extension path) are the only exception. + ### For Command Actions The telemetry middleware (`cmd/middleware/telemetry.go`) automatically creates a span for every command. You just need to add your feature-specific attributes: diff --git a/docs/reference/telemetry-data.md b/docs/reference/telemetry-data.md index 63fbbf1fb33..520ee6fc0ac 100644 --- a/docs/reference/telemetry-data.md +++ b/docs/reference/telemetry-data.md @@ -306,6 +306,7 @@ Set **only when an external command-line tool invocation fails**, during error c | Field Key | Type | Values | |-----------|------|--------| | `auth.method` | string | `browser`, `device-code`, `service-principal-secret`, `service-principal-certificate`, `federated-github`, `federated-azure-pipelines`, `federated-oidc`, `managed-identity`, `external`, `oneauth`, `check-status` | +| `auth.cache_clear_failed` | string | `auth`, `subscriptions` — which cache failed to clear during the pre-login cleanup. Emitted on `auth login`. |
@@ -426,6 +427,15 @@ Emitted at provision start by the `microsoft.foundry` provisioning provider (the | Field Key | Type | Description | |-----------|------|-------------| | `container.remoteBuild.count` | measurement | Number of remote container builds performed | +| `container.remotebuild` | bool | Whether the image was built remotely (ACR) rather than locally. | +
+ +
+AKS + +| Field Key | Type | Description | +|-----------|------|-------------| +| `skip.reason` | string | Why AKS postprovision Kubernetes context setup was skipped. Bounded enum: `cluster_not_provisioned`. Emitted on `aks.postprovision.skip`. |
@@ -785,7 +795,7 @@ How to find telemetry for a given feature area. Start here if you know the featu | **Execution Environment** | All events | `execution.environment` | Usage by environment, CI vs local | | **Self-Update** | `cmd.update` | `update.installMethod`, `update.fromVersion` | Update adoption | | **Hooks** | `hooks.exec` | `hooks.name`, `hooks.type`, `hooks.kind` | Hook usage by type | -| **Container Build** | `container.publish`, `container.remotebuild`, `tools.pack.build` | `pack.builder.image` | Build method usage, success rates | +| **Container Build** | `container.publish`, `container.remotebuild`, `tools.pack.build` | `pack.builder.image`, `container.remotebuild` | Build method usage (local vs. remote ACR build), success rates | | **App Detection (Aspire polyglot)** | `aspire.apphost.unsupported` | `aspire.apphost.language` (`typescript`/`python`/`go`/`java`/`rust`) | How often an unsupported Aspire polyglot (non-C#) AppHost is encountered, by language. **Emitted only during app detection for `init` and fresh `up` (no existing `azure.yaml`)** — not for already-initialized projects, so absence does not mean zero unsupported AppHosts. | | **Tool Management (`azd tool`)** | `cmd.tool.install`, `cmd.tool.update`, `cmd.tool.uninstall`, `cmd.tool.check` | `tool.id`, `tool.install.strategy` | Install/update/uninstall success, update availability | diff --git a/docs/specs/metrics-audit/feature-telemetry-matrix.md b/docs/specs/metrics-audit/feature-telemetry-matrix.md index 99f59d393d6..23d5c118f18 100644 --- a/docs/specs/metrics-audit/feature-telemetry-matrix.md +++ b/docs/specs/metrics-audit/feature-telemetry-matrix.md @@ -43,7 +43,7 @@ These commands emit attributes or events beyond the global middleware span. | Command | Subcommands | Global Span | Command-Specific Attrs | Feature Events | Notes | |---------|-------------|:-----------:|:----------------------:|:--------------:|-------| | **Auth** | | | | | | -| `auth login` | — | ✅ | ✅ | ❌ | `auth.method` (browser, device-code, service-principal-secret, etc.) | +| `auth login` | — | ✅ | ✅ | ❌ | `auth.method` (browser, device-code, service-principal-secret, etc.); `auth.cache_clear_failed` (which credential cache failed to clear during pre-login cleanup) | | `auth logout` | — | ✅ | ❌ | ❌ | Global telemetry sufficient — no command-specific attributes emitted | | `auth status` | — | ✅ | ❌ | ❌ | Global telemetry sufficient — simple pass/fail check | | `auth token` | — | ✅ | ❌ | ❌ | Global telemetry sufficient | @@ -116,6 +116,7 @@ command-specific telemetry fields provide analytical value beyond the command na | Field | OTel Key | Commands | Justification | |-------|----------|----------|---------------| | Auth method | `auth.method` | `auth login`, `auth logout` | Distinguishes authentication flow type (browser, device-code, SP, federated, etc.) | +| Auth cache-clear failed | `auth.cache_clear_failed` | `auth login` | Identifies which credential cache failed to clear during the pre-login cleanup (`auth` / `subscriptions`) | | Env count | `env.count` | `env list` | Measurement — number of environments is a quantitative metric | | Hooks name | `hooks.name` | `hooks run` | Identifies which hook script ran (hashed — user-defined name) | | Hooks type | `hooks.type` | `hooks run` | Distinguishes project / service / **layer** hooks | @@ -161,9 +162,9 @@ reserved field contracts. | **ARM deployment client** | `provision` (any Bicep flow) | `arm.deploy.subscription`, `arm.deploy.resourcegroup`, `arm.stack.deploy.subscription`, `arm.stack.deploy.resourcegroup`, `arm.whatif.subscription`, `arm.whatif.resourcegroup`, `arm.validate.subscription`, `arm.validate.resourcegroup` | ARM operation status + duration | Per-call instrumentation in the ARM client; covers regular + stack deployments at both scopes | | **Multi-layer provision** | `provision` (when `infra.layers[]` is configured in `azure.yaml`) | (none — enriches the `provision` span) | `provision.layer.count`, `provision.layer.max_parallel`, `provision.layer.safe_fallback_count`, `provision.layer.explicit_dependson_count` | All four are integer measurements emitted from `internal/cmd/provision_graph.go`; no per-layer duration or outcome attribute is emitted | | **Execution graph (scheduler)** | `up`, `provision`, `deploy`, `package`, `publish`, `down` | `exegraph.run`, `exegraph.step` | `exegraph.step.count`, `exegraph.max_concurrency`, `exegraph.error_policy`, `exegraph.step.name` (hashed), `exegraph.step.deps` (hashed slice), `exegraph.step.tags` (raw — hardcoded literals only), `exegraph.step.timeout_s` | Step names embed user-defined service / layer names from `azure.yaml`; both `name` and `deps` use `fields.StringHashed` / `fields.StringSliceHashed` | -| **Container lifecycle** | `package`, `deploy` (container service targets) | `container.credentials`, `container.publish`, `container.remotebuild` | `container.publish` sets `container.remotebuild` (bool) only; `container.credentials` and `container.remotebuild` set no attributes (span status carries success/failure and duration) | The hashed `pack.builder.image` / `pack.builder.tag` attributes are emitted on the separate `tools.pack.build` span, not the `container.*` spans | +| **Container lifecycle** | `package`, `deploy` (container service targets) | `container.credentials`, `container.publish`, `container.remotebuild` | `container.publish` sets a `container.remotebuild` property (bool) only; the `container.credentials` and `container.remotebuild` events set no attributes (span status carries success/failure and duration) | The hashed `pack.builder.image` / `pack.builder.tag` attributes are emitted on the separate `tools.pack.build` span, not the `container.*` spans | | **App Service deploy** | `deploy`, `publish` (App Service targets) | `deploy.appservice.zip` | `deploy.appservice.linux` (bool), `deploy.appservice.attempt` (retry attempt number) | Zip-deploy path only; outcome / duration are carried by the span status and span timing, not by dedicated attributes | -| **AKS service target** | `provision` (AKS preprovision/postprovision) | `aks.postprovision.skip` | Skip reason | Recorded when cluster is not yet available for context setup | +| **AKS service target** | `provision` (AKS preprovision/postprovision) | `aks.postprovision.skip` | `skip.reason` (bounded enum — `cluster_not_provisioned`) | Recorded when cluster is not yet available for context setup | | **Agent troubleshoot middleware** | Triggered on command failure when troubleshooting is engaged | `agent.troubleshoot` | Error chain attributes, hashed error fields | Emitted from `cmd/middleware/error.go` | | **Up-graph performance** | `up` (graph execution) | (none — enriches the `up` command span) | `perf.provision_duration_ms`, `perf.deploy_duration_ms`, `perf.total_duration_ms` | Emitted from `internal/cmd/up_graph.go` after the graph completes; provision/deploy durations set only when those phases run | | **VS RPC** | `vs-server` long-running session | `vsrpc.*` (event prefix) | Per-RPC attributes documented in `telemetry-schema.md` | Long-running RPC server for VS integration | diff --git a/docs/specs/metrics-audit/privacy-review-checklist.md b/docs/specs/metrics-audit/privacy-review-checklist.md index 8da9aecd8d7..c9da6730aae 100644 --- a/docs/specs/metrics-audit/privacy-review-checklist.md +++ b/docs/specs/metrics-audit/privacy-review-checklist.md @@ -171,8 +171,11 @@ A new field **must** be hashed if any of the following are true: A new field should **not** be hashed if: -- The value is from a fixed enum (e.g., `auth.method` = `"browser"`, or - `aspire.apphost.language` = `"typescript"` / `"python"` / `"go"` / `"java"` / `"rust"`). +- The value is from a fixed enum (e.g., `auth.method` = `"browser"`, + `aspire.apphost.language` = `"typescript"` / `"python"` / `"go"` / `"java"` / `"rust"`, + `auth.cache_clear_failed` = `"auth"` / `"subscriptions"`, or + `skip.reason` = `"cluster_not_provisioned"`), or a boolean + (e.g., `container.remotebuild`). - The value is a count or duration (measurements). - The value is system-generated metadata (e.g., OS type). - The value is a hardcoded literal in source code (e.g., `exegraph.step.tags`, which @@ -198,6 +201,7 @@ When adding a new telemetry field: - OTel key name - Classification - Purpose + - Endpoint (only when the value is a known endpoint identifier) - Whether it is hashed - Whether it is a measurement - Allowed values (if enum) @@ -222,6 +226,8 @@ Copy this checklist into your PR description when making telemetry changes. ### New Events - [ ] Event constant defined in `events/events.go` +- [ ] Event constant is an exported string `const` whose Go identifier contains `Event` (end it with + `Prefix` for a prefix-match group) so the GDPR classifier discovers it - [ ] Event documented in `docs/specs/metrics-audit/telemetry-schema.md` - [ ] Event follows naming convention (`prefix.noun.verb`) diff --git a/docs/specs/metrics-audit/telemetry-schema.md b/docs/specs/metrics-audit/telemetry-schema.md index f050477953c..8f63f01d3d6 100644 --- a/docs/specs/metrics-audit/telemetry-schema.md +++ b/docs/specs/metrics-audit/telemetry-schema.md @@ -190,6 +190,13 @@ not emitted by azd spans. | Field | OTel Key | Classification | Purpose | Notes | |-------|----------|----------------|---------|-------| | Remote build count | `container.remoteBuild.count` | SystemMetadata | FeatureInsight | **Measurement** | +| Publish remote build | `container.remotebuild` | SystemMetadata | FeatureInsight | Bool — whether the image was built remotely (ACR) rather than locally. Not hashed; not a measurement. | + +### AKS + +| Field | OTel Key | Classification | Purpose | Notes | +|-------|----------|----------------|---------|-------| +| Skip reason | `skip.reason` | SystemMetadata | FeatureInsight | Bounded enum (`cluster_not_provisioned`); never raw error text. Emitted on `aks.postprovision.skip`. Not hashed; not a measurement. | ### JSON-RPC @@ -317,6 +324,7 @@ The following fields are defined in `fields.go`. | Field | OTel Key | Classification | Purpose | Values | |-------|----------|----------------|---------|--------| | Auth method | `auth.method` | SystemMetadata | FeatureInsight | `browser`, `device-code`, `service-principal-secret`, `service-principal-certificate`, `federated-github`, `federated-azure-pipelines`, `federated-oidc`, `managed-identity`, `external`, `oneauth`, `check-status` | +| Auth cache-clear failed | `auth.cache_clear_failed` | SystemMetadata | PerformanceAndHealth | Fixed enum (`auth`, `subscriptions`) identifying which cache failed to clear during the pre-login cleanup. Emitted on the `auth login` usage event. Not hashed; not a measurement. | | Env count | `env.count` | SystemMetadata | FeatureInsight | **Measurement** — number of environments | | Hooks name | `hooks.name` | SystemMetadata | FeatureInsight | Built-in hook name (raw) or SHA-256 hash for extension/custom hooks. Known values: `prebuild`, `postbuild`, `predeploy`, `postdeploy`, `predown`, `postdown`, `prepackage`, `postpackage`, `preprovision`, `postprovision`, `prepublish`, `postpublish`, `prerestore`, `postrestore`, `preup`, `postup` | | Hooks type | `hooks.type` | SystemMetadata | FeatureInsight | `project`, `service`, `layer` | @@ -420,7 +428,7 @@ Telemetry for the `infra.layers[]` parallel provisioning feature, emitted from ` ## Data Classifications -Classifications are defined in `internal/telemetry/fields/fields.go` and control how data +Classifications are defined in `cli/azd/internal/tracing/fields/fields.go` and control how data is stored, retained, and who may access it. | Classification | Description | @@ -483,3 +491,44 @@ Fields that are hashed: 3. **Queue**: Envelopes are written to disk under `~/.azd/telemetry/`. 4. **Upload**: The `azd telemetry upload` command (run as a background process) reads the queue and sends data to Azure Monitor. 5. **Analysis**: Data flows into Kusto tables for dashboarding and analysis via LENS jobs and cooked tables. + +## GDPR Data-Catalog Classification + +Runtime emission (above) is separate from **classification**. The GDPR data catalog is kept in +sync by an external metadata tool that **statically scans the telemetry source** — it does not read this document or observe live telemetry, so it can only classify a property that is declared where the scan looks: an exported `fields.AttributeKey`. A raw `attribute.String("my.key", v)` at a call site is invisible to the scan, so its catalog row stays **Unclassified / `Complete=false`**. This is enforced in-repo by `TestNoRawTelemetryAttributes` (`cli/azd/cmd/telemetry_test.go`). + +### Field (attribute) discovery contract + +For a field to be discovered and classified it must be: + +- an **exported**, **package-level** `var` (not a `const`, not function-local, not unexported), and +- typed exactly **`AttributeKey`** (the struct declared in `fields.go`). + +The scanner reads these `AttributeKey` members: + +| `AttributeKey` member | What the classifier reads | +|-----------------------|---------------------------| +| `Key` | The dotted OTel key (a string literal, `attribute.Key("…")`, or a string const). | +| `Classification` | One of the six [Data Classifications](#data-classifications). | +| `Purpose` | One or more [Purposes](#purposes). | +| `Endpoint` | Optional identifier-type tag; set only when the value is a known endpoint identifier. | +| `IsMeasurement` | `true` for numeric values (routed to the Measurements column); `false` (default) for Properties. | + +`Classification` and `Purpose` must be written as **bare identifiers from the `fields` package** +(e.g. `Classification: SystemMetadata`) — the scanner reads the identifier name, so a qualified +`fields.SystemMetadata` reference from another package would not be recognized. Keep all +`AttributeKey` definitions inside the `fields` package. Fields are registered as **common +properties** that apply across events (the scan does not tie an attribute to a specific event). + +### Event discovery contract + +For an event to be discovered its constant must be: + +- an **exported** `const` with a **string** value in the `events` package, and +- named with a Go identifier that **contains the substring `Event`** (e.g. `PackBuildEvent`). A + constant whose identifier omits `Event` is silently skipped even if it is emitted at runtime. + +An identifier that **ends with `Prefix`** (e.g. `CommandEventPrefix`) registers a **prefix group**: +any emitted event name starting with that prefix is classified under it. Every other event constant +registers an exact event name. (Note: this `Event`/`Prefix` rule is about the Go **identifier**; the +string **value** still follows the `prefix.noun.verb` naming convention.)