Azure / Azure/azure-dev

Use matched YAML rule IDs as the error ResultCode

Open
#8,018 0 comments 0 reactions 0 assignees View on GitHub
area/error-handling area/telemetry bug
Dominant language
Go
Stars
569
Forks
364
Avg merge
2d 19h
Merged PRs (30d)
136

Description

Phase 3 of #8011. Depends on #8017.

## Summary

The 48 string-pattern rules in `cli/azd/resources/error_suggestions.yaml` already encode the team's best classification of unstructured upstream errors. When a rule matches, the ResultCode collapses to `error.suggestion` — discarding the precise classification we just paid to compute.

Replace `ResultCode = "error.suggestion"` with `ResultCode = "suggest."` (e.g. `suggest.arm.soft_delete_conflict`). Per design discussion, the data churn from this replacement is acceptable.

## To deliver

### 1. YAML schema

- Add `id: ` (regex `^[a-z0-9_]+(\.[a-z0-9_]+)*$`) to each rule in `cli/azd/resources/error_suggestions.yaml`.
- Update `cli/azd/resources/error_suggestions.schema.json` to declare `id` as required for new rules.
- Add a startup test that asserts uniqueness of all rule IDs and the regex format.

### 2. Plumbing

- Add `ID string \`yaml:"id,omitempty"\`` to `ErrorSuggestionRule` (`pkg/errorhandler/types.go`).
- Add `RuleID string` to `ErrorWithSuggestion` (`pkg/errorhandler/errors.go`).
- Set `RuleID` in `ErrorHandlerPipeline` when wrapping a matched error.
- In the shared classifier from #8017, when the outer is `*ErrorWithSuggestion` with non-empty `RuleID`:
- Set `Classification.Code = "suggest." + RuleID`
- Recursively classify the inner; surface its `ChainTypes` and `Attrs`
- Add `fields.ErrSuggestionRuleID` (`error.suggestion.rule_id`) and emit it as an attribute alongside the new ResultCode.

### 3. Prerequisite: middleware ordering fix

`TelemetryMiddleware.Run` (`cmd/middleware/telemetry.go:117`) calls `cmd.MapError(err, span)` **before** `ErrorMiddleware.Run` (`cmd/middleware/error.go:192`) wraps the error in `ErrorWithSuggestion`. Today, even if a rule matches, the rule ID never reaches the telemetry span.

Resolve by either:

- Swapping the middleware order so `ErrorMiddleware` runs first and wraps before telemetry classifies, **or**
- Keeping the order but propagating `RuleID` via context so telemetry can read it after the action returns.

Either approach is acceptable; the first is more direct.

## Tests

- Pipeline unit test proving `RuleID` is set on `ErrorWithSuggestion` when a synthetic rule matches.
- End-to-end test for at least one important production rule (e.g. ARM soft-delete conflict). Per repo guidance, exercise actual YAML rules, not just framework mechanics.
- Uniqueness/format test for all rule IDs.
- AppInsights serialization test for the new `error.suggestion.rule_id` attribute.

## Out of scope (split out)

The original draft of this issue also proposed promoting bare `errors.New` / `fmt.Errorf` callsites to typed sentinels. That work is now split into a separate, **data-driven** issue (#8079), to be picked up after #8015 ships and we have Kusto data on the actual high-volume sites.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.