Azure / Azure/azure-dev

Add functional tests that validate error telemetry (ResultCode, error.type, error attributes)

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

Description

## Problem

We currently have no functional tests that assert on the telemetry payload produced by failed commands. The existing `Test_CLI_Telemetry_*` tests validate trace IDs, attribute propagation, and nested span structure, but they never check `Status.Code`, `Status.Description` (which becomes `ResultCode` in Kusto), or `error.*` attributes. The test `Span` struct does not even have a `Status` field — it silently discards the status data already present in the trace JSON.

This means:
- **New error classifications** (sentinels, YAML rules, `classifySuggestionType`) are only validated by unit tests that construct synthetic errors — never by running actual azd commands
- **Regressions in the error → telemetry pipeline** (middleware, MapError, trace export) are only caught after release by checking Kusto
- **Extension error transport** (`LocalError`/`ServiceError` → gRPC → host → `MapError`) is never validated end-to-end

## Background

We have been steadily improving error telemetry:
- #6827 — YAML-driven error handling pipeline
- #7051 — Typed sentinel errors + AST enforcement
- #6901 — Structured error handling for extensions
- #6835 — `azdext.Run` lifecycle and structured extension error transport
- #7140 — `classifySuggestionType()` for wrapped error classification

We want to "shift-left" and catch telemetry quality issues **in CI** rather than through the current reactive loop of: Kusto → fix → release → wait for adoption → repeat.

## Proposal

### Phase 1: Test infrastructure

1. **Add `Status` field to the test `Span` struct** in `telemetry_test.go`:
```go
type SpanStatus struct {
Code int `json:"Code"`
Description string `json:"Description"`
}
```
The trace NDJSON (via `stdouttrace` exporter) already includes this data — it is just not parsed.

2. **Extract shared trace assertion helpers** (reusable across test files):
```go
func parseTraceFile(t *testing.T, path string) []Span
func findCmdSpan(spans []Span, cmdName string) *Span
func assertResultCode(t *testing.T, span *Span, expected string)
func assertErrorType(t *testing.T, span *Span, expected string)
func assertAttribute(t *testing.T, span *Span, key attribute.Key, expected any)
```

### Phase 2: Local/fast test cases

| Test | Command | Expected ResultCode | Expected `error.type` |
|------|---------|--------------------|-----------------------|
| NoProject | `azd provision` in empty dir | `internal.no_project` | — |
| InvalidArgs | `azd env set` (no args) | `error.suggestion` | `internal.invalid_args` |
| EnvNotFound | `azd env select nonexistent` | `error.suggestion` | `internal.env_not_found` |
| KeyNotFound | `azd env get-value missing` (after `env new`) | `error.suggestion` | `internal.key_not_found` |
| InvalidFlagCombo | `azd deploy --all --from-package x` | `error.suggestion` | `internal.invalid_flag_combination` |
| SuccessPath | `azd env new` + `azd env list` | OK status, no `error.*` attrs | — |
| CatchAllCanary | Trigger unclassified error | Starts with `internal.` | present |

All tests follow the pattern:
1. Set up temp dir (optionally with `copySample()`)
2. Run `azd --trace-log-file ` via `azdcli.NewCLI`
3. Parse NDJSON trace file
4. Find command span by name
5. Assert `Status.Code`, `Status.Description`, and specific attributes

### Phase 3: Extension error tests

| Test | Scenario | Expected ResultCode | Key attributes |
|------|----------|--------------------|-|
| ExtLocalError | Extension returns `LocalError{Category: validation, Code: invalid_config}` | `ext.validation.invalid_config` | `error.category=validation`, `error.code=invalid_config` |
| ExtServiceError | Extension returns `ServiceError{StatusCode: 429, ErrorCode: RateLimitExceeded}` | `ext.service.ratelimitexceeded` | `error.service.statusCode=429` |
| ExtRunFailed | Extension process crashes | `ext.run.failed` | — |

These may require a test extension binary or fixture.

### Phase 4: Recorded Azure failure fixtures (nightly CI)

| Test | Scenario | Expected |
|------|----------|----------|
| SuggestionWithARM | ARM deployment error wrapped in suggestion | `error.suggestion` + structured `error.type` (not `*azapi.DeploymentErrorLine`) |
| NestedErrorPropagation | `azd up` where provision fails | Error status propagates to parent span |

## Existing infrastructure to reuse

| Component | Status |
|-----------|--------|
| `azdcli.CLI` subprocess runner | ✅ Reuse |
| `copySample()` / testdata | ✅ Reuse |
| `tempDirWithDiagnostics()` | ✅ Reuse |
| `--trace-log-file` | ✅ Reuse |
| NDJSON parsing in `telemetry_test.go` | ⚠️ Extract to shared helper |
| `Span` struct | ⚠️ Add `Status` field |

## Success criteria

- Phase 1+2 merged: Every sentinel-classified error that can be triggered without Azure credentials has a functional test asserting its exact `ResultCode` and `error.type`
- CI prevents regressions in error telemetry classification for these scenarios
- Future PRs that modify `MapError`, `classifySentinel`, `classifySuggestionType`, or `error_suggestions.yaml` get automatic validation

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.