Add x-ms-client-request-id header, use azdext.Run - #7010
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the azure.ai.finetune extension to align with the newer azd extension entrypoint and to propagate a Microsoft request/correlation ID header on outbound Azure SDK requests for improved trace correlation.
Changes:
- Switch extension entrypoint to
azdext.Run(cmd.NewRootCommand())(enables TRACEPARENT propagation, FORCE_COLOR handling, structured error reporting). - Add an azsdk policy to set an
x-ms-*-request-idheader on Azure SDK pipelines used by the extension.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cli/azd/extensions/azure.ai.finetune/main.go |
Replaces manual context + error printing with azdext.Run entrypoint. |
cli/azd/extensions/azure.ai.finetune/internal/providers/factory/provider_factory.go |
Adds azsdk.NewMsClientRequestIdPolicy() to Azure SDK per-call policies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Wallace Breza (wbreza)
left a comment
There was a problem hiding this comment.
Code Review - PR #7010
Overall Summary
Clean, focused PR that makes two well-understood changes to the azure.ai.finetune extension:
- Adds
x-ms-client-request-idheader viaazsdk.NewMsClientRequestIdPolicy()to both SDK client creation sites - Replaces manual cobra command setup with the standard
azdext.Run()entrypoint pattern
No issues found. Net deletion of 14 lines by leveraging the framework.
What Looks Good
azdext.Run()migration correctly matches the standard extension entrypoint pattern (same asazure.ai.agents)NewMsClientRequestIdPolicy()added consistently to both SDK client creation sites inprovider_factory.go- Good code reduction by removing boilerplate
Minor Observations
- Sparse PR description - the body only says "Refer -> #6969". A one-liner summarizing the two changes would help reviewers.
FORCE_COLORhandling removed - theinit()function inmain.gopreviously handledFORCE_COLOR=1. Presumablyazdext.Run()handles this internally - worth confirming.- CI is failing - not related to code quality, but worth investigating before merge.
| Priority | Count |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 0 |
| Low | 0 |
| Total | 0 |
Overall Assessment: Approve
Wallace Breza (wbreza)
left a comment
There was a problem hiding this comment.
Re-review: Approve
The only change since my previous approval is the go.mod/go.sum dependency bump to an azd version (a633d43bb543) that includes NewMsClientRequestIdPolicy(). This resolves the earlier compilation concern. No code logic changes.
All CI checks passing. Looks good to merge! ![]()
* Add x-ms-client-request-id header, use azdext.Run * Update `github.com/azure/azure-dev/cli/azd` --------- Co-authored-by: Jeffrey Chen <jeffreychen@microsoft.com>
Fixes #6993
Refer -> #6969