Govern first-party extension telemetry attributes - #10068
Draft
Marina He (hemarina) wants to merge 12 commits into
Draft
Marina He (hemarina) wants to merge 12 commits into
Marina He (hemarina) wants to merge 12 commits into
Conversation
Add a source-declared inventory and PR validation for first-party extension telemetry attributes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 20 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
The scanner can reject unrelated extension code, and endpoint metadata validation remains incomplete.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds source-governed telemetry metadata and validation for first-party extensions without changing runtime reporting.
Changes:
- Adds an extension telemetry field inventory and AST-based validator.
- Updates telemetry documentation and contributor guidance.
- Adds an always-running PR validation workflow.
File summaries
| File | Description |
|---|---|
docs/specs/metrics-audit/telemetry-schema.md |
Documents extension fields and governance. |
docs/specs/metrics-audit/privacy-review-checklist.md |
Adds extension declaration requirements. |
docs/specs/metrics-audit/feature-telemetry-matrix.md |
Updates extension telemetry coverage. |
docs/reference/telemetry-data.md |
Documents concrete extension fields. |
docs/guides/creating-an-extension.md |
Links extension telemetry guidance. |
docs/concepts/glossary.md |
Updates usage-event terminology. |
docs/architecture/telemetry.md |
Describes source validation architecture. |
docs/architecture/adr-001-extension-telemetry-events.md |
Revises the telemetry design decision. |
cli/azd/internal/tracing/fields/key_test.go |
Tests metadata-free dynamic keys. |
cli/azd/internal/tracing/fields/fields.go |
Removes default dynamic-key classification. |
cli/azd/extensions/telemetry/source_scanner_test.go |
Implements source scanning. |
cli/azd/extensions/telemetry/source_scanner_cases_test.go |
Tests scanner grammar and failures. |
cli/azd/extensions/telemetry/fields.go |
Declares extension field metadata. |
cli/azd/extensions/telemetry/fields_test.go |
Validates declaration coverage. |
cli/azd/extensions/telemetry/declaration_validation_test.go |
Validates declaration metadata. |
cli/azd/extensions/azure.ai.agents/internal/cmd/telemetry.go |
Inlines telemetry attributes. |
cli/azd/docs/extensions/extension-telemetry.md |
Documents declaration workflow. |
cli/azd/cmd/telemetry_test.go |
Recognizes the extension registry. |
cli/azd/AGENTS.md |
Adds contributor requirements. |
.github/workflows/ext-telemetry-ci.yml |
Adds the telemetry PR check. |
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 4
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keep parser-local lexical identity behind one documented Staticcheck exception for the source-only scanner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
ext-demo
Scope scanner activation through local package dependencies, pin workflow actions, clarify endpoint validation, and complete schema metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The required telemetry checklist and privacy-review record are missing from the PR description.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
cli/azd/extensions/telemetry/fields.go:16
- These new
AttributeKeydeclarations require the telemetry PR checklist and a privacy review, but the PR description includes neither.cli/azd/AGENTS.md:337-340anddocs/specs/metrics-audit/privacy-review-checklist.md:241-271require both; add the checklist and record the review before merging.
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Validation currently ignores field ownership, and the required privacy-review checklist is absent from the PR description.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The classifier documentation is inconsistent and mandatory owner/privacy review remains pending.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The validator rejects two classifications that the updated documentation presents as valid for extension fields.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Privacy approval remains pending, and the new static-analysis enforcement requires final human review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/ext-telemetry-ci.yml:15
- The new validator package is excluded from core lint by
.github/workflows/cli-ci.yml:7-8and does not match an extension-specific lint workflow, while this job only runs its tests. Future changes to this large Go package can therefore bypass the repository's golangci-lint andgo fixchecks. Add a reusablelint-go.ymljob forcli/azd/extensions/telemetry.
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The scanner can reject unrelated same-package code, public SDK documentation remains contradictory, and privacy review is pending.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
ext-foundry
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The scanner misses Go 1.26 new(value) payloads, and privacy approval remains pending.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
docs/specs/metrics-audit/telemetry-schema.md:293
ext.agent.operationis classified as a bounded product enum, but this row does not document its allowed set; “Fixed extension command path” is only a derivation description. The telemetry schema contract requires allowed enum values, and the PR checklist marks that item complete. Enumerate the current operation values (includingunknown) so privacy reviewers can verify boundedness and future extensions can determine whether reuse is semantically valid.
- Files reviewed: 22/22 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Selector-based factory and method returns can bypass the required telemetry source validation.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
cli/azd/extensions/telemetry/source_scanner_test.go:1677
- Calls through a selector are never resolved here. A valid pattern such as
event := helper.NewEvent(); event.Attributes[dynamicKey] = value(or a method returningEvent) therefore does not markeventas a telemetry payload, so the later mutation is skipped with no diagnostic and an undeclared dynamic field can pass this required check. Resolve return types for local-package selector calls/methods, or fail closed when their result is used throughAttributes, and add regressions for both forms.
- Files reviewed: 22/22 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Payload provenance gaps allow valid Go expressions to bypass undeclared telemetry-key validation.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
cli/azd/extensions/telemetry/source_scanner_test.go:1836
- Type conversions are not propagated as payload-producing calls. For example,
event := foundryTelemetry.Event(localEvent{})is scanned at construction, buteventis not added topayloadObjects; a laterevent.Attributes[dynamicKey] = valuetherefore bypasses both the dynamic-key and post-construction diagnostics. Recognize one-argument conversions whose target resolves to a telemetry payload, and add a mutation regression.
cli/azd/extensions/telemetry/source_scanner_test.go:1814
- A dereferenced payload loses provenance because Go represents
*requestasast.StarExpr, notast.UnaryExpr. Thus(*request).Attributes[dynamicKey] = valueis accepted without discovering the key, even whenrequestis a knownReportUsageRequest. Propagate payload status throughStarExprand cover this form in the scanner cases.
case *ast.UnaryExpr:
return value.Op == token.AND && isTelemetryPayloadExpression(value.X, source, pkg)
- Files reviewed: 22/22 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Comment on lines
+1562
to
+1566
| case *ast.UnaryExpr: | ||
| if value.Op == token.AND || value.Op == token.MUL { | ||
| return localExpressionTypeName(value.X, pkg, resolving) | ||
| } | ||
| return "" |
Contributor
azure.ai.agents 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/10068/azure-ai-agents.zip"
|
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.
Fix https://github.com/Azure/azure-dev-pr/issues/1815
Summary
Validation
go test ./extensions/telemetry -count=1go vet ./extensions/telemetrygolangci-lint run ./extensions/telemetry --timeout 10m --max-same-issues 0go test ./internal/tracing/fields -count=1go test ./cmd -run '^(TestTelemetryEventConstants|TestTelemetryFieldConstants|TestNoRawTelemetryAttributes|TestRawTelemetryAttributeScanner)$' -count=1go test ./internal/cmd -run '^(TestAgentContextReporter.*|TestTelemetryClassifications.*)$' -count=1fromextensions/azure.ai.agentsgit diff --checkPrivacy review status
Pending owner/privacy review. This PR remains draft and does not claim privacy approval.
Telemetry Change Checklist
New Fields
fields/fields.go, or first-party extension field defined inextensions/telemetry/fields.godocs/specs/metrics-audit/telemetry-schema.mdgo test ./extensions/telemetrypasses for extensionReportUsagechangesNew Events
events/events.go(N/A; no new core event is introduced)constwhose Go identifier containsEvent(end it withPrefixfor a prefix-match group) so repository metadata tooling discovers it (N/A; no new core event is introduced)docs/specs/metrics-audit/telemetry-schema.md(N/A; no new event is introduced)prefix.noun.verb) (N/A; existing extension event names are unchanged)Privacy
CustomerContentemitted in telemetryTesting
Downstream
Documentation
docs/specs/metrics-audit/feature-telemetry-matrix.md)if a new command emits telemetry, a gap is being closed, or a new cross-cutting
subsystem is added
docs/specs/metrics-audit/telemetry-schema.md) withnew field/event, including its Hashing section if the field is hashed
privacy-review-checklist.md) updated ifa new field is hashed or a previously-raw field becomes hashed (N/A; no hashed field changes)