Azure / Azure/azure-dev

[EPIC] Add chain, classifier, and origin context to azd error telemetry

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

Description

## Summary

When an azd command fails, telemetry today emits a single classification code as the AppInsights `RequestData.ResponseCode` — and not much else useful. The dominant catch-all bucket `internal.errors_errorString` (along with siblings like `internal.fmt_wrapError`) tells operators only that *something* failed, not what or where. There are no stacktraces, no chain visibility, no origin frame, and free-form error messages are deliberately not emitted (they may carry PII).

This epic covers the work to make every failed command in production telemetry diagnosable from Kusto with high probability.

## Goal

A triaging engineer who sees a Kusto bucket of azd errors should be able to answer *with high probability and without local repro*:

1. **Where in azd did this come from?** (package + function + line)
2. **What kind of error is it?** (named type chain, including any wrappers we control)
3. **Did one of our YAML suggestion rules already match it?** (if so, which)

Capturing the actual error *message text* (with redactions) is a nice-to-have (see #8016)

## Background — root cause of `internal.errors_errorString`

`MapError`'s fallback walks the chain and reports the *leaf* type (`internal/cmd/errors.go:428-458`). For `errors.New(...)` and `fmt.Errorf(...)` without `%w`, the leaf type is `*errors.errorString`, which sanitizes to the bucket name we see today. Named outer wrappers — which carry domain context — are silently discarded. Plus there are at least three duplicated classifier paths (`MapError`, `tracer.errorDescription`, `classifySuggestionType`) that drift apart over time.

## Phased plan

| Phase | Subissue | What |
|---|---|---|
| 1 | #8015 | Emit `error.chain.types` (string slice). Acts as the **measurement instrument** for everything that follows — gives Kusto data on which type chains dominate the catch-all today. |
| 2 | #8017 | Extract a shared `Classify(err) Classification` package. Unify the three classifier paths. Fix `errorType()` fallback to prefer **deepest-named-non-generic type**, with `internal.unclassified` only when the entire chain is generic stdlib types. |
| 3 | #8018 | Add stable `id` to each YAML rule in `error_suggestions.yaml`. Plumb through `ErrorWithSuggestion`. **Replace** `ResultCode = "error.suggestion"` with `suggest.`. Includes prerequisite middleware-ordering fix. |
| 4 | #8077 | Introduce `internal/azderrors` mini-package (~50–100 LOC) with `New/Wrap/Errorf` that capture a single origin frame via `runtime.Caller(1)`. Emit `error.origin.{func,file,line}`. No callsite migration in this phase. |
| 5 | #8079 | After Phase 1 ships and we have ~1–2 releases of Kusto data: hand-migrate top-N highest-volume bare-error sites to `azderrors.New(cls, ...)` with proper codes. Data-driven, not speculative. |
| 6 | #8016 | **Deferred**: capture redacted `error.message.redacted`/`error.message.hash`. Revisit only if Phases 1–5 leave a triage gap. |
| 7 | #8078 | **Deferred**: capture error return-path / stacktrace. Library options under evaluation. |

## Out of scope (with rationale)

- AppInsights `ExceptionData` / OTel `RecordError` migration — doubles row count, exporter rewrite needed; span properties on `RequestData` are sufficient for the planned signals.
- First-party extension instrumentation under `cli/azd/extensions/` — core-first; architecture stays extension-friendly.
- Adoption of `cockroachdb/errors` — module weight (~+1–3 MB binary, protobuf transitive deps) outweighs benefit when redactor is deferred.
- A static analyzer that flags non-`%w` `fmt.Errorf` — speculative without Kusto data; revisit if Phase 1 shows lossy-wrap dominates the catch-all.

## Cross-cutting cleanups (folded into the phases above)

- Existing `error.frame` field is misleading — it's an ARM nesting depth integer, not a stack frame (`internal/cmd/errors.go:460-488`). Rename to free the name for actual frame data later. (In Phase 1.)
- Existing `error.inner` field is defined in `internal/tracing/fields/fields.go:507-519` but never emitted. Remove or repurpose. (In Phase 1.)
- Fix middleware ordering: `TelemetryMiddleware` runs before `ErrorMiddleware`, so YAML rule IDs are not visible to `MapError` today. (In Phase 3.)

## References

- Prior PRs: #6827, #6835, #6865, #6901, #7051, #7235, #7797
- Related issues: #5493, #6861, #7143, #7233, #7884

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.