Conversation
…validation (#7114) Add telemetry to the local preflight validation system to track validation outcomes, which checks are failing, and user actions after warnings/errors. Changes: - Add DiagnosticID field to PreflightCheckResult for per-finding telemetry - Add PreflightCheck struct with RuleID for per-rule crash tracking - Add validation.preflight span with outcome, diagnostics, and severity counts - Set usage-level attributes for deployment correlation (false positive and true positive tracking when users accept warnings) - Add telemetry field definitions and event constant - Update checkRoleAssignmentPermissions with diagnostic IDs: role_assignment_missing and role_assignment_conditional - Add DiagnosticID to PreflightReportItem for UX layer propagation - Add tests for telemetry emission and ID propagation Telemetry outcomes: - passed: no issues found - warnings_accepted: user chose to continue despite warnings - aborted_by_errors: blocking errors detected - aborted_by_user: user declined to continue after warnings Fixes #7114 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vhvb1989
requested review from
JeffreyCA,
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds telemetry instrumentation to the Bicep local preflight validation pipeline by introducing stable identifiers for rules/diagnostics and emitting a new validation.preflight span with outcome + metrics, enabling correlation with later deployment outcomes.
Changes:
- Introduces stable
RuleID(per check) andDiagnosticID(per finding) and propagates them through preflight results and UX reporting. - Adds a
validation.preflighttracing span invalidatePreflight()and sets span + usage-level attributes (outcome, diagnostics, counts). - Extends tests to cover outcome-setting behavior and ID propagation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/output/ux/preflight_report.go | Adds DiagnosticID to report items for downstream telemetry correlation. |
| cli/azd/pkg/infra/provisioning/bicep/local_preflight.go | Adds DiagnosticID to results and wraps checks in a PreflightCheck{RuleID, Fn} for stable rule identification. |
| cli/azd/pkg/infra/provisioning/bicep/local_preflight_test.go | Updates tests for new types and adds coverage around DiagnosticID propagation. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go | Starts validation.preflight span, records rule/diagnostic IDs, counts, and sets preflight outcome + usage attributes. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go | Adds tests for setPreflightOutcome behavior and outcome constants. |
| cli/azd/internal/tracing/fields/fields.go | Defines new tracing attribute keys for preflight telemetry. |
| cli/azd/internal/tracing/events/events.go | Adds validation.preflight event name constant. |
jongio
requested changes
Mar 30, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7396
Add telemetry tracking with unique rule/diagnostic IDs for preflight validation by @vhvb1989
Summary
What: Instruments local preflight validation with structured telemetry - stable RuleIDs per check function, DiagnosticIDs per finding type, a validation.preflight span with outcome/counts, and usage-level attributes for deployment correlation.
Why: Enables crash attribution (RuleID), warning/deployment correlation for false/true positive tracking (DiagnosticID + outcome), and abort rate measurement. Fixes #7114.
Assessment: Clean design with good RuleID/DiagnosticID separation. Usage-level attributes for deployment correlation are well-motivated. One gap: error paths don't record an outcome, creating a telemetry blind spot for internal failures.
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic | 0 | 0 | 1 | 0 |
| Total | 0 | 0 | 1 | 0 |
Test Coverage Estimate
- Well covered:
setPreflightOutcomeall 4 outcome values, DiagnosticID propagation, check ordering - Gaps: Full
validatePreflightintegration test with telemetry, error path telemetry, usage attributes
What's Done Well
- Two-level ID design (RuleID for crash tracking, DiagnosticID for finding correlation) is solid
- Usage-level attributes for false/true positive tracking is a strong telemetry pattern
- Error messages now include RuleID for debugging (
preflight check %q failed)
1 inline comment below.
- Fix import grouping: merge internal/tracing imports with azure-dev group - Use named returns + deferred span.EndWithStatus(err) for proper span status - Move rules attribute after validate() to avoid reporting skipped checks as executed - Add 'skipped' outcome when bicep snapshot is unavailable (nil results, nil err) - Add 'error' outcome on all error early-return paths (validate failure, prompt error) - Rename TestPreflightCheck_ErrorIncludesRuleID to _AddCheckStoresRuleID - Add usage attribute verification in TestSetPreflightOutcome test - Add 'skipped' and 'error' outcome test cases - Add 'actioned' to cspell overrides for local_preflight.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vhvb1989
commented
Mar 31, 2026
Member
Author
vhvb1989
left a comment
vhvb1989
left a comment
There was a problem hiding this comment.
Addressed all 6 review comments in commit 8e111a8:
-
Import grouping — Merged
internal/tracingimports into theazure-devgroup (no separate block after envsubst). -
span.EndWithStatus — Switched to named returns +
defer func() { span.EndWithStatus(err) }()so the span correctly reflects error status. -
Rules attribute after validate — Moved
validation.preflight.rulesto aftervalidate()runs. Added a"skipped"outcome whenvalidate()returnsnil, nil(snapshot unavailable), distinguishing it from"passed"(checks ran, no findings). -
Test name mismatch — Renamed
TestPreflightCheck_ErrorIncludesRuleID→TestPreflightCheck_AddCheckStoresRuleIDto match what it actually asserts. -
Usage attributes test — Now verifies
tracing.GetUsageAttributes()contains bothvalidation.preflight.outcomeandvalidation.preflight.diagnostics. -
Missing outcome on error paths (@jongio) — Added
preflightOutcomeError = "error"constant. Bothvalidate()error return andconsole.Confirm()error return now callsetPreflightOutcome(span, preflightOutcomeError, ...)before returning.
Also fixed the cspell failure by adding actioned to the file-scoped overrides for local_preflight.go.
jongio
approved these changes
Mar 31, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Reviewed the incremental changes in 8e111a87 addressing prior review feedback. All 6 comments addressed correctly:
span.EndWithStatus(err)via named returns + defer - idiomatic and correct (no shadowing sinceresults, err :=reuses the named return)- Error paths now record
""error""outcome before returning ""skipped""vs""passed""distinction viaresults == nilguard is clean- Rules attribute moved after
validate()with proper skipped-path handling - Usage attributes now verified in tests
- Test name accurately reflects what's asserted
Remaining items are the doc consistency nits the bot flagged (outcome examples in fields.go/events.go comments). Straightforward to fix but non-blocking.
- Fix import grouping: move envsubst to external group, azd internal imports in separate group (stdlib -> external -> azd internal) - Only set PreflightRulesKey when checks actually ran; use empty slice for skipped outcome - Fix grammar: 'A nil results' -> 'A nil result' - Update PreflightOutcomeKey doc to list all 6 outcomes - Remove duplicate envsubst import Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
jongio
approved these changes
Mar 31, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Reviewed commit 84d4911 (changes since 8e111a8).
All five second-round feedback items are properly addressed:
- Import grouping: envsubst moved to external group, duplicate removed
- Grammar fix:
A nil results->A nil result - Rules attribute: empty slice for skipped, actual IDs when checks ran
- PreflightOutcomeKey doc: now lists all 6 outcomes
- No new issues introduced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds telemetry to the local preflight validation system so we can track validation outcomes, which checks are failing, and user actions after warnings/errors.
Fixes #7114 — follow-up from PR #7053 review feedback.
What changed
Unique IDs for rules and diagnostics
PreflightCheck.RuleID): Stable identifier per check function (e.g."role_assignment_permissions"). Used for crash tracking — if a rule errors, telemetry shows which one.PreflightCheckResult.DiagnosticID): Stable identifier per finding type (e.g."role_assignment_missing","role_assignment_conditional"). Used for error correlation — correlating actioned warnings with deployment outcomes.Telemetry span:
validation.preflightA new span is created in
validatePreflight()with these attributes:validation.preflight.rulesvalidation.preflight.diagnosticsvalidation.preflight.warning.countvalidation.preflight.error.countvalidation.preflight.outcomeOutcome values
passedwarnings_acceptedaborted_by_errorsaborted_by_userskippederrorDeployment correlation (usage attributes)
validation.preflight.outcomeandvalidation.preflight.diagnosticsare also set as usage-level attributes on the parent command span (cmd.provision/cmd.up). This enables:warnings_accepted+ deployment succeedswarnings_accepted+ deployment failsaborted_by_errorsoraborted_by_user(deployment never ran)Files changed
local_preflight.goDiagnosticIDtoPreflightCheckResult, newPreflightCheckstruct withRuleID, updatedAddCheck/validatebicep_provider.govalidatePreflight()fields/fields.goAttributeKeydefinitions for preflight telemetryevents/events.goPreflightValidationEventconstantpreflight_report.goDiagnosticIDtoPreflightReportItemcspell.yamlactionedto file-scoped overrideslocal_preflight_test.gobicep_provider_test.gosetPreflightOutcomeand all outcome valuesTesting
go test ./pkg/infra/provisioning/bicep/...)golangci-lint,gofmt,cspell, copyright check)