fix(azd): preserve system error attribution - #9810
Merged
Merged
Conversation
|
Azure Pipelines: Successfully started running 4 pipeline(s). 23 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Add direct extension invocation metadata, structured error transport, typed tool failures, and stable core classifications so hosted-agent deployment failures can be attributed without inferring ownership from installed-extension inventory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hui Miao (huimiu)
force-pushed
the
hui/system-error-attribution
branch
from
September 1, 2026 09:45
29ce68f to
f5d44a8
Compare
github-actions
Bot
added
area/extensions
Hui Miao (huimiu)
marked this pull request as ready for review
September 2, 2026 09:31
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Relayed gRPC metadata is lost on reserialization, and telemetry bounds and validation are incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Preserves extension failure attribution and structured error metadata across azd’s gRPC and telemetry boundaries.
Changes:
- Adds extension invocation, error-chain, mapper, gRPC, and tool-failure attribution.
- Extends the protobuf error contract with cause and tool details.
- Updates tests, telemetry documentation, and privacy records.
File summaries
| File | Description |
|---|---|
docs/specs/metrics-audit/telemetry-schema.md |
Documents new telemetry contracts. |
docs/specs/metrics-audit/privacy-review-checklist.md |
Records privacy decisions. |
docs/specs/metrics-audit/feature-telemetry-matrix.md |
Maps extension failure telemetry. |
docs/reference/telemetry-data.md |
Updates public telemetry reference. |
cli/azd/pkg/extensions/runner.go |
Adds extension version attribution. |
cli/azd/pkg/extensions/runner_test.go |
Tests runner attribution. |
cli/azd/pkg/extensions/invocation_error.go |
Introduces invocation metadata wrappers. |
cli/azd/pkg/extensions/invocation_error_test.go |
Tests wrapper behavior. |
cli/azd/pkg/extensions/extension.go |
Attributes reported extension errors. |
cli/azd/pkg/extensions/extension_test.go |
Tests reported-error metadata. |
cli/azd/pkg/exec/command_name.go |
Extracts normalized tool names. |
cli/azd/pkg/errorchain/errorchain.go |
Adds public error-chain utilities. |
cli/azd/pkg/errorchain/errorchain_test.go |
Tests error-chain normalization. |
cli/azd/pkg/azdext/run.go |
Supports structured tool errors. |
cli/azd/pkg/azdext/run_test.go |
Tests tool error helpers. |
cli/azd/pkg/azdext/extension_error.go |
Extends structured error transport. |
cli/azd/pkg/azdext/extension_error_test.go |
Tests transport round trips. |
cli/azd/pkg/azdext/errors.pb.go |
Regenerates protobuf bindings. |
cli/azd/internal/tracing/fields/fields.go |
Defines mapper telemetry fields. |
cli/azd/internal/tracing/errchain/errchain.go |
Delegates to public utilities. |
cli/azd/internal/tracing/errchain/errchain_test.go |
Updates cycle-safety tests. |
cli/azd/internal/mapper/errors.go |
Exposes mapper type diagnostics. |
cli/azd/internal/mapper/errors_test.go |
Tests mapper diagnostics. |
cli/azd/internal/grpcserver/server_test.go |
Tests tool-error relay. |
cli/azd/internal/grpcserver/event_service.go |
Adds lifecycle invocation attribution. |
cli/azd/internal/grpcserver/event_service_test.go |
Tests lifecycle metadata. |
cli/azd/internal/grpcserver/errors.go |
Relays structured tool details. |
cli/azd/internal/grpcserver/container_service.go |
Maps container tool failures. |
cli/azd/internal/grpcserver/container_service_test.go |
Tests container error mapping. |
cli/azd/internal/cmd/errors.go |
Expands telemetry classification. |
cli/azd/internal/cmd/errors_test.go |
Tests new classifications. |
cli/azd/grpc/proto/errors.proto |
Adds cause and tool error messages. |
cli/azd/docs/extensions/extensions-style-guide.md |
Documents structured tool errors. |
cli/azd/docs/extensions/extension-sdk-reference.md |
Updates SDK error reference. |
cli/azd/.vscode/cspell.yaml |
Adds errorchain spelling. |
Review details
Files not reviewed (1)
- cli/azd/pkg/azdext/errors.pb.go: Generated file
- Files reviewed: 34/35 changed files
- Comments generated: 7
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Relayed cause metadata can be lost, process telemetry remains insufficiently bounded, and the protobuf contract has compatibility and scaffold gaps.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- cli/azd/pkg/azdext/errors.pb.go: Generated file
Suppressed comments (1)
cli/azd/grpc/proto/errors.proto:39
int32cannot represent every Windows process exit status: Windows exposes a 32-bit unsigned code, and common values such as0xC0000135exceedmath.MaxInt32. The conversion inWrapErrortherefore changes these codes into negative values during transport. Use anint64wire field and regenerate/update the Go mapping so the SDK'sintvalue round-trips losslessly on supported 64-bit platforms.
optional int32 exit_code = 3; // Process exit code when the tool ran and failed
- Files reviewed: 36/37 changed files
- Comments generated: 6
- Review effort level: Balanced
github-actions
Bot
added
the
ext-x
JeffreyCA
reviewed
Sep 2, 2026
JeffreyCA
left a comment
JeffreyCA
left a comment
Contributor
There was a problem hiding this comment.
The overall direction makes sense.
I left a few comments on tool-name normalization, safe error-chain traversal, and preserving tool attribution during publish.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Some known tools and multi-handler or plain-gRPC failure paths still lose the attribution this change intends to preserve.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- cli/azd/pkg/azdext/errors.pb.go: Generated file
Suppressed comments (1)
cli/azd/pkg/azdext/extension_error.go:252
- Plain gRPC statuses still lose their status code on an extension round trip. This branch leaves only an unspecified
ExtensionError;UnwrapErrorthen converts it toLocalError, so a hostcodes.Unavailablereturned by an extension is classified asext.local.failedrather than the documentedinternal.grpc.unavailable. Preserve the bounded gRPC code in the transport and reconstruct/classify it on return, with a plain-status round-trip test.
if actionable == nil && !isAuth {
// Plain gRPC error with no host metadata; leave extErr as-is so the caller
// surfaces the original message via the unspecified origin path.
return
- Files reviewed: 41/42 changed files
- Comments generated: 2
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Untrusted cause types can enter raw telemetry, mapper attributes remain unbounded, and protocol direction documentation is inconsistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- cli/azd/pkg/azdext/errors.pb.go: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
cli/azd/internal/cmd/errors.go:265
- The PR states mapper names are validated and bounded before telemetry emission, but both values here come directly from
cleanTypeName, which has no length or character bound. Registered types can therefore produce arbitrarily longerror.mapper.*.typeattributes. Apply an explicit bounded normalization (with a stable fallback for oversized/unsupported names) before setting these attributes.
cli/azd/pkg/azdext/extension_error.go:213 - This adds a host-to-extension relay for
ExtensionError, but the source protocol still documents that message as “extension -> host only” ingrpc/proto/errors.proto:65-66. The generated file also mentionsToolErrorin anActionableErrorDetailcomment that the source proto does not, so regeneration will overwrite that edit. Update the source and scaffold protocol comments to describe the bidirectional relay, then regenerate the Go output.
- Files reviewed: 43/44 changed files
- Comments generated: 1
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The hashed-field inventories are incomplete, and the protobuf direction comment contradicts the implemented relay behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- cli/azd/pkg/azdext/errors.pb.go: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
cli/azd/grpc/proto/errors.proto:68
ExtensionErroris no longer extension-to-host only:mapHostErrorattaches it to statuses sent back to extensions, and the new round-trip tests rely on that relay. Update the direction comment and regenerateerrors.pb.goto describe this bidirectional/multi-hop use; otherwise protocol consumers are told not to expect a supported wire path.
docs/specs/metrics-audit/privacy-review-checklist.md:14- This review record documents hashing, but
error.extension.cause_typesis absent from the mandatory “Fields That Must Be Hashed” table later in this file.cli/azd/AGENTS.md:308-321requires that table to be updated for every newly hashed field; add this key withStringSliceHashedso future raw-emission reviews cover it.
- Files reviewed: 43/44 changed files
- Comments generated: 1
- Review effort level: Balanced
Contributor
microsoft.azd.extensions PR buildNote This is an unsigned development build. Install it only if you trust this PR. Install the extension: azd ext install "https://azuresdkartifacts.z5.web.core.windows.net/azd/extensions/pr/9810/microsoft-azd-extensions.zip"
|
Contributor
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
JeffreyCA
approved these changes
Sep 3, 2026
JeffreyCA
left a comment
JeffreyCA
left a comment
Contributor
There was a problem hiding this comment.
The follow-up changes address my earlier comments. The remaining attribution ambiguity is limited to multi-handler failures and does not block approval.
Member
Author
|
/check-enforcer override |
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
extension.id,extension.version, and lifecycle event.Local,Service, andToolerror metadata now crosses the extension gRPC boundary. This includes bounded cause types and tool failure details.extension.installedremains an inventory and cohort signal, not proof of runtime ownership.Why this change
Hosted-agent System Errors do not include enough producer or root-cause information to tell whether a failure came from azd, an extension, a service, or a local tool. This PR adds bounded diagnostic metadata without customer content and leaves user-facing error messages unchanged.
The existing classification ladder remains. The new metadata is added at the transport and span boundaries. The protobuf changes are additive and wire-compatible. Typed errors keep their structured details across gRPC, while invocation metadata travels in a wrapper that does not change classification or UX. Cause types and mapper names are validated and bounded before telemetry emission.
The Agents extension currently depends on released azd
v1.32.0, which does not include these SDK APIs. This PR does not make the extension consume them. After the core change is released, a follow-up can update the dependency and add Agents-specific attribution without a temporaryreplacedirective or breaking extension builds.Closes: #9845