docs: Document extension authoring models — SDK-Managed vs Self-Managed#7406
Open
docs: Document extension authoring models — SDK-Managed vs Self-Managed#7406
Conversation
vhvb1989
requested review from
JeffreyCA,
jongio,
rajeshkamal5050 and
tg-msft
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR primarily adds guidance for azd extension authors on choosing between SDK-Managed vs Self-Managed authoring models, and also introduces new preflight-validation telemetry plumbing (diagnostic IDs, rule IDs, tracing span + attributes) to correlate preflight outcomes with deployments.
Changes:
- Add new documentation: “Extension Authoring Models: SDK-Managed vs Self-Managed”, and link it from the extensions style guide.
- Add
DiagnosticID/RuleIDconcepts to local preflight checks and wire these into tracing/telemetry attributes. - Extend cspell overrides for new terminology used in preflight code comments.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/output/ux/preflight_report.go | Adds DiagnosticID to UX report items for correlation. |
| cli/azd/pkg/infra/provisioning/bicep/local_preflight.go | Adds DiagnosticID to results and introduces PreflightCheck{RuleID, Fn}. |
| cli/azd/pkg/infra/provisioning/bicep/local_preflight_test.go | Updates tests for new preflight types and adds propagation tests. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go | Adds a preflight tracing span and records outcome/rules/diagnostics attributes. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go | Adds tests for setPreflightOutcome span + usage attributes. |
| cli/azd/internal/tracing/fields/fields.go | Adds new preflight-related attribute keys. |
| cli/azd/internal/tracing/events/events.go | Adds a new validation.preflight tracing event name. |
| cli/azd/docs/extensions/extensions-style-guide.md | Links to the new authoring models document. |
| cli/azd/docs/extensions/extension-authoring-models.md | New doc explaining the two extension authoring models and flag/env-var behavior. |
| cli/azd/.vscode/cspell.yaml | Adds a file-scoped dictionary override for “actioned”. |
Comment on lines
+2192
to
+2213
| } | ||
| ruleIDs := make([]string, len(localPreflight.checks)) | ||
| for i, check := range localPreflight.checks { | ||
| ruleIDs[i] = check.RuleID | ||
| } | ||
| span.SetAttributes(fields.PreflightRulesKey.StringSlice(ruleIDs)) | ||
|
|
||
| // Compute telemetry metrics from the results. | ||
| var diagnosticIDs []string | ||
| var warningCount, errorCount int | ||
| for _, result := range results { | ||
| if result.DiagnosticID != "" { | ||
| diagnosticIDs = append(diagnosticIDs, result.DiagnosticID) | ||
| } | ||
| if result.Severity == PreflightCheckError { | ||
| errorCount++ | ||
| } else { | ||
| warningCount++ | ||
| } | ||
| } | ||
| span.SetAttributes(fields.PreflightDiagnosticsKey.StringSlice(diagnosticIDs)) | ||
| span.SetAttributes(fields.PreflightWarningCountKey.Int(warningCount)) |
There was a problem hiding this comment.
When local preflight is skipped (results == nil), this still records validation.preflight.rules using all registered checks. That contradicts the comment about recording rule IDs that actually executed, and will make telemetry look like rules ran even when snapshot was unavailable. Consider only setting PreflightRulesKey when results != nil (or set it to an empty slice when skipped).
Suggested change
| } | |
| ruleIDs := make([]string, len(localPreflight.checks)) | |
| for i, check := range localPreflight.checks { | |
| ruleIDs[i] = check.RuleID | |
| } | |
| span.SetAttributes(fields.PreflightRulesKey.StringSlice(ruleIDs)) | |
| // Compute telemetry metrics from the results. | |
| var diagnosticIDs []string | |
| var warningCount, errorCount int | |
| for _, result := range results { | |
| if result.DiagnosticID != "" { | |
| diagnosticIDs = append(diagnosticIDs, result.DiagnosticID) | |
| } | |
| if result.Severity == PreflightCheckError { | |
| errorCount++ | |
| } else { | |
| warningCount++ | |
| } | |
| } | |
| span.SetAttributes(fields.PreflightDiagnosticsKey.StringSlice(diagnosticIDs)) | |
| span.SetAttributes(fields.PreflightWarningCountKey.Int(warningCount)) | |
| } else { | |
| ruleIDs := make([]string, len(localPreflight.checks)) | |
| for i, check := range localPreflight.checks { | |
| ruleIDs[i] = check.RuleID | |
| } | |
| span.SetAttributes(fields.PreflightRulesKey.StringSlice(ruleIDs)) | |
| // Compute telemetry metrics from the results. | |
| var diagnosticIDs []string | |
| var warningCount, errorCount int | |
| for _, result := range results { | |
| if result.DiagnosticID != "" { | |
| diagnosticIDs = append(diagnosticIDs, result.DiagnosticID) | |
| } | |
| if result.Severity == PreflightCheckError { | |
| errorCount++ | |
| } else { | |
| warningCount++ | |
| } | |
| } | |
| span.SetAttributes(fields.PreflightDiagnosticsKey.StringSlice(diagnosticIDs)) | |
| span.SetAttributes(fields.PreflightWarningCountKey.Int(warningCount)) | |
| span.SetAttributes(fields.PreflightErrorCountKey.Int(errorCount)) | |
| } |
Comment on lines
+18
to
+21
| 1. **Pre-parses global flags** — `ParseGlobalFlags()` extracts `--debug`, | ||
| `--cwd`, `--no-prompt`, and trace flags from the full argument list. | ||
| Note: `-e/--environment` is **not** pre-parsed — it is a per-command flag. | ||
|
|
There was a problem hiding this comment.
This description says ParseGlobalFlags() extracts trace flags, but in the current codebase trace flags are parsed separately by the telemetry system (see internal/telemetry/telemetry.go:getTraceFlags) and ParseGlobalFlags only binds cwd/debug/no-prompt into GlobalCommandOptions. Updating the wording here would avoid misleading extension authors about where trace flags are handled.
Suggested change
| 1. **Pre-parses global flags** — `ParseGlobalFlags()` extracts `--debug`, | |
| `--cwd`, `--no-prompt`, and trace flags from the full argument list. | |
| Note: `-e/--environment` is **not** pre-parsed — it is a per-command flag. | |
| 1. **Pre-parses selected global flags** — `ParseGlobalFlags()` extracts `--debug`, | |
| `--cwd`, and `--no-prompt` from the full argument list and binds them into | |
| `GlobalCommandOptions`. Trace-related flags are parsed separately by the telemetry | |
| system. Note: `-e/--environment` is **not** pre-parsed — it is a per-command flag. |
Comment on lines
+61
to
+63
| - Reserved flag conflict validation | ||
| (`ValidateNoReservedFlagConflicts` — [PR #7312](https://github.com/Azure/azure-dev/pull/7312)) | ||
|
|
There was a problem hiding this comment.
This section claims azdext.Run() adds reserved-flag conflict validation via ValidateNoReservedFlagConflicts, but neither the function nor that behavior exists in pkg/azdext in this repo version (Run currently handles FORCE_COLOR, ExecuteContext, and ReportError). Please update this doc to match the shipped behavior (or clearly label it as future/planned behavior and link to where/when it will land).
Suggested change
| - Reserved flag conflict validation | |
| (`ValidateNoReservedFlagConflicts` — [PR #7312](https://github.com/Azure/azure-dev/pull/7312)) |
Comment on lines
2162
to
2170
| ) (abort bool, err error) { | ||
| ctx, span := tracing.Start(ctx, events.PreflightValidationEvent) | ||
| defer func() { | ||
| span.EndWithStatus(err) | ||
| }() | ||
|
|
||
| // Run local preflight validation before sending to Azure. | ||
| // Local validation catches common issues without requiring a network round-trip. | ||
| localPreflight := newLocalArmPreflight(modulePath, p.bicepCli, target) |
There was a problem hiding this comment.
The PR title/description focus on extension authoring model documentation, but this change also adds new preflight telemetry instrumentation and new tracing fields/events. Please either update the PR description to include this scope (and the motivation for it) or split the telemetry changes into a separate PR to keep review scope clear.
Adds extension-authoring-models.md documenting the two ways to build azd extensions and when to use each: - SDK-Managed: uses NewExtensionRootCommand()/azdext.Run() for automatic global flag handling, OTel propagation, and error reporting (Go only) - Self-Managed: extension wires its own CLI root, full flag control, works with any language Includes decision guide, best practices, tradeoff tables, and current adoption state. References from extensions style guide. Closes #7405 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vhvb1989
force-pushed
the
docs/extension-authoring-models
branch
from
be773f5 to
6506191
Compare
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
reviewed
Mar 31, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7406
docs: Document extension authoring models - SDK-Managed vs Self-Managed by @vhvb1989
Summary
What: Adds documentation explaining the two extension authoring models (SDK-Managed vs Self-Managed) and instruments the local preflight validation pipeline with telemetry (diagnostic IDs, rule IDs, outcome tracking).
Why: With two authoring models now available since PR #6856, extension authors need clear guidance on trade-offs. The telemetry tracks preflight outcomes for false/true positive correlation with deployment results.
Assessment: The documentation is well-structured with practical examples and trade-off tables. The telemetry implementation follows established patterns (named returns + deferred EndWithStatus, usage-level attributes for correlation). A few factual claims in the doc don't match the current codebase.
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Quality | 0 | 1 | 1 | 1 |
| Total | 0 | 1 | 1 | 1 |
Key Findings
- [HIGH] Quality: Dead reference to
extension-flag-architecture.mdwhich doesn't exist in the repo - [MEDIUM] Quality:
NewContext()described as providing gRPC access in three places, but it only does OTel trace propagation - [LOW] Quality:
helpflag listed under "registered by the framework" but it's Cobra's built-in
Test Coverage Estimate
setPreflightOutcome: well covered (span + usage attributes, all 6 outcome values)PreflightCheckstruct: covered (DiagnosticID propagation, AddCheck stores RuleID)validatePreflighttelemetry integration: no end-to-end test (acceptable given complexity)- New
fields.goentries: declaration-only, no logic to test
What's Done Well
- Clear, practical decision guide with concrete examples in both Go and Python
- Telemetry design uses usage-level attributes for cross-span correlation - enables false positive tracking
- Proper use of named returns + deferred EndWithStatus pattern per repo conventions
- Table-driven test covering all 6 outcome values
- cspell override for "actioned" properly scoped to the specific file
3 inline comments below.
Comment on lines
+227
to
+228
| - [Extension Flag Architecture](../design/extension-flag-architecture.md) — | ||
| flag dispatch spec (PR #7312) |
Member
There was a problem hiding this comment.
[HIGH] Quality: Dead reference - file doesn't exist
../design/extension-flag-architecture.md isn't in the repo. If it's planned for PR #7312, either include it in this PR, link directly to the PR instead, or remove the reference until the file lands. Per AGENTS.md: Don't reference files, scripts, directories, or workflows that don't exist in the PR.
Comment on lines
+169
to
+170
| - **gRPC access token injection:** Call `azdext.NewContext()` (Go) or read | ||
| `AZD_ACCESS_TOKEN` and `AZD_SERVER` directly |
Member
There was a problem hiding this comment.
[MEDIUM] Quality: NewContext() doesn't inject gRPC access token
NewContext() only extracts OTel trace context from TRACEPARENT/TRACESTATE (see pkg/azdext/context.go). The gRPC access token is injected by WithAccessToken(ctx), which Run() calls internally but NewContext() doesn't. This same inaccuracy appears at lines 118 and 207. For Self-Managed extensions needing gRPC, the guidance should either mention WithAccessToken() or just recommend reading AZD_ACCESS_TOKEN/AZD_SERVER directly.
| | `debug` | — | Enables debug logging | | ||
| | `no-prompt` | — | Non-interactive mode | | ||
| | `output` | `o` | Output format (json, table, none) | | ||
| | `help` | `h` | Command help (Cobra built-in) | |
Member
There was a problem hiding this comment.
[LOW] Quality: help isn't registered by NewExtensionRootCommand()
The section header says registered by NewExtensionRootCommand() as persistent flags but help/-h is Cobra's automatic built-in. Consider a footnote or separating it from the framework-registered flags to avoid confusion.
jongio
added a commit
that referenced
this pull request
Mar 31, 2026
Document that reserved flag enforcement only applies to SDK-managed extensions using azdext.Run(). Self-managed extensions (6 of 9 in-repo) bypass enforcement. Reference #7405 and #7406 for tracking and the authoring models documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
added a commit
that referenced
this pull request
Mar 31, 2026
Document that reserved flag enforcement only applies to SDK-managed extensions using azdext.Run(). Self-managed extensions (6 of 9 in-repo) bypass enforcement. Reference #7405 and #7406 for tracking and the authoring models documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wbreza
reviewed
Mar 31, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
PR Review — #7406
docs: Document extension authoring models — SDK-Managed vs Self-Managed by @vhvb1989
Summary
What: Adds comprehensive documentation comparing SDK-Managed (NewExtensionRootCommand() + azdext.Run()) vs Self-Managed (custom CLI root) extension authoring models. Includes flag propagation mechanics, tradeoff tables, Go/Python examples, decision guide, and current adoption state.
Why: With two authoring models available since PR #6856, extension authors need clear guidance to make an informed choice.
Assessment: Excellent documentation that fills a critical gap. Well-structured with practical examples and balanced tradeoff analysis. A few factual claims should be corrected before merge — NewContext() capabilities, help flag attribution, and one claim that will go stale when #7314 merges.
Prior Review Verification
@jongio's review was on an earlier version that included telemetry code changes (since force-pushed out). Of 3 findings:
| # | Priority | Finding | Current Status |
|---|---|---|---|
| 1 | High | Dead reference to extension-flag-architecture.md |
✅ Resolved (no longer in file) |
| 2 | Medium | NewContext() described as providing gRPC access |
⌛ Still applies |
| 3 | Low | help listed under framework-registered flags |
⌛ Still applies |
New Finding
| # | Priority | Finding |
|---|---|---|
| 4 | Low | -e/--environment described as "not pre-parsed" will go stale when #7314 merges |
What's Done Well
- Fills a critical documentation gap for extension authors choosing between models
- Excellent "Background: How azd Invokes Extensions" section makes the mechanics clear
- Balanced tradeoff analysis with honest "What can be hard to anticipate" section
- Multi-language examples (Go + Python) make it accessible to all extension authors
- Decision guide with concrete criteria, plus a practical hybrid approach recommendation
- Current state table grounds the discussion in reality
Review performed with GitHub Copilot CLI
| import typer | ||
| app = typer.Typer() | ||
|
|
||
| @app.command() |
Contributor
There was a problem hiding this comment.
[MEDIUM] NewContext() inaccurately described as providing gRPC access
Agreeing with @jongio — azdext.NewContext() only extracts OTel trace context from TRACEPARENT/TRACESTATE. The gRPC access token is injected by WithAccessToken(ctx), which Run() calls internally but NewContext() does not.
This same claim appears at lines ~140 ("optionally calls azdext.NewContext() (Go) for gRPC access") and ~195 ("gRPC access token injection: Call azdext.NewContext() (Go)"). For Self-Managed extensions needing gRPC, the guidance should recommend reading AZD_ACCESS_TOKEN/AZD_SERVER directly or calling WithAccessToken(ctx) explicitly.
| `--cwd`, and `--no-prompt` from the full argument list and binds them into | ||
| `GlobalCommandOptions`. Trace-related flags are parsed separately by the telemetry | ||
| system. Note: `-e/--environment` is **not** pre-parsed — it is a per-command flag. | ||
|
|
Contributor
There was a problem hiding this comment.
[LOW] -e/--environment claim will go stale when PR #7314 merges
This says "-e/--environment is not pre-parsed — it is a per-command flag." PR #7314 (approved, ready to merge) adds -e/--environment to ParseGlobalFlags(). Once it merges, this statement becomes incorrect. Consider either removing the note or adding temporal context.
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
Documents the two authoring models for azd extensions and helps extension authors choose the right one.
Closes #7405
What's included
cli/azd/docs/extensions/extension-authoring-models.md— New document that:NewExtensionRootCommand()+azdext.Run()) and Self-Managed (custom CLI root)cli/azd/docs/extensions/extensions-style-guide.md— Added reference to the new doc under "Parameter and Flag Consistency"Context
After PR #6856 introduced
NewExtensionRootCommand()andazdext.Run()(Feb 2026), there are now two ways to build extensions. The SDK-Managed model provides convenience but introduces opaque flag dependencies and is Go-only. The Self-Managed model gives full control and works with any language but requires manual handling of azd's global flags.PR #7312 adds reserved flag enforcement that only applies to SDK-Managed extensions using
azdext.Run(), widening the gap between the two models.Extension authors need clear guidance to make an informed choice.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com