Skip to content

feat: add host-validated extension telemetry gRPC service#9174

Open
huimiu wants to merge 4 commits into
mainfrom
hui/telemetry-deployment-mode
Open

feat: add host-validated extension telemetry gRPC service#9174
huimiu wants to merge 4 commits into
mainfrom
hui/telemetry-deployment-mode

Conversation

@huimiu

@huimiu huimiu commented Jul 16, 2026

Copy link
Copy Markdown
Member

Fixes #9255. Part of #9230 (the producer lands in a follow-up PR).

Why

We want to know which path a hosted agent deploy actually took: source code (code), a container azd built (container), or an existing image the user brought (byo_image). The manifest language alone can't tell these apart, so adoption and troubleshooting telemetry is ambiguous today.

An earlier version of this PR taught azd core to read that value out of deploy artifact metadata and hardcoded the azure.ai.agents extension. That was rejected: it pointed the dependency the wrong way (core knowing about one specific extension), shipped telemetry-only data through the deploy artifact contract, and would force a core change for every future extension.

What this PR does

This PR replaces that with a generic, host-controlled way for any authenticated azd extension to contribute a telemetry value, and registers the first field for it. It is core-only: the plumbing and the field are in, but nothing emits a value yet. The agents extension starts producing agent.deploy.mode in a follow-up PR, after this ships in a core release.

  • New gRPC TelemetryService with a single AddCommandUsageAttribute(key, value) call. An extension calls it; azd core validates the key/value against a host-owned allowlist and attaches the value to the current command's telemetry event (cmd.deploy or cmd.up) as a de-duplicated string slice.
  • Register agent.deploy.mode as SystemMetadata / FeatureInsight with the fixed values code, container, byo_image.
  • Command-scoped storage in core so a value lands only on the owning command event and never leaks onto synthetic child spans or sibling commands.
  • Authenticated and capability-gated: only a signed extension with the service-target-provider capability may report, and only for allowed keys/values. Invalid input is rejected and never echoed into errors or logs.
  • Best-effort and backward compatible: telemetry never affects deploy; an older host returns Unimplemented and the extension ignores it.
  • Add a generic azd.fromPackage artifact provenance marker (payload provenance, not telemetry) so a future producer can tell a user-supplied --from-package payload from one azd built.
  • Document the SDK API and the registered field (SDK reference and telemetry schema). The user-facing "emitted data" docs land with the producer in the follow-up PR, since nothing emits the field yet.

Why this approach

Core stays extension-agnostic: it owns the telemetry schema (key, allowed values, classification, purpose, eligible commands) and validates everything an extension sends. Extensions can't invent telemetry or push arbitrary data, which keeps azd's existing per-field privacy governance intact across the extension boundary. Values are collected per command and flushed onto the correct command span, so they can't leak onto synthetic package/provision spans or later commands.

Open question — the allowlist

Core keeps an allowlist: every key/value an extension can send is pre-registered in core with its classification and eligible commands.

  • Pro: preserves our strict privacy model — no extension can dump arbitrary data (PII, resource names, paths) into azd telemetry.
  • Con: a new telemetry field can't ship with the extension alone — it has to be added in core and picked up in a released azd, so extension telemetry is tied to the core release cadence (the same as all other azd telemetry today).
  • Decision needed: keep the strict allowlist, or loosen it (e.g. namespaced keys) and accept more privacy risk. Worth a quick check with the telemetry/privacy owner.

Follow-up (not in this PR)

Tracked in #9230:

  • Agents extension producer: report agent.deploy.mode at the earliest authoritative decision, on a released core SDK.
  • User-facing telemetry docs (docs/reference/telemetry-data.md, feature-telemetry-matrix.md) once the field actually emits.

Testing

go build, targeted unit and integration tests, gofmt, go vet, golangci-lint (0 issues), go fix -diff (clean), and cspell all pass. Race checks run in CI.

Telemetry Change Checklist

  • Field defined in fields/fields.go with correct classification (SystemMetadata) and purpose (FeatureInsight)
  • Field documented in docs/specs/metrics-audit/telemetry-schema.md
  • Enum values documented with the allowed value set (code, container, byo_image)
  • Hashing applied where required — N/A; only fixed enum values are accepted
  • Measurement OTel type — N/A; this is a string slice, not a measurement
  • No customer content or PII: only the three fixed enums can be accepted; unknown keys and values are rejected and never logged
  • Privacy review of the new field and the new extension-to-host telemetry boundary

@azure-pipelines

This comment was marked as outdated.

@github-actions github-actions Bot added the ext-agents azure.ai.agents extension label Jul 16, 2026
@huimiu
huimiu force-pushed the hui/telemetry-deployment-mode branch from 4c63574 to d1f3de6 Compare July 21, 2026 07:39
@huimiu
huimiu marked this pull request as ready for review July 21, 2026 10:18
Copilot AI review requested due to automatic review settings July 21, 2026 10:18
@azure-pipelines

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

This comment was marked as outdated.

jongio

This comment was marked as outdated.

Copilot AI review requested due to automatic review settings July 21, 2026 13:13

This comment was marked as outdated.

jongio

This comment was marked as outdated.

Copilot AI review requested due to automatic review settings July 21, 2026 15:35

This comment was marked as outdated.

jongio

This comment was marked as outdated.

Copilot AI review requested due to automatic review settings July 21, 2026 15:53

This comment was marked as outdated.

jongio

This comment was marked as outdated.

@vhvb1989 vhvb1989 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design concern — please review before accepting as proposed.

My main concern with this PR is that it overloads the result Artifact.Metadata bag to carry telemetry-only data (azure.ai.agents.deploymentMode) alongside genuine payload information (location, kind, sha256), and then teaches azd core how to fish that value back out based on hardcoded knowledge of a single extension (if extension.Id == "azure.ai.agents" + the extension's private metadata key). This inverts azd's normal dependency direction — core is supposed to stay extension-agnostic, with extensions depending on the SDK, not the other way around. The result is a stringly-typed, cross-module side-channel (the magic strings are duplicated in core and in the extension with no compiler enforcement) that doesn't generalize: any future hosted-agent-style extension gets nothing until someone edits a core allowlist. It seems like a lot of core plumbing for what is ultimately telemetry generation.

I think the cleaner approach is a generic capability — e.g. an SDK/gRPC method that lets an extension set a telemetry attribute directly — so the extension emits agent.deploy.mode itself, Artifact stays purely about payload, and core stays extension-agnostic with no hardcoded extension name. Same telemetry outcome, none of the inverted coupling. Can we discuss whether that direction is preferable before merging this as-is?

jongio

This comment was marked as outdated.

@huimiu
huimiu marked this pull request as draft July 22, 2026 01:53
@huimiu
huimiu force-pushed the hui/telemetry-deployment-mode branch from a2bc94c to 5644075 Compare July 22, 2026 09:01
@github-actions github-actions Bot added the area/extensions Extensions (general) label Jul 22, 2026
@huimiu
huimiu force-pushed the hui/telemetry-deployment-mode branch from 5644075 to 38fd61a Compare July 22, 2026 09:40
@huimiu huimiu changed the title feat: record hosted agent deployment mode feat: add host-validated extension telemetry gRPC service Jul 22, 2026
@Azure Azure deleted a comment from azure-sdk-automation Bot Jul 22, 2026
@huimiu huimiu linked an issue Jul 22, 2026 that may be closed by this pull request
@huimiu
huimiu marked this pull request as ready for review July 22, 2026 10:17
Copilot AI review requested due to automatic review settings July 22, 2026 10:18
@azure-pipelines

This comment was marked as outdated.

This comment was marked as outdated.

Copilot AI review requested due to automatic review settings July 22, 2026 12:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 25 changed files in this pull request and generated 1 comment.

Files not reviewed (2)
  • cli/azd/pkg/azdext/telemetry.pb.go: Generated file
  • cli/azd/pkg/azdext/telemetry_grpc.pb.go: Generated file
Comments suppressed due to low confidence (2)

docs/specs/metrics-audit/telemetry-schema.md:91

  • This field is emit-capable once this host ships, even if the first-party producer follows later. Repository telemetry guidance requires new fields to be reflected in docs/reference/telemetry-data.md and new cross-cutting emission paths in feature-telemetry-matrix.md (cli/azd/AGENTS.md:246-281); currently neither mentions this field/service. Add those entries now so the public reference and audit inventory stay aligned with the host behavior. [azd-code-reviewer]
| Agent deployment mode | `agent.deploy.mode` | SystemMetadata | FeatureInsight | `string[]`; per-command set of `code`/`container`/`byo_image` contributed by an authenticated extension through the telemetry service; fixed enum, not hashed; App Insights stores JSON text |

cli/azd/internal/cmd/from_package_test.go:20

  • This test only verifies that two constants are equal; it never exercises either changed --from-package artifact construction path. Both metadata assignments in service_graph.go and publish.go could be removed while this still passes. Add behavior tests for both paths that inspect the resulting package artifact and assert azd.fromPackage == "true". [azd-code-reviewer]
func TestFromPackageMetadataConstant(t *testing.T) {
	t.Parallel()
	require.Equal(t, "azd.fromPackage", project.MetadataKeyFromPackage)
	require.Equal(t, azdext.ArtifactMetadataKeyFromPackage, project.MetadataKeyFromPackage)

// service, which routes them to the current scope. Closing the scope below
// attaches those values to this command span only, so they never leak onto
// synthetic child spans or sibling commands.
usageScope := tracing.BeginCommandUsageScope(eventName)
@azure-sdk-automation

This comment was marked as off-topic.

@jongio jongio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic host-validated telemetry service resolves the coupling concern from my earlier note. Core no longer hardcodes the agents extension id or reads its private metadata key: eligibility now runs through the signed capability check plus the host-owned key/value allowlist, and the from-package marker is a plain provenance boolean with the constant aliased from the SDK (MetadataKeyFromPackage = azdext.ArtifactMetadataKeyFromPackage) so core and extensions can't drift. That's the right direction, and it keeps Artifact.Metadata about payload rather than telemetry.

One thing to settle before merge, and it's your own open item rather than a new one: the privacy review of the new extension-to-host telemetry boundary is still unchecked. The allowlist keeps the strict model intact, but the choice to keep it strict versus loosening to namespaced keys should get a sign-off from the telemetry/privacy owner, since it changes who can put values into azd telemetry. Worth locking that down instead of carrying it as an open question into merge.

The test-coverage and docs gaps the automated reviewer flagged (publish not classified in TestCommandTelemetryCoverage, from_package_test only asserting constant equality) are worth a quick pass. Deferring the emitted-data docs to the producer PR makes sense given nothing emits the field yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/extensions Extensions (general) ext-agents azure.ai.agents extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a generic host-validated extension telemetry gRPC API

4 participants