diff --git a/.github/skills/azd-preflight/SKILL.md b/.github/skills/azd-preflight/SKILL.md index d5a5a1e21ed..f351a3bab93 100644 --- a/.github/skills/azd-preflight/SKILL.md +++ b/.github/skills/azd-preflight/SKILL.md @@ -24,7 +24,7 @@ Runs the full azd preflight suite and auto-fixes failures. ## Overview -The azd preflight suite (`mage preflight`) validates code quality across 9 checks before +The azd preflight suite (`mage preflight`) validates code quality across 10 checks before changes are submitted. This skill runs the suite, parses failures, applies automated fixes, and re-runs until all checks pass — or escalates to the user when a fix requires human judgment. @@ -36,7 +36,7 @@ and re-runs until all checks pass — or escalates to the user when a fix requir ## Exit Criteria -- All 9 preflight checks pass (or user explicitly chose to skip specific checks) +- All 10 preflight checks pass (or user explicitly chose to skip specific checks) - Every changed `CHANGELOG.md` passes its targeted spell check - All auto-applied fixes are saved to disk (not staged or committed — the user decides when to commit) - A clear summary of what passed, what was fixed, and what was skipped is displayed diff --git a/.github/skills/azd-preflight/references/fix-strategies.md b/.github/skills/azd-preflight/references/fix-strategies.md index 7f751f61927..f80aba0413a 100644 --- a/.github/skills/azd-preflight/references/fix-strategies.md +++ b/.github/skills/azd-preflight/references/fix-strategies.md @@ -1,7 +1,7 @@ # Fix Strategies Detailed fix procedures for each preflight check failure. Process checks in -their original order (1-9) because earlier fixes can resolve later failures. +their original order (1-10) because earlier fixes can resolve later failures. ## Formatting (`gofmt`) — Auto-fix @@ -45,6 +45,18 @@ For each finding: If a lint finding is ambiguous or the fix would change behavior, ask the user via `ask_user`. +## Telemetry Documentation (`telemetrylint`) — Analyze and Fix + +Run the checker to get the source location and missing document: + +```bash +cd cli/azd && go run ./tools/telemetrylint 2>&1 +``` + +Add each missing core event or field to both telemetry reference documents. +Add each missing extension event or field to Markdown in that extension's +directory. Re-run the checker after updating the documentation. + ## Go Spell Check (`cspell`) — Analyze and Fix Re-run the Go source spell check to get the specific unknown words: diff --git a/.github/skills/azd-preflight/references/output-and-errors.md b/.github/skills/azd-preflight/references/output-and-errors.md index 42a6094d0c2..5b42a533e0b 100644 --- a/.github/skills/azd-preflight/references/output-and-errors.md +++ b/.github/skills/azd-preflight/references/output-and-errors.md @@ -5,12 +5,13 @@ ### Success ``` -Preflight passed — all 9 checks clean. +Preflight passed — all 10 checks clean. ✓ gofmt ✓ go fix ✓ copyright ✓ lint + ✓ telemetry docs ✓ cspell ✓ cspell-misc ✓ build @@ -29,6 +30,7 @@ Preflight passed after fixes. ✓ go fix (fixed: 2 modernizations applied) ✓ copyright (no issues) ✓ lint (fixed: 5 findings resolved) + ✓ telemetry docs (fixed: missing entries added) ✓ cspell (fixed: 1 word added to dictionary) ✓ cspell-misc (no issues) ✓ build (no issues) @@ -42,12 +44,13 @@ Files modified: {list of changed files} ### Partial Success ``` -Preflight partially passed — {N} of 9 checks clean, {M} skipped. +Preflight partially passed — {N} of 10 checks clean, {M} skipped. ✓ gofmt ✓ go fix ✓ copyright ✓ lint + ✓ telemetry docs ✓ cspell ✓ cspell-misc ✓ build diff --git a/.github/skills/azd-preflight/references/preflight-checks.md b/.github/skills/azd-preflight/references/preflight-checks.md index 63407a7d064..46d0efaf891 100644 --- a/.github/skills/azd-preflight/references/preflight-checks.md +++ b/.github/skills/azd-preflight/references/preflight-checks.md @@ -1,6 +1,6 @@ # Preflight Checks Reference -The `mage preflight` command runs these 9 checks in order. Each check, its +The `mage preflight` command runs these 10 checks in order. Each check, its purpose, and the automated fix strategy are listed below. ## 1. Formatting (`gofmt`) @@ -33,7 +33,17 @@ issues include: - `unused` — remove dead code - `staticcheck` — fix static analysis warnings -## 5. Go Spell Check (`cspell`) +## 5. Telemetry Documentation (`telemetrylint`) + +**Command**: `go run ./tools/telemetrylint` (from `cli/azd/`) + +**Passes when**: Every core event and field appears in both telemetry +reference documents, and static extension usage items appear in extension +Markdown documentation. + +**Auto-fix**: Add the missing event or field to the relevant documentation. + +## 6. Go Spell Check (`cspell`) **Command**: `cspell lint "**/*.go" --relative --config ./.vscode/cspell.yaml --no-progress` (from `cli/azd/`) **Passes when**: No unknown words found. @@ -41,7 +51,7 @@ issues include: using file-scoped `overrides` entries (not the global `words` list). For actual typos, fix the spelling in source code. -## 6. Misc/Docs Spell Check (`cspell-misc`) +## 7. Misc/Docs Spell Check (`cspell-misc`) **Command**: `cspell lint "**/*" --relative --config ./.vscode/cspell.misc.yaml --no-progress` (from the repository root) @@ -49,7 +59,7 @@ typos, fix the spelling in source code. **Auto-fix**: Fix typos. Add legitimate terms to file-scoped `overrides` entries in `.vscode/cspell.misc.yaml`. -## 7. Build (`go build`) +## 8. Build (`go build`) **Command**: `go build ./...` (from `cli/azd/`) **Passes when**: Compilation succeeds with zero errors. @@ -59,14 +69,14 @@ typos, fix the spelling in source code. - Undefined symbols - Syntax errors -## 8. Unit Tests (`go test -short`) +## 9. Unit Tests (`go test -short`) **Command**: `go test ./... -short -cover -count=1` (from `cli/azd/`) **Passes when**: All tests pass. **Auto-fix**: Analyze test failures and fix the root cause in source code or tests. Do NOT skip or delete failing tests — fix them. -## 9. Playback Tests (Functional) +## 10. Playback Tests (Functional) **Command**: Discovers test recordings in `test/functional/testdata/recordings/` and runs matching functional tests with `AZURE_RECORD_MODE=playback`. diff --git a/.github/skills/azd-preflight/references/workflow.md b/.github/skills/azd-preflight/references/workflow.md index d5c0f499235..1cad4c9794b 100644 --- a/.github/skills/azd-preflight/references/workflow.md +++ b/.github/skills/azd-preflight/references/workflow.md @@ -42,7 +42,7 @@ Run this targeted check from the repository root: cspell lint "" --relative --config "" --no-progress ``` -Capture these results separately from the nine `mage preflight` checks. This conditional check is +Capture these results separately from the ten `mage preflight` checks. This conditional check is required because the repository-wide misc config excludes directories that have their own cspell configuration, including `cli/`. @@ -70,10 +70,10 @@ Also parse the targeted changelog spell-check results when changed changelogs we ### Step 4 — Fix Failures (Iterative) For each failing check, apply the fix strategy from the references. Process checks in -their original order (1-9) because earlier fixes can resolve later failures (e.g., `gofmt` +their original order (1-10) because earlier fixes can resolve later failures (e.g., `gofmt` fixes may resolve `lint` issues, `build` fixes resolve `test` failures). -After the nine standard checks, fix any targeted changelog spelling failures. +After the ten standard checks, fix any targeted changelog spelling failures. {{ references/fix-strategies.md }} @@ -91,7 +91,7 @@ Re-run the targeted spell check for every changed changelog. **If failures remain**: Return to Step 4 for the remaining failures. This is an iterative loop — continue until either: -- All 9 checks and every changed changelog pass, OR +- All 10 checks and every changed changelog pass, OR - 3 full cycles have been attempted without progress on a specific check ### Step 6 — Escalate if Stuck diff --git a/.github/workflows/telemetry-docs.yml b/.github/workflows/telemetry-docs.yml new file mode 100644 index 00000000000..76765ba7fc6 --- /dev/null +++ b/.github/workflows/telemetry-docs.yml @@ -0,0 +1,25 @@ +name: telemetry-docs + +on: + pull_request: + paths: + - "cli/azd/**" + - "docs/reference/telemetry-data.md" + - "docs/specs/metrics-audit/telemetry-schema.md" + - ".github/workflows/telemetry-docs.yml" + branches: [main] + +permissions: + contents: read + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version-file: cli/azd/go.mod + - name: Check telemetry documentation + working-directory: cli/azd + run: go run ./tools/telemetrylint diff --git a/.vscode/cspell.misc.yaml b/.vscode/cspell.misc.yaml index 56bc83aa518..215940c9488 100644 --- a/.vscode/cspell.misc.yaml +++ b/.vscode/cspell.misc.yaml @@ -47,6 +47,7 @@ overrides: - exegraph - pseudonymizing - pulumi + - telemetrylint - filename: ./README.md words: - VSIX diff --git a/cli/azd/.vscode/cspell.yaml b/cli/azd/.vscode/cspell.yaml index 1abae989043..14ad1e8f5fe 100644 --- a/cli/azd/.vscode/cspell.yaml +++ b/cli/azd/.vscode/cspell.yaml @@ -376,6 +376,13 @@ overrides: - Logf - Getenv - httptest + - filename: magefile.go + words: + - telemetrylint + - filename: tools/telemetrylint/** + words: + - Elts + - telemetrylint - filename: docs/extensions/extension-framework.md words: - invopop diff --git a/cli/azd/docs/tracing-in-azd.md b/cli/azd/docs/tracing-in-azd.md index 78b28edfccb..04ca67acfbf 100644 --- a/cli/azd/docs/tracing-in-azd.md +++ b/cli/azd/docs/tracing-in-azd.md @@ -71,6 +71,26 @@ tracing.SetUsageAttributes(fields.EnvName.StringHashed(envName)) This example sets a usage attribute to be included in the root command event. +### 4. Update the telemetry documentation + +Add every new event and attribute key to both the +[public telemetry reference](../../../docs/reference/telemetry-data.md) and +the [metrics audit schema](../../../docs/specs/metrics-audit/telemetry-schema.md). +For extension usage events, document the event and each fixed attribute in +the extension's README or another Markdown file in the extension directory. + +Run the completeness checker from `cli/azd`: + +```bash +go run ./tools/telemetrylint +``` + +The checker reads the event and field declarations, scans static +`tracing.Start` and `attribute.*` literals, and checks static extension +`ReportUsageRequest` values. It understands the documented `cmd.`, `mcp.`, +and `vsrpc.` prefixes, but dynamic extension keys cannot be enumerated and +must be documented by the extension author. + --- ## Existing Event Taxonomy diff --git a/cli/azd/extensions/microsoft.azd.demo/README.md b/cli/azd/extensions/microsoft.azd.demo/README.md index a0594c272d0..c7bc1e15cd5 100644 --- a/cli/azd/extensions/microsoft.azd.demo/README.md +++ b/cli/azd/extensions/microsoft.azd.demo/README.md @@ -138,6 +138,11 @@ The `telemetry` command demonstrates the telemetry service, which lets an extens The command reports a single `demo.telemetry.reported` event with two fixed, low-cardinality attributes and prints whether the host recorded it. +The attributes are: + +- `demo.mode`: the fixed demo mode, currently `safe` +- `demo.outcome`: whether the demo telemetry call was `accepted` or `dropped` + This example shows two things: - **Every value is a fixed enum.** Resource names, paths, prompts, and anything a user typed are customer content and must never be sent. diff --git a/cli/azd/magefile.go b/cli/azd/magefile.go index cb27189ad31..95082355058 100644 --- a/cli/azd/magefile.go +++ b/cli/azd/magefile.go @@ -159,6 +159,7 @@ func Preflight() error { checkGoFix checkCopyright checkLint + checkTelemetryDocs checkCspell checkCspellMisc checkBuild @@ -167,7 +168,7 @@ func Preflight() error { numChecks ) checkNames := [numChecks]string{ - "gofmt", "go fix", "copyright", "lint", + "gofmt", "go fix", "copyright", "lint", "telemetry docs", "cspell", "cspell-misc", "build", "test", "playback tests", } @@ -269,7 +270,22 @@ func Preflight() error { printResult(checkLint) }) - // 5a. cspell (Go source) + // 5. Telemetry documentation + wg.Go(func() { + out, err := runCaptureAll( + azdDir, nil, "go", "run", "./tools/telemetrylint", + ) + if err != nil { + results[checkTelemetryDocs] = checkResult{ + "fail", err.Error(), out, + } + } else { + results[checkTelemetryDocs] = checkResult{"pass", "", out} + } + printResult(checkTelemetryDocs) + }) + + // 6a. cspell (Go source) wg.Go(func() { out, err := runCaptureAll(azdDir, nil, "cspell", "lint", "**/*.go", @@ -282,7 +298,7 @@ func Preflight() error { printResult(checkCspell) }) - // 5b. cspell (misc/docs) + // 6b. cspell (misc/docs) wg.Go(func() { out, err := runCaptureAll(repoRoot, nil, "cspell", "lint", "**/*", @@ -295,7 +311,7 @@ func Preflight() error { printResult(checkCspellMisc) }) - // 6. go build — compile all packages AND pre-build the azd + azd-record + // 7. go build — compile all packages AND pre-build the azd + azd-record // binaries so that Wave 2 tests can skip auto-building. This lets unit // tests and playback tests run in parallel safely. wg.Go(func() { @@ -351,7 +367,7 @@ func Preflight() error { skipBuildEnv := []string{"CLI_TEST_SKIP_BUILD=true"} var wg2 sync.WaitGroup - // 7. Unit tests + // 8. Unit tests wg2.Go(func() { if err := runStreamingWithEnv( azdDir, skipBuildEnv, @@ -364,7 +380,7 @@ func Preflight() error { printResult(checkTest) }) - // 8. Playback tests + // 9. Playback tests wg2.Go(func() { if err := runFunctionalTests(azdDir, testRunOpts{ mode: "playback", diff --git a/cli/azd/tools/telemetrylint/main.go b/cli/azd/tools/telemetrylint/main.go new file mode 100644 index 00000000000..1214752b370 --- /dev/null +++ b/cli/azd/tools/telemetrylint/main.go @@ -0,0 +1,841 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package main + +import ( + "flag" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "os" + "path/filepath" + "runtime" + "sort" + "strconv" + "strings" +) + +const ( + eventsSource = "cli/azd/internal/tracing/events/events.go" + fieldsSource = "cli/azd/internal/tracing/fields/fields.go" + schemaDoc = "docs/specs/metrics-audit/telemetry-schema.md" + referenceDoc = "docs/reference/telemetry-data.md" +) + +type definition struct { + kind string + value string + source string + line int +} + +type document struct { + path string + content string +} + +type issue struct { + def definition + doc string +} + +type extensionUsage struct { + root string + definitions []definition +} + +func main() { + repoRootFlag := flag.String( + "repo-root", + "", + "repository root (defaults to the current repository)", + ) + flag.Parse() + + repoRoot := *repoRootFlag + var err error + if repoRoot == "" { + repoRoot, err = findRepoRoot(".") + } else { + repoRoot, err = filepath.Abs(repoRoot) + } + if err != nil { + fmt.Fprintf(os.Stderr, "telemetry lint: %v\n", err) + os.Exit(1) + } + + issues, err := lintRepository(repoRoot) + if err != nil { + fmt.Fprintf(os.Stderr, "telemetry lint: %v\n", err) + os.Exit(1) + } + + for _, current := range issues { + fmt.Fprintf( + os.Stderr, + "%s:%d: %s %q is not documented in %s\n", + relativePath(repoRoot, current.def.source), + current.def.line, + current.def.kind, + current.def.value, + relativePath(repoRoot, current.doc), + ) + } + + if len(issues) > 0 { + fmt.Fprintf( + os.Stderr, + "telemetry lint: %d undocumented telemetry item(s)\n", + len(issues), + ) + os.Exit(1) + } + + fmt.Println("Telemetry documentation is up to date.") +} + +func findRepoRoot(start string) (string, error) { + current, err := filepath.Abs(start) + if err != nil { + return "", err + } + + for { + if fileExists(filepath.Join(current, filepath.FromSlash(eventsSource))) && + fileExists(filepath.Join(current, filepath.FromSlash(schemaDoc))) { + return current, nil + } + + parent := filepath.Dir(current) + if parent == current { + break + } + current = parent + } + + return "", fmt.Errorf("repository root not found from %q", start) +} + +func lintRepository(repoRoot string) ([]issue, error) { + documents, err := loadDocuments( + filepath.Join(repoRoot, filepath.FromSlash(referenceDoc)), + filepath.Join(repoRoot, filepath.FromSlash(schemaDoc)), + ) + if err != nil { + return nil, err + } + + eventDefinitions, err := parseEvents( + filepath.Join(repoRoot, filepath.FromSlash(eventsSource)), + ) + if err != nil { + return nil, err + } + + fieldDefinitions, err := parseFields( + filepath.Join(repoRoot, filepath.FromSlash(fieldsSource)), + ) + if err != nil { + return nil, err + } + + rawFieldDefinitions, err := parseRawAttributes( + filepath.Join(repoRoot, "cli", "azd"), + filepath.Join(repoRoot, filepath.FromSlash(fieldsSource)), + ) + if err != nil { + return nil, err + } + + literalEventDefinitions, err := parseLiteralEvents( + filepath.Join(repoRoot, "cli", "azd"), + ) + if err != nil { + return nil, err + } + + coreDefinitions := uniqueDefinitions(append( + append(eventDefinitions, fieldDefinitions...), + append(rawFieldDefinitions, literalEventDefinitions...)..., + )) + issues := checkDefinitions(coreDefinitions, documents) + + extensionUsages, err := parseExtensionUsages( + filepath.Join(repoRoot, "cli", "azd", "extensions"), + ) + if err != nil { + return nil, err + } + + for _, usage := range extensionUsages { + extensionDocuments, err := loadMarkdownDocuments(usage.root) + if err != nil { + return nil, err + } + issues = append( + issues, + checkDefinitions(usage.definitions, extensionDocuments)..., + ) + } + + sortIssues(issues, repoRoot) + return issues, nil +} + +func loadDocuments(paths ...string) ([]document, error) { + documents := make([]document, 0, len(paths)) + for _, path := range paths { + content, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read %s: %w", path, err) + } + documents = append(documents, document{ + path: path, + content: string(content), + }) + } + return documents, nil +} + +func loadMarkdownDocuments(root string) ([]document, error) { + var documents []document + err := filepath.WalkDir(root, func( + path string, + entry fs.DirEntry, + err error, + ) error { + if err != nil { + return err + } + if entry.IsDir() { + if entry.Name() == ".git" || entry.Name() == "node_modules" { + return fs.SkipDir + } + return nil + } + if !strings.HasSuffix(strings.ToLower(path), ".md") { + return nil + } + if strings.EqualFold(filepath.Base(path), "changelog.md") { + return nil + } + + content, err := os.ReadFile(path) + if err != nil { + return err + } + documents = append(documents, document{ + path: path, + content: string(content), + }) + return nil + }) + if err != nil { + return nil, fmt.Errorf("read Markdown under %s: %w", root, err) + } + + sort.Slice(documents, func(i, j int) bool { + return documents[i].path < documents[j].path + }) + return documents, nil +} + +func parseEvents(path string) ([]definition, error) { + file, fileSet, err := parseGoFile(path) + if err != nil { + return nil, err + } + + var definitions []definition + for _, declaration := range file.Decls { + general, ok := declaration.(*ast.GenDecl) + if !ok || general.Tok != token.CONST { + continue + } + for _, specification := range general.Specs { + values, ok := specification.(*ast.ValueSpec) + if !ok { + continue + } + for index, name := range values.Names { + if index >= len(values.Values) { + continue + } + value, ok := stringLiteral(values.Values[index]) + if !ok { + continue + } + definitions = append(definitions, definition{ + kind: "event", + value: value, + source: path, + line: fileSet.Position(name.Pos()).Line, + }) + } + } + } + return uniqueDefinitions(definitions), nil +} + +func parseFields(path string) ([]definition, error) { + file, fileSet, err := parseGoFile(path) + if err != nil { + return nil, err + } + + var definitions []definition + for _, declaration := range file.Decls { + general, ok := declaration.(*ast.GenDecl) + if !ok || general.Tok != token.VAR { + continue + } + for _, specification := range general.Specs { + values, ok := specification.(*ast.ValueSpec) + if !ok { + continue + } + for index, name := range values.Names { + if index >= len(values.Values) { + continue + } + keys, err := fieldKeys( + values.Values[index], + name.Name, + file, + fileSet, + ) + if err != nil { + return nil, fmt.Errorf("%s:%d: %w", + path, fileSet.Position(name.Pos()).Line, err) + } + for _, key := range keys { + definitions = append(definitions, definition{ + kind: "field", + value: key, + source: path, + line: fileSet.Position(name.Pos()).Line, + }) + } + } + } + } + return uniqueDefinitions(definitions), nil +} + +func fieldKeys( + expression ast.Expr, + name string, + file *ast.File, + fileSet *token.FileSet, +) ([]string, error) { + switch current := expression.(type) { + case *ast.CompositeLit: + var keys []string + for _, element := range current.Elts { + keyValue, ok := element.(*ast.KeyValueExpr) + if !ok { + continue + } + keyName, ok := keyValue.Key.(*ast.Ident) + if !ok || keyName.Name != "Key" { + continue + } + key, err := resolveKeyExpression( + keyValue.Value, + file, + fileSet, + ) + if err != nil { + return nil, err + } + if key != "" { + keys = append(keys, key) + } + } + return keys, nil + case *ast.CallExpr: + if !isAttributeKeyCall(current) || len(current.Args) == 0 { + return nil, nil + } + key, err := resolveKeyExpression(current.Args[0], file, fileSet) + if err != nil { + return nil, err + } + if key == "" && name == "ObjectIdKey" { + return []string{"user_AuthenticatedId"}, nil + } + if key == "" { + return nil, fmt.Errorf( + "could not resolve telemetry field key for %s", name) + } + return []string{key}, nil + default: + return nil, nil + } +} + +func resolveKeyExpression( + expression ast.Expr, + file *ast.File, + fileSet *token.FileSet, +) (string, error) { + if value, ok := stringLiteral(expression); ok { + return value, nil + } + + if call, ok := expression.(*ast.CallExpr); ok { + if !isAttributeKeyCall(call) || len(call.Args) == 0 { + return "", nil + } + return resolveKeyExpression(call.Args[0], file, fileSet) + } + + selector, ok := expression.(*ast.SelectorExpr) + if !ok { + return "", nil + } + + packageName, ok := selector.X.(*ast.Ident) + if !ok { + return "", nil + } + if packageName.Name == "contracts" && + selector.Sel.Name == "UserAuthUserId" { + return "user_AuthenticatedId", nil + } + if packageName.Name != "semconv" { + return "", nil + } + + if key, ok := semanticConventionKeys[selector.Sel.Name]; ok { + return key, nil + } + + key := inlineCommentKey(file, fileSet, expression.Pos()) + if key == "" { + return "", fmt.Errorf( + "could not resolve semantic convention key %s", + selector.Sel.Name, + ) + } + return key, nil +} + +var semanticConventionKeys = map[string]string{ + "JSONRPCRequestIDKey": "rpc.jsonrpc.request_id", + "RPCMethodKey": "rpc.method", +} + +func inlineCommentKey( + file *ast.File, + fileSet *token.FileSet, + position token.Pos, +) string { + line := fileSet.Position(position).Line + for _, group := range file.Comments { + if fileSet.Position(group.Pos()).Line != line { + continue + } + for word := range strings.FieldsSeq(group.Text()) { + word = strings.Trim(word, "`\"'.,;:()[]{}") + if strings.Contains(word, ".") { + return word + } + } + } + return "" +} + +func parseRawAttributes(root, fieldsPath string) ([]definition, error) { + var definitions []definition + err := walkGoFiles(root, func(path string) error { + if samePath(path, fieldsPath) { + return nil + } + + file, fileSet, err := parseGoFile(path) + if err != nil { + return err + } + ast.Inspect(file, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok || !isLiteralAttributeCall(call) { + return true + } + value, ok := stringLiteral(call.Args[0]) + if !ok { + return true + } + definitions = append(definitions, definition{ + kind: "field", + value: value, + source: path, + line: fileSet.Position(call.Pos()).Line, + }) + return true + }) + return nil + }) + if err != nil { + return nil, err + } + return uniqueDefinitions(definitions), nil +} + +func parseLiteralEvents(root string) ([]definition, error) { + var definitions []definition + err := walkGoFiles(root, func(path string) error { + file, fileSet, err := parseGoFile(path) + if err != nil { + return err + } + ast.Inspect(file, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok || len(call.Args) < 2 { + return true + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "Start" { + return true + } + packageName, ok := selector.X.(*ast.Ident) + if !ok || packageName.Name != "tracing" { + return true + } + value, ok := stringLiteral(call.Args[1]) + if !ok { + return true + } + definitions = append(definitions, definition{ + kind: "event", + value: value, + source: path, + line: fileSet.Position(call.Pos()).Line, + }) + return true + }) + return nil + }) + if err != nil { + return nil, err + } + return uniqueDefinitions(definitions), nil +} + +func parseExtensionUsages(root string) ([]extensionUsage, error) { + byRoot := map[string][]definition{} + err := walkGoFiles(root, func(path string) error { + file, fileSet, err := parseGoFile(path) + if err != nil { + return err + } + + extensionRoot := firstPathSegment(root, path) + ast.Inspect(file, func(node ast.Node) bool { + literal, ok := node.(*ast.CompositeLit) + if !ok || !isReportUsageRequest(literal.Type) { + return true + } + definitions := reportUsageDefinitions(literal, path, fileSet) + byRoot[extensionRoot] = append( + byRoot[extensionRoot], + definitions..., + ) + return true + }) + return nil + }) + if err != nil { + return nil, err + } + + roots := make([]string, 0, len(byRoot)) + for root := range byRoot { + roots = append(roots, root) + } + sort.Strings(roots) + + usages := make([]extensionUsage, 0, len(roots)) + for _, extensionRoot := range roots { + usages = append(usages, extensionUsage{ + root: extensionRoot, + definitions: uniqueDefinitions(byRoot[extensionRoot]), + }) + } + return usages, nil +} + +func reportUsageDefinitions( + literal *ast.CompositeLit, + path string, + fileSet *token.FileSet, +) []definition { + var definitions []definition + for _, element := range literal.Elts { + keyValue, ok := element.(*ast.KeyValueExpr) + if !ok { + continue + } + fieldName, ok := keyValue.Key.(*ast.Ident) + if !ok { + continue + } + + switch fieldName.Name { + case "EventName": + if value, ok := stringLiteral(keyValue.Value); ok { + definitions = append(definitions, definition{ + kind: "extension event", + value: value, + source: path, + line: fileSet.Position(keyValue.Pos()).Line, + }) + } + case "Attributes": + attributes, ok := keyValue.Value.(*ast.CompositeLit) + if !ok { + continue + } + for _, attributeElement := range attributes.Elts { + attribute, ok := attributeElement.(*ast.KeyValueExpr) + if !ok { + continue + } + value, ok := stringLiteral(attribute.Key) + if !ok { + continue + } + definitions = append(definitions, definition{ + kind: "extension field", + value: value, + source: path, + line: fileSet.Position(attribute.Pos()).Line, + }) + } + } + } + return definitions +} + +func checkDefinitions( + definitions []definition, + documents []document, +) []issue { + if len(documents) == 0 { + var issues []issue + for _, current := range definitions { + issues = append(issues, issue{ + def: current, + doc: filepath.Join(filepath.Dir(current.source), "README.md"), + }) + } + return issues + } + + var issues []issue + for _, current := range definitions { + for _, doc := range documents { + if !isDocumented(doc.content, current.value) { + issues = append(issues, issue{ + def: current, + doc: doc.path, + }) + } + } + } + return issues +} + +func isDocumented(content, value string) bool { + if strings.Contains(content, value) { + return true + } + + for _, prefix := range []string{"cmd.", "mcp.", "vsrpc."} { + if strings.HasPrefix(value, prefix) && + strings.Contains(content, "`"+prefix) { + return true + } + } + return false +} + +func uniqueDefinitions(definitions []definition) []definition { + byKey := map[string]definition{} + for _, current := range definitions { + key := current.kind + "\x00" + current.value + existing, ok := byKey[key] + if !ok || definitionLess(current, existing) { + byKey[key] = current + } + } + + result := make([]definition, 0, len(byKey)) + for _, current := range byKey { + result = append(result, current) + } + sort.Slice(result, func(i, j int) bool { + return definitionLess(result[i], result[j]) + }) + return result +} + +func sortIssues(issues []issue, repoRoot string) { + sort.Slice(issues, func(i, j int) bool { + leftSource := relativePath(repoRoot, issues[i].def.source) + rightSource := relativePath(repoRoot, issues[j].def.source) + if leftSource != rightSource { + return leftSource < rightSource + } + if issues[i].def.line != issues[j].def.line { + return issues[i].def.line < issues[j].def.line + } + if issues[i].def.value != issues[j].def.value { + return issues[i].def.value < issues[j].def.value + } + return relativePath(repoRoot, issues[i].doc) < + relativePath(repoRoot, issues[j].doc) + }) +} + +func definitionLess(left, right definition) bool { + if left.source != right.source { + return left.source < right.source + } + if left.line != right.line { + return left.line < right.line + } + return left.value < right.value +} + +func walkGoFiles(root string, callback func(string) error) error { + return filepath.WalkDir(root, func( + path string, + entry fs.DirEntry, + err error, + ) error { + if err != nil { + return err + } + if entry.IsDir() { + switch entry.Name() { + case ".git", "node_modules", "vendor": + return fs.SkipDir + } + return nil + } + if strings.HasSuffix(path, ".go") && + !strings.HasSuffix(path, "_test.go") { + return callback(path) + } + return nil + }) +} + +func parseGoFile(path string) (*ast.File, *token.FileSet, error) { + fileSet := token.NewFileSet() + file, err := parser.ParseFile( + fileSet, + path, + nil, + parser.ParseComments, + ) + if err != nil { + return nil, nil, fmt.Errorf("parse %s: %w", path, err) + } + return file, fileSet, nil +} + +func stringLiteral(expression ast.Expr) (string, bool) { + literal, ok := expression.(*ast.BasicLit) + if !ok || literal.Kind != token.STRING { + return "", false + } + value, err := strconv.Unquote(literal.Value) + return value, err == nil +} + +func isAttributeKeyCall(call *ast.CallExpr) bool { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "Key" { + return false + } + packageName, ok := selector.X.(*ast.Ident) + return ok && packageName.Name == "attribute" +} + +func isLiteralAttributeCall(call *ast.CallExpr) bool { + if len(call.Args) == 0 { + return false + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + packageName, ok := selector.X.(*ast.Ident) + if !ok || packageName.Name != "attribute" { + return false + } + + switch selector.Sel.Name { + case "Bool", "BoolSlice", "Float64", "Float64Slice", + "Int", "Int64", "Int64Slice", "IntSlice", + "Key", "String", "StringSlice": + return true + default: + return false + } +} + +func isReportUsageRequest(expression ast.Expr) bool { + switch current := expression.(type) { + case *ast.StarExpr: + return isReportUsageRequest(current.X) + case *ast.SelectorExpr: + return current.Sel.Name == "ReportUsageRequest" + case *ast.Ident: + return current.Name == "ReportUsageRequest" + default: + return false + } +} + +func firstPathSegment(root, path string) string { + relative, err := filepath.Rel(root, path) + if err != nil { + return root + } + segment := relative + if index := strings.IndexRune(relative, os.PathSeparator); index >= 0 { + segment = relative[:index] + } + return filepath.Join(root, segment) +} + +func relativePath(root, path string) string { + relative, err := filepath.Rel(root, path) + if err != nil { + return path + } + return filepath.ToSlash(relative) +} + +func samePath(left, right string) bool { + leftAbs, leftErr := filepath.Abs(left) + rightAbs, rightErr := filepath.Abs(right) + if leftErr != nil || rightErr != nil { + return false + } + if runtime.GOOS == "windows" { + return strings.EqualFold(leftAbs, rightAbs) + } + return leftAbs == rightAbs +} + +func fileExists(path string) bool { + info, err := os.Stat(path) + return err == nil && !info.IsDir() +} diff --git a/cli/azd/tools/telemetrylint/main_test.go b/cli/azd/tools/telemetrylint/main_test.go new file mode 100644 index 00000000000..033ac3a8b56 --- /dev/null +++ b/cli/azd/tools/telemetrylint/main_test.go @@ -0,0 +1,183 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestIsDocumented(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + value string + want bool + }{ + { + name: "exact value", + content: "| `demo.event` |", + value: "demo.event", + want: true, + }, + { + name: "dynamic command event", + content: "| `cmd.` |", + value: "cmd.provision", + want: true, + }, + { + name: "unrelated prefix", + content: "| `cmd.` |", + value: "other.event", + want: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + if got := isDocumented(test.content, test.value); got != test.want { + t.Fatalf("isDocumented() = %v, want %v", got, test.want) + } + }) + } +} + +func TestCheckDefinitions(t *testing.T) { + t.Parallel() + + definitions := []definition{ + {kind: "event", value: "demo.event", source: "events.go", line: 10}, + {kind: "field", value: "demo.field", source: "fields.go", line: 20}, + } + documents := []document{ + {path: "reference.md", content: "`demo.event`"}, + {path: "schema.md", content: "`demo.field`"}, + } + + issues := checkDefinitions(definitions, documents) + if len(issues) != 2 { + t.Fatalf("checkDefinitions() returned %d issues, want 2", len(issues)) + } + if issues[0].doc != "schema.md" || issues[1].doc != "reference.md" { + t.Fatalf("unexpected issues: %#v", issues) + } +} + +func TestParseEvents(t *testing.T) { + t.Parallel() + + path := writeTestFile(t, "events.go", `package events + +const ( + CommandEventPrefix = "cmd." + ExampleEvent = "demo.event" +) +`) + + definitions, err := parseEvents(path) + if err != nil { + t.Fatal(err) + } + if len(definitions) != 2 { + t.Fatalf("parseEvents() returned %d definitions, want 2", len(definitions)) + } + if definitions[0].value != "cmd." || + definitions[1].value != "demo.event" { + t.Fatalf("unexpected definitions: %#v", definitions) + } +} + +func TestParseFields(t *testing.T) { + t.Parallel() + + path := writeTestFile(t, "fields.go", `package fields + +var ( + ServiceNameKey = AttributeKey{ + Key: semconv.ServiceNameKey, // service.name + } + MachineIDKey = AttributeKey{ + Key: attribute.Key("machine.id"), + } + ObjectIdKey = attribute.Key(contracts.UserAuthUserId) +) +`) + + definitions, err := parseFields(path) + if err != nil { + t.Fatal(err) + } + values := make([]string, 0, len(definitions)) + for _, definition := range definitions { + values = append(values, definition.value) + } + want := []string{"service.name", "machine.id", "user_AuthenticatedId"} + if strings.Join(values, ",") != strings.Join(want, ",") { + t.Fatalf("parseFields() = %v, want %v", values, want) + } +} + +func TestParseExtensionUsages(t *testing.T) { + t.Parallel() + + root := t.TempDir() + extension := filepath.Join(root, "demo") + source := filepath.Join(extension, "telemetry.go") + if err := os.MkdirAll(extension, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(source, []byte(`package demo + +var request = &azdext.ReportUsageRequest{ + EventName: "demo.event", + Attributes: map[string]string{ + "demo.mode": "safe", + }, +} +`), 0o600); err != nil { + t.Fatal(err) + } + + usages, err := parseExtensionUsages(root) + if err != nil { + t.Fatal(err) + } + if len(usages) != 1 || len(usages[0].definitions) != 2 { + t.Fatalf("unexpected usages: %#v", usages) + } + if usages[0].definitions[0].value != "demo.event" || + usages[0].definitions[1].value != "demo.mode" { + t.Fatalf("unexpected definitions: %#v", usages[0].definitions) + } +} + +func TestLintRepository(t *testing.T) { + repoRoot, err := findRepoRoot(".") + if err != nil { + t.Fatal(err) + } + + issues, err := lintRepository(repoRoot) + if err != nil { + t.Fatal(err) + } + if len(issues) != 0 { + t.Fatalf("lintRepository() found issues: %#v", issues) + } +} + +func writeTestFile(t *testing.T, name, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + return path +} diff --git a/docs/reference/telemetry-data.md b/docs/reference/telemetry-data.md index 5ff3e850e83..05c8fd72344 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 | Cache cleanup that failed: `auth` or `subscriptions` |
@@ -389,6 +390,14 @@ Set **only when an external command-line tool invocation fails**, during error c | `provision.cancellation` | string | `none`, `leave_running`, `canceled`, `cancel_timed_out`, `cancel_timed_out_nested`, `cancel_raced_succeeded`, `cancel_raced_failed`, `cancel_raced_deleted`, `cancel_too_late`, `cancel_failed` |
+
+AKS Postprovision + +| Field Key | Type | Description | +|-----------|------|-------------| +| `skip.reason` | string | Reason AKS postprovision skipped Kubernetes context setup | +
+
Multi-Layer Provision diff --git a/docs/specs/metrics-audit/telemetry-schema.md b/docs/specs/metrics-audit/telemetry-schema.md index f050477953c..c6238b29c41 100644 --- a/docs/specs/metrics-audit/telemetry-schema.md +++ b/docs/specs/metrics-audit/telemetry-schema.md @@ -3,6 +3,23 @@ This document is the authoritative reference for all telemetry events, fields, classifications, and data pipeline details in the Azure Developer CLI (`azd`). +## Documentation completeness check + +Run the repository-local checker from `cli/azd` before submitting telemetry changes: + +```bash +go run ./tools/telemetrylint +``` + +The checker parses event constants in `internal/tracing/events/events.go`, field +keys in `internal/tracing/fields/fields.go`, and static telemetry literals in +production Go code. It also checks statically declared extension +`ReportUsageRequest` events and attributes against Markdown files in the +extension directory. Every core item must appear in both this schema and the +[public telemetry reference](../../reference/telemetry-data.md). Dynamic +`cmd.`, `mcp.`, and `vsrpc.` names are covered by their documented prefix; +dynamic extension keys remain the responsibility of the extension author. + ## Events Events are defined in `cli/azd/internal/tracing/events/events.go`. Each event is emitted as an @@ -312,11 +329,13 @@ set on its own. ## Command-Specific Fields -The following fields are defined in `fields.go`. +The following fields are defined in `fields.go` or emitted as +operation-specific attributes at their instrumentation site. | 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 cleanup failure | `auth.cache_clear_failed` | SystemMetadata | PerformanceAndHealth | Cache cleanup that failed: `auth` or `subscriptions` | | 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` | @@ -325,6 +344,13 @@ The following fields are defined in `fields.go`. | Pipeline auth | `pipeline.auth` | SystemMetadata | FeatureInsight | Emitted only when `--auth-type` is set on `pipeline config`: `federated`, `client-credentials` | | Infra provider | `infra.provider` | SystemMetadata | FeatureInsight | provision/up/down: sorted, de-duplicated string slice of resolved providers — `bicep`/`terraform`/`arm`/`pulumi` verbatim, `custom` for any other (extension) provider (raw name not emitted); multi-layer projects that combine providers record each distinct value (e.g. `["bicep","terraform"]`). `infra generate`/`synth`: the value read from azure.yaml's `infra.provider` directly as a single string (`bicep`/`terraform`/`arm`/`pulumi`, `auto` when unset, or `custom` for any other (extension) provider — raw name not emitted) | +### Provider-specific fields + +| Field | OTel Key | Classification | Purpose | Values | +|-------|----------|----------------|---------|--------| +| Foundry network mode | `provision.network_mode` | SystemMetadata | FeatureInsight | `none`, `byo`, or `managed` | +| AKS skip reason | `skip.reason` | SystemMetadata | FeatureInsight | Reason Kubernetes context setup was skipped | + ### App Service Deploy | Field | OTel Key | Classification | Purpose | Notes |