From 406edb8a3e9ef839699b9903dd60d8f4c76217d8 Mon Sep 17 00:00:00 2001 From: huimiu Date: Wed, 22 Jul 2026 16:45:34 +0800 Subject: [PATCH 01/12] feat: add host-validated extension telemetry gRPC service --- cli/azd/cmd/container.go | 1 + cli/azd/cmd/middleware/telemetry.go | 16 ++ cli/azd/cmd/telemetry_test.go | 13 + cli/azd/grpc/proto/telemetry.proto | 33 +++ cli/azd/internal/cmd/from_package_test.go | 54 ++++ cli/azd/internal/cmd/publish.go | 28 +- cli/azd/internal/cmd/service_graph.go | 7 +- .../grpcserver/prompt_service_test.go | 1 + cli/azd/internal/grpcserver/server.go | 4 + cli/azd/internal/grpcserver/server_test.go | 74 +++++- .../internal/grpcserver/telemetry_service.go | 131 +++++++++ .../grpcserver/telemetry_service_test.go | 250 ++++++++++++++++++ cli/azd/internal/tracing/command_usage.go | 147 ++++++++++ .../internal/tracing/command_usage_test.go | 222 ++++++++++++++++ cli/azd/internal/tracing/fields/fields.go | 13 + cli/azd/pkg/azdext/artifact_metadata.go | 9 + cli/azd/pkg/azdext/azd_client.go | 11 + cli/azd/pkg/azdext/telemetry.pb.go | 189 +++++++++++++ cli/azd/pkg/azdext/telemetry/attributes.go | 28 ++ .../pkg/azdext/telemetry/attributes_test.go | 28 ++ cli/azd/pkg/azdext/telemetry_grpc.pb.go | 140 ++++++++++ cli/azd/pkg/project/artifact.go | 7 + 22 files changed, 1397 insertions(+), 9 deletions(-) create mode 100644 cli/azd/grpc/proto/telemetry.proto create mode 100644 cli/azd/internal/cmd/from_package_test.go create mode 100644 cli/azd/internal/grpcserver/telemetry_service.go create mode 100644 cli/azd/internal/grpcserver/telemetry_service_test.go create mode 100644 cli/azd/internal/tracing/command_usage.go create mode 100644 cli/azd/internal/tracing/command_usage_test.go create mode 100644 cli/azd/pkg/azdext/artifact_metadata.go create mode 100644 cli/azd/pkg/azdext/telemetry.pb.go create mode 100644 cli/azd/pkg/azdext/telemetry/attributes.go create mode 100644 cli/azd/pkg/azdext/telemetry/attributes_test.go create mode 100644 cli/azd/pkg/azdext/telemetry_grpc.pb.go diff --git a/cli/azd/cmd/container.go b/cli/azd/cmd/container.go index 4aa90b41155..61a0c5194b2 100644 --- a/cli/azd/cmd/container.go +++ b/cli/azd/cmd/container.go @@ -1035,6 +1035,7 @@ func registerCommonDependencies(container *ioc.NestedContainer) { ) container.MustRegisterSingleton(grpcserver.NewAiModelService) container.MustRegisterScoped(grpcserver.NewCopilotService) + container.MustRegisterSingleton(grpcserver.NewTelemetryService) // Required for nested actions called from composite actions like 'up' registerAction[*cmd.ProvisionAction](container, "azd-provision-action") diff --git a/cli/azd/cmd/middleware/telemetry.go b/cli/azd/cmd/middleware/telemetry.go index 1a105878c47..7033343dd7b 100644 --- a/cli/azd/cmd/middleware/telemetry.go +++ b/cli/azd/cmd/middleware/telemetry.go @@ -65,6 +65,13 @@ func (m *TelemetryMiddleware) Run(ctx context.Context, next NextFn) (*actions.Ac spanCtx, span := tracing.Start(ctx, eventName) + // Begin a command usage scope keyed by this exact event name. Extensions + // contribute host-validated usage attributes through the telemetry gRPC + // 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) + log.Printf("TraceID: %s", span.SpanContext().TraceID()) if !IsChildAction(ctx) { @@ -103,6 +110,15 @@ func (m *TelemetryMiddleware) Run(ctx context.Context, next NextFn) (*actions.Ac m.setInstalledExtensionsAttributes(span) defer func() { + // Attach any command-scoped usage attributes reported by extensions, + // then close the scope. A close error is unexpected and only logged; + // it never changes the command result. + if commandAttrs, closeErr := tracing.CloseCommandUsageScope(usageScope); closeErr != nil { + log.Printf("closing command usage scope: %v", closeErr) + } else { + span.SetAttributes(commandAttrs...) + } + // Include any usage attributes set span.SetAttributes(tracing.GetUsageAttributes()...) span.SetAttributes(fields.PerfInteractTime.Int64(tracing.InteractTimeMs.Load())) diff --git a/cli/azd/cmd/telemetry_test.go b/cli/azd/cmd/telemetry_test.go index 18328955e78..54b92b6d854 100644 --- a/cli/azd/cmd/telemetry_test.go +++ b/cli/azd/cmd/telemetry_test.go @@ -83,6 +83,19 @@ func TestTelemetryFieldConstants(t *testing.T) { } }) + // Agent deployment mode telemetry field (contributed by extensions through + // the host-validated telemetry service). + t.Run("AgentDeploymentModeField", func(t *testing.T) { + t.Parallel() + require.Equal(t, "agent.deploy.mode", string(fields.AgentDeploymentModeKey.Key)) + require.Equal(t, fields.SystemMetadata, fields.AgentDeploymentModeKey.Classification) + require.Equal(t, fields.FeatureInsight, fields.AgentDeploymentModeKey.Purpose) + require.False(t, fields.AgentDeploymentModeKey.IsMeasurement) + + kv := fields.AgentDeploymentModeKey.StringSlice([]string{"code", "container", "byo_image"}) + require.Equal(t, []string{"code", "container", "byo_image"}, kv.Value.AsStringSlice()) + }) + // Tool command telemetry fields t.Run("ToolFields", func(t *testing.T) { t.Parallel() diff --git a/cli/azd/grpc/proto/telemetry.proto b/cli/azd/grpc/proto/telemetry.proto new file mode 100644 index 00000000000..8c6e20faa96 --- /dev/null +++ b/cli/azd/grpc/proto/telemetry.proto @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +syntax = "proto3"; + +package azdext; + +option go_package = "github.com/azure/azure-dev/cli/azd/pkg/azdext"; + +// TelemetryService accepts reviewed, host-owned command usage attribute values +// from authenticated extensions. The host validates the key and value against +// an allowlist and attaches accepted values to the current command telemetry +// event. Extensions cannot choose classification, purpose, hashing, or +// aggregation. +service TelemetryService { + // AddCommandUsageAttribute adds one bounded value to a host-owned command + // usage attribute. Duplicate values are collapsed by the host. + rpc AddCommandUsageAttribute(AddCommandUsageAttributeRequest) + returns (AddCommandUsageAttributeResponse); +} + +message AddCommandUsageAttributeRequest { + // Key must be registered and owned by the host. + string key = 1; + + // Value must be in the host-owned allowed set for key. + string value = 2; +} + +message AddCommandUsageAttributeResponse { + // Accepted is false when the value was rejected because no eligible command + // scope is currently active. A false response is not an error. + bool accepted = 1; +} diff --git a/cli/azd/internal/cmd/from_package_test.go b/cli/azd/internal/cmd/from_package_test.go new file mode 100644 index 00000000000..2f0e528a544 --- /dev/null +++ b/cli/azd/internal/cmd/from_package_test.go @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/project" +) + +// TestDetermineArtifactKind locks --from-package classification, including the +// compound archive suffixes that filepath.Ext cannot recognize. +func TestDetermineArtifactKind(t *testing.T) { + t.Parallel() + dir := t.TempDir() + + archiveNames := []string{ + "pkg.zip", "pkg.tar", "pkg.tgz", "pkg.txz", "pkg.tbz2", + "pkg.tar.gz", "pkg.tar.bz2", "pkg.tar.xz", + "PKG.TAR.GZ", // classification is case-insensitive + } + for _, name := range archiveNames { + p := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(p, []byte("x"), 0o600)) + require.Equalf(t, project.ArtifactKindArchive, determineArtifactKind(p), + "expected archive for %q", name) + } + + // An existing directory is a directory artifact. + require.Equal(t, project.ArtifactKindDirectory, determineArtifactKind(dir)) + + // A path that does not exist is treated as a container image reference. + require.Equal(t, project.ArtifactKindContainer, + determineArtifactKind("myregistry.azurecr.io/app:latest")) + + // An existing non-archive file is also treated as a container reference. + other := filepath.Join(dir, "notanarchive.bin") + require.NoError(t, os.WriteFile(other, []byte("x"), 0o600)) + require.Equal(t, project.ArtifactKindContainer, determineArtifactKind(other)) +} + +// TestFromPackageMetadataConstant keeps the core alias and the SDK constant in +// sync so core and extensions agree on the provenance marker. +func TestFromPackageMetadataConstant(t *testing.T) { + t.Parallel() + require.Equal(t, "azd.fromPackage", project.MetadataKeyFromPackage) + require.Equal(t, azdext.ArtifactMetadataKeyFromPackage, project.MetadataKeyFromPackage) +} diff --git a/cli/azd/internal/cmd/publish.go b/cli/azd/internal/cmd/publish.go index 4d109640e1c..bdae32d64dc 100644 --- a/cli/azd/internal/cmd/publish.go +++ b/cli/azd/internal/cmd/publish.go @@ -9,7 +9,6 @@ import ( "io" "log" "os" - "path/filepath" "slices" "strings" "time" @@ -286,11 +285,16 @@ func (pa *PublishAction) Run(ctx context.Context) (*actions.ActionResult, error) serviceContext := &project.ServiceContext{} if pa.flags.FromPackage != "" { - // --from-package set, skip packaging and create package artifact + // --from-package set, skip packaging and create package artifact. + // Mark it with generic provenance so a service target can tell a + // caller-supplied payload from one azd produced. err = serviceContext.Package.Add(&project.Artifact{ Kind: determineArtifactKind(pa.flags.FromPackage), Location: pa.flags.FromPackage, LocationKind: project.LocationKindLocal, + Metadata: map[string]string{ + project.MetadataKeyFromPackage: "true", + }, }) if err != nil { @@ -411,13 +415,23 @@ func (pa *PublishAction) supportsPublish(ctx context.Context, serviceConfig *pro // 1. Archive (zip file) - checks if it exists and matches popular archive extensions // 2. Directory - checks if it is an existing directory (absolute or relative) // 3. Container - otherwise, it's likely a local container reference +// archiveSuffixes are the file suffixes treated as a code archive for +// --from-package classification. Compound suffixes such as ".tar.gz" are +// matched with strings.HasSuffix because filepath.Ext only returns the final +// segment (".gz") and would misclassify them as a container reference. +var archiveSuffixes = []string{ + ".zip", ".tar", ".tar.gz", ".tgz", ".tar.bz2", ".tbz2", ".tar.xz", ".txz", +} + func determineArtifactKind(fromPackage string) project.ArtifactKind { - // Check if it's an existing file with archive extension + lower := strings.ToLower(fromPackage) + + // Check if it's an existing file with an archive suffix. if info, err := os.Stat(fromPackage); err == nil && !info.IsDir() { - ext := strings.ToLower(filepath.Ext(fromPackage)) - switch ext { - case ".zip", ".tar", ".tar.gz", ".tgz", ".tar.bz2", ".tbz2", ".tar.xz", ".txz": - return project.ArtifactKindArchive + for _, suffix := range archiveSuffixes { + if strings.HasSuffix(lower, suffix) { + return project.ArtifactKindArchive + } } } diff --git a/cli/azd/internal/cmd/service_graph.go b/cli/azd/internal/cmd/service_graph.go index f8adcbfa3aa..0e228159b11 100644 --- a/cli/azd/internal/cmd/service_graph.go +++ b/cli/azd/internal/cmd/service_graph.go @@ -373,11 +373,16 @@ func addServiceStepsToGraph(g *exegraph.Graph, opts serviceGraphOptions) (*servi if opts.fromPackage != "" { // --from-package bypasses the packager and wraps the - // user-supplied artifact directly. + // user-supplied artifact directly. Mark it with generic + // provenance so a service target can distinguish a + // caller-supplied payload from one azd produced. if pkgErr := sc.Package.Add(&project.Artifact{ Kind: determineArtifactKind(opts.fromPackage), Location: opts.fromPackage, LocationKind: project.LocationKindLocal, + Metadata: map[string]string{ + project.MetadataKeyFromPackage: "true", + }, }); pkgErr != nil { return fmt.Errorf("packaging service %s: %w", pkgSvc.Name, pkgErr) } diff --git a/cli/azd/internal/grpcserver/prompt_service_test.go b/cli/azd/internal/grpcserver/prompt_service_test.go index 99868e564c9..5fac998dc8b 100644 --- a/cli/azd/internal/grpcserver/prompt_service_test.go +++ b/cli/azd/internal/grpcserver/prompt_service_test.go @@ -622,6 +622,7 @@ func setupTestServer(t *testing.T, promptSvc azdext.PromptServiceServer) ( azdext.UnimplementedCopilotServiceServer{}, azdext.UnimplementedProvisioningServiceServer{}, azdext.UnimplementedValidationServiceServer{}, + azdext.UnimplementedTelemetryServiceServer{}, ) serverInfo, err := server.Start() diff --git a/cli/azd/internal/grpcserver/server.go b/cli/azd/internal/grpcserver/server.go index 48779be0d8d..7215343dc70 100644 --- a/cli/azd/internal/grpcserver/server.go +++ b/cli/azd/internal/grpcserver/server.go @@ -44,6 +44,7 @@ type Server struct { copilotService azdext.CopilotServiceServer provisioningService azdext.ProvisioningServiceServer validationService azdext.ValidationServiceServer + telemetryService azdext.TelemetryServiceServer } func NewServer( @@ -64,6 +65,7 @@ func NewServer( copilotService azdext.CopilotServiceServer, provisioningService azdext.ProvisioningServiceServer, validationService azdext.ValidationServiceServer, + telemetryService azdext.TelemetryServiceServer, ) *Server { return &Server{ projectService: projectService, @@ -83,6 +85,7 @@ func NewServer( copilotService: copilotService, provisioningService: provisioningService, validationService: validationService, + telemetryService: telemetryService, } } @@ -132,6 +135,7 @@ func (s *Server) Start() (*ServerInfo, error) { azdext.RegisterCopilotServiceServer(s.grpcServer, s.copilotService) azdext.RegisterProvisioningServiceServer(s.grpcServer, s.provisioningService) azdext.RegisterValidationServiceServer(s.grpcServer, s.validationService) + azdext.RegisterTelemetryServiceServer(s.grpcServer, s.telemetryService) serverInfo.Address = fmt.Sprintf("127.0.0.1:%d", randomPort) serverInfo.Port = randomPort diff --git a/cli/azd/internal/grpcserver/server_test.go b/cli/azd/internal/grpcserver/server_test.go index 5289e31344e..43951327022 100644 --- a/cli/azd/internal/grpcserver/server_test.go +++ b/cli/azd/internal/grpcserver/server_test.go @@ -23,10 +23,12 @@ import ( "google.golang.org/protobuf/proto" "github.com/azure/azure-dev/cli/azd/internal" + "github.com/azure/azure-dev/cli/azd/internal/tracing" "github.com/azure/azure-dev/cli/azd/pkg/account" "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/azapi" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/azure/azure-dev/cli/azd/pkg/prompt" @@ -53,6 +55,7 @@ func Test_Server_Start(t *testing.T) { azdext.UnimplementedCopilotServiceServer{}, azdext.UnimplementedProvisioningServiceServer{}, azdext.UnimplementedValidationServiceServer{}, + NewTelemetryService(), ) serverInfo, err := server.Start() @@ -119,6 +122,74 @@ func Test_Server_Start(t *testing.T) { require.True(t, ok) require.Equal(t, codes.Unauthenticated, st.Code()) }) + + t.Run("TelemetryAccepted", func(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + stExtension := &extensions.Extension{ + Id: "azd.internal.telemetry", + Capabilities: []extensions.CapabilityType{ + extensions.ServiceTargetProviderCapability, + }, + Namespace: "test", + } + accessToken, err := GenerateExtensionToken(stExtension, serverInfo) + require.NoError(t, err) + + ctx := azdext.WithAccessToken(t.Context(), accessToken) + client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) + require.NoError(t, err) + + // The server runs in-process, so the handler writes to this scope. + scope := tracing.BeginCommandUsageScope("cmd.deploy") + + resp, err := client.Telemetry().AddCommandUsageAttribute(ctx, &azdext.AddCommandUsageAttributeRequest{ + Key: telemetry.AgentDeploymentModeAttribute, + Value: string(telemetry.AgentDeploymentModeCode), + }) + require.NoError(t, err) + require.True(t, resp.Accepted) + + attrs, err := tracing.CloseCommandUsageScope(scope) + require.NoError(t, err) + require.Len(t, attrs, 1) + require.Equal(t, []string{"code"}, attrs[0].Value.AsStringSlice()) + }) + + t.Run("TelemetryMissingCapability", func(t *testing.T) { + // The base extension only declares CustomCommandCapability. + accessToken, err := GenerateExtensionToken(extension, serverInfo) + require.NoError(t, err) + + ctx := azdext.WithAccessToken(t.Context(), accessToken) + client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) + require.NoError(t, err) + + _, err = client.Telemetry().AddCommandUsageAttribute(ctx, &azdext.AddCommandUsageAttributeRequest{ + Key: telemetry.AgentDeploymentModeAttribute, + Value: string(telemetry.AgentDeploymentModeCode), + }) + st, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, codes.PermissionDenied, st.Code()) + }) + + t.Run("TelemetryMissingToken", func(t *testing.T) { + client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) + require.NoError(t, err) + + _, err = client.Telemetry().AddCommandUsageAttribute( + t.Context(), + &azdext.AddCommandUsageAttributeRequest{ + Key: telemetry.AgentDeploymentModeAttribute, + Value: string(telemetry.AgentDeploymentModeCode), + }, + ) + st, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, codes.Unauthenticated, st.Code()) + }) } // Test_Server_StreamInterceptor validates that the streaming RPC interceptor @@ -142,6 +213,7 @@ func Test_Server_StreamInterceptor(t *testing.T) { azdext.UnimplementedCopilotServiceServer{}, azdext.UnimplementedProvisioningServiceServer{}, azdext.UnimplementedValidationServiceServer{}, + azdext.UnimplementedTelemetryServiceServer{}, ) serverInfo, err := server.Start() @@ -618,7 +690,7 @@ func TestValidateAuthToken_InvalidToken(t *testing.T) { func TestNewServer(t *testing.T) { t.Parallel() - s := NewServer(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + s := NewServer(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) require.NotNil(t, s) assert.Nil(t, s.grpcServer, "grpcServer should be nil before Start") } diff --git a/cli/azd/internal/grpcserver/telemetry_service.go b/cli/azd/internal/grpcserver/telemetry_service.go new file mode 100644 index 00000000000..e96499c223b --- /dev/null +++ b/cli/azd/internal/grpcserver/telemetry_service.go @@ -0,0 +1,131 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package grpcserver + +import ( + "context" + "slices" + + "github.com/azure/azure-dev/cli/azd/internal/tracing" + "github.com/azure/azure-dev/cli/azd/internal/tracing/events" + "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" + "github.com/azure/azure-dev/cli/azd/pkg/extensions" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// maxTelemetryFieldLength bounds the accepted key and value length. This is a +// defensive limit against oversized input, not a privacy control. The value +// allowlist is the privacy control. +const maxTelemetryFieldLength = 128 + +// commandUsageFieldPolicy is the host-owned policy for one extension-settable +// command usage attribute. +type commandUsageFieldPolicy struct { + key fields.AttributeKey + allowedValues map[string]struct{} + eligibleEvents map[string]struct{} + requiredCapabilities map[extensions.CapabilityType]struct{} +} + +// extensionUsageFields is the allowlist of command usage attributes an +// authenticated extension may contribute. The host owns every field's key, +// classification, allowed values, eligible command events, and required +// capabilities. No extension identifier is hardcoded: eligibility is expressed +// through signed extension capabilities and the fixed value set. +var extensionUsageFields = map[string]commandUsageFieldPolicy{ + telemetry.AgentDeploymentModeAttribute: { + key: fields.AgentDeploymentModeKey, + allowedValues: map[string]struct{}{ + string(telemetry.AgentDeploymentModeCode): {}, + string(telemetry.AgentDeploymentModeContainer): {}, + string(telemetry.AgentDeploymentModeByoImage): {}, + }, + eligibleEvents: map[string]struct{}{ + events.GetCommandEventName("azd deploy"): {}, + events.GetCommandEventName("azd up"): {}, + }, + requiredCapabilities: map[extensions.CapabilityType]struct{}{ + extensions.ServiceTargetProviderCapability: {}, + }, + }, +} + +// telemetryService implements azdext.TelemetryServiceServer. +type telemetryService struct { + azdext.UnimplementedTelemetryServiceServer + fields map[string]commandUsageFieldPolicy +} + +// NewTelemetryService creates the telemetry gRPC service. It holds no injected +// state; the handler reaches the process-global command usage store through the +// tracing package. Returning the interface type lets the IoC container satisfy +// the azdext.TelemetryServiceServer parameter on NewServer without an adapter. +func NewTelemetryService() azdext.TelemetryServiceServer { + return newTelemetryService(extensionUsageFields) +} + +func newTelemetryService(fieldPolicies map[string]commandUsageFieldPolicy) *telemetryService { + return &telemetryService{fields: fieldPolicies} +} + +// AddCommandUsageAttribute validates an authenticated extension's request +// against the host allowlist and, when valid, appends the value to the current +// command usage scope. It fails closed: unknown keys, missing capabilities, +// invalid policies, and disallowed values are all rejected before anything is +// recorded. Rejected caller text is never echoed into the returned error. +func (s *telemetryService) AddCommandUsageAttribute( + ctx context.Context, + req *azdext.AddCommandUsageAttributeRequest, +) (*azdext.AddCommandUsageAttributeResponse, error) { + claims, err := extensions.GetClaimsFromContext(ctx) + if err != nil { + return nil, status.Error(codes.Unauthenticated, "validated extension claims are required") + } + + if req == nil || + req.Key == "" || req.Value == "" || + len(req.Key) > maxTelemetryFieldLength || len(req.Value) > maxTelemetryFieldLength { + return nil, status.Error(codes.InvalidArgument, "telemetry key and value are required") + } + + policy, ok := s.fields[req.Key] + if !ok { + return nil, status.Error(codes.InvalidArgument, "telemetry key is not registered") + } + + if !hasRequiredCapability(claims, policy.requiredCapabilities) { + return nil, status.Error(codes.PermissionDenied, "extension lacks the required capability") + } + + if len(policy.allowedValues) == 0 || len(policy.eligibleEvents) == 0 { + return nil, status.Error(codes.Internal, "telemetry field policy is invalid") + } + + if _, ok := policy.allowedValues[req.Value]; !ok { + return nil, status.Error(codes.InvalidArgument, "telemetry value is not allowed") + } + + accepted := tracing.TryAppendCommandUsageUnique(policy.eligibleEvents, policy.key.Key, req.Value) + + return &azdext.AddCommandUsageAttributeResponse{Accepted: accepted}, nil +} + +// hasRequiredCapability reports whether the signed extension claims carry every +// required capability. Capabilities are part of the host-signed token, so they +// are trustworthy without a separate manager lookup. +func hasRequiredCapability( + claims *extensions.ExtensionClaims, + required map[extensions.CapabilityType]struct{}, +) bool { + for capability := range required { + if !slices.Contains(claims.Capabilities, capability) { + return false + } + } + + return true +} diff --git a/cli/azd/internal/grpcserver/telemetry_service_test.go b/cli/azd/internal/grpcserver/telemetry_service_test.go new file mode 100644 index 00000000000..abea91e2b6c --- /dev/null +++ b/cli/azd/internal/grpcserver/telemetry_service_test.go @@ -0,0 +1,250 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package grpcserver + +import ( + "context" + "strings" + "sync" + "testing" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/azure/azure-dev/cli/azd/internal/tracing" + "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" + "github.com/azure/azure-dev/cli/azd/pkg/extensions" +) + +func claimsContext(caps ...extensions.CapabilityType) context.Context { + claims := &extensions.ExtensionClaims{ + RegisteredClaims: jwt.RegisteredClaims{Subject: "test.extension"}, + Capabilities: caps, + } + return extensions.WithClaimsContext(context.Background(), claims) +} + +func serviceTargetClaims() context.Context { + return claimsContext(extensions.ServiceTargetProviderCapability) +} + +func validRequest(value string) *azdext.AddCommandUsageAttributeRequest { + return &azdext.AddCommandUsageAttributeRequest{ + Key: telemetry.AgentDeploymentModeAttribute, + Value: value, + } +} + +func TestTelemetryService_MissingClaims(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + svc := NewTelemetryService() + _, err := svc.AddCommandUsageAttribute(context.Background(), validRequest("code")) + require.Equal(t, codes.Unauthenticated, status.Code(err)) +} + +func TestTelemetryService_MissingCapability(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + scope := tracing.BeginCommandUsageScope("cmd.deploy") + t.Cleanup(func() { _, _ = tracing.CloseCommandUsageScope(scope) }) + + svc := NewTelemetryService() + // Authenticated but without the service-target-provider capability. + _, err := svc.AddCommandUsageAttribute(claimsContext(), validRequest("code")) + require.Equal(t, codes.PermissionDenied, status.Code(err)) +} + +func TestTelemetryService_InvalidArguments(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + svc := NewTelemetryService() + ctx := serviceTargetClaims() + + cases := map[string]*azdext.AddCommandUsageAttributeRequest{ + "nil request": nil, + "empty key": {Key: "", Value: "code"}, + "empty value": {Key: telemetry.AgentDeploymentModeAttribute, Value: ""}, + "oversize key": {Key: strings.Repeat("k", maxTelemetryFieldLength+1), Value: "code"}, + "oversize value": { + Key: telemetry.AgentDeploymentModeAttribute, + Value: strings.Repeat("v", maxTelemetryFieldLength+1), + }, + "unknown key": {Key: "some.other.key", Value: "code"}, + "invalid enum": validRequest("bogus"), + } + + for name, req := range cases { + t.Run(name, func(t *testing.T) { + _, err := svc.AddCommandUsageAttribute(ctx, req) + require.Equal(t, codes.InvalidArgument, status.Code(err), "case %q", name) + }) + } +} + +func TestTelemetryService_InvalidPolicyIsInternal(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + ctx := serviceTargetClaims() + capSet := map[extensions.CapabilityType]struct{}{ + extensions.ServiceTargetProviderCapability: {}, + } + + t.Run("empty allowed values", func(t *testing.T) { + svc := newTelemetryService(map[string]commandUsageFieldPolicy{ + telemetry.AgentDeploymentModeAttribute: { + key: fields.AgentDeploymentModeKey, + allowedValues: map[string]struct{}{}, + eligibleEvents: map[string]struct{}{"cmd.deploy": {}}, + requiredCapabilities: capSet, + }, + }) + _, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) + require.Equal(t, codes.Internal, status.Code(err)) + }) + + t.Run("empty eligible events", func(t *testing.T) { + svc := newTelemetryService(map[string]commandUsageFieldPolicy{ + telemetry.AgentDeploymentModeAttribute: { + key: fields.AgentDeploymentModeKey, + allowedValues: map[string]struct{}{"code": {}}, + eligibleEvents: map[string]struct{}{}, + requiredCapabilities: capSet, + }, + }) + _, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) + require.Equal(t, codes.Internal, status.Code(err)) + }) +} + +func TestTelemetryService_EligibleScopesAccept(t *testing.T) { + for _, eventName := range []string{"cmd.deploy", "cmd.up"} { + t.Run(eventName, func(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + scope := tracing.BeginCommandUsageScope(eventName) + svc := NewTelemetryService() + + resp, err := svc.AddCommandUsageAttribute(serviceTargetClaims(), validRequest("code")) + require.NoError(t, err) + require.True(t, resp.Accepted) + + attrs, err := tracing.CloseCommandUsageScope(scope) + require.NoError(t, err) + require.Len(t, attrs, 1) + require.Equal(t, telemetry.AgentDeploymentModeAttribute, string(attrs[0].Key)) + require.Equal(t, []string{"code"}, attrs[0].Value.AsStringSlice()) + }) + } +} + +func TestTelemetryService_IneligibleScopeNotAccepted(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + scope := tracing.BeginCommandUsageScope("cmd.package") + svc := NewTelemetryService() + + resp, err := svc.AddCommandUsageAttribute(serviceTargetClaims(), validRequest("code")) + require.NoError(t, err) + require.False(t, resp.Accepted) + + attrs, err := tracing.CloseCommandUsageScope(scope) + require.NoError(t, err) + require.Empty(t, attrs) +} + +func TestTelemetryService_NoActiveScopeNotAccepted(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + svc := NewTelemetryService() + resp, err := svc.AddCommandUsageAttribute(serviceTargetClaims(), validRequest("code")) + require.NoError(t, err) + require.False(t, resp.Accepted) +} + +func TestTelemetryService_DuplicateCollapses(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + scope := tracing.BeginCommandUsageScope("cmd.deploy") + svc := NewTelemetryService() + ctx := serviceTargetClaims() + + for i := 0; i < 3; i++ { + resp, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) + require.NoError(t, err) + require.True(t, resp.Accepted) + } + + attrs, err := tracing.CloseCommandUsageScope(scope) + require.NoError(t, err) + require.Len(t, attrs, 1) + require.Equal(t, []string{"code"}, attrs[0].Value.AsStringSlice()) +} + +func TestTelemetryService_ConcurrentReports(t *testing.T) { + tracing.ResetCommandUsageForTest() + t.Cleanup(tracing.ResetCommandUsageForTest) + + scope := tracing.BeginCommandUsageScope("cmd.up") + svc := NewTelemetryService() + ctx := serviceTargetClaims() + + modes := []string{"code", "container", "byo_image"} + var wg sync.WaitGroup + for i := 0; i < 60; i++ { + value := modes[i%len(modes)] + wg.Add(1) + go func() { + defer wg.Done() + _, err := svc.AddCommandUsageAttribute(ctx, validRequest(value)) + require.NoError(t, err) + }() + } + wg.Wait() + + attrs, err := tracing.CloseCommandUsageScope(scope) + require.NoError(t, err) + require.Len(t, attrs, 1) + require.ElementsMatch(t, modes, attrs[0].Value.AsStringSlice()) +} + +// TestExtensionUsageFieldsInvariants guards the production allowlist so a future +// field cannot accidentally open a free-form or unauthenticated path. +func TestExtensionUsageFieldsInvariants(t *testing.T) { + t.Parallel() + + require.NotEmpty(t, extensionUsageFields) + + for registryKey, policy := range extensionUsageFields { + require.NotEmpty(t, registryKey) + require.Equal(t, registryKey, string(policy.key.Key), + "policy key must match its registry key") + require.NotEmpty(t, string(policy.key.Classification), "classification must be set") + require.NotEmpty(t, string(policy.key.Purpose), "purpose must be set") + + require.NotEmpty(t, policy.allowedValues, "allowed values must be set") + for value := range policy.allowedValues { + require.NotEmpty(t, value, "allowed value must not be empty") + } + + require.NotEmpty(t, policy.eligibleEvents, "eligible events must be set") + for event := range policy.eligibleEvents { + require.NotEmpty(t, event, "eligible event must not be empty") + } + + require.NotEmpty(t, policy.requiredCapabilities, "required capabilities must be set") + } +} diff --git a/cli/azd/internal/tracing/command_usage.go b/cli/azd/internal/tracing/command_usage.go new file mode 100644 index 00000000000..67b4bbfe33c --- /dev/null +++ b/cli/azd/internal/tracing/command_usage.go @@ -0,0 +1,147 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +import ( + "fmt" + "slices" + "strings" + "sync" + + "go.opentelemetry.io/otel/attribute" +) + +// commandUsage is the process-global command usage scope stack. It mirrors the +// package-global usageVal store: a single process-wide instance reachable from +// the command middleware and from any gRPC handler goroutine without dependency +// injection. Extension-contributed command usage attributes are held here and +// attached to the owning command span when its scope closes. +var commandUsage = commandUsageStore{} + +type commandUsageStore struct { + mu sync.Mutex + nextID uint64 + scopes []*commandUsageScope +} + +type commandUsageScope struct { + id uint64 + eventName string + values map[attribute.Key]map[string]struct{} +} + +// CommandUsageScope is an opaque handle to a pushed command usage scope. The +// command middleware holds it between BeginCommandUsageScope and +// CloseCommandUsageScope. +type CommandUsageScope struct { + id uint64 +} + +// BeginCommandUsageScope pushes a new command usage scope for eventName and +// returns its handle. Every command telemetry middleware invocation begins +// exactly one scope, including nested child actions, so the top of the stack +// always identifies the command that owns any values reported while it runs. +func BeginCommandUsageScope(eventName string) CommandUsageScope { + commandUsage.mu.Lock() + defer commandUsage.mu.Unlock() + + commandUsage.nextID++ + id := commandUsage.nextID + commandUsage.scopes = append(commandUsage.scopes, &commandUsageScope{ + id: id, + eventName: eventName, + values: map[attribute.Key]map[string]struct{}{}, + }) + + return CommandUsageScope{id: id} +} + +// TryAppendCommandUsageUnique appends value to the current (top) scope when its +// exact event name is present in eligibleEvents. It returns false when no scope +// is active or the current scope is not eligible. It never falls back to an +// ancestor scope, so a value reported while an ineligible command (for example +// a synthetic child cmd.package) is on top is discarded rather than attributed +// to a parent command. +func TryAppendCommandUsageUnique( + eligibleEvents map[string]struct{}, + key attribute.Key, + value string, +) bool { + commandUsage.mu.Lock() + defer commandUsage.mu.Unlock() + + if len(commandUsage.scopes) == 0 { + return false + } + + top := commandUsage.scopes[len(commandUsage.scopes)-1] + if _, ok := eligibleEvents[top.eventName]; !ok { + return false + } + + set, ok := top.values[key] + if !ok { + set = map[string]struct{}{} + top.values[key] = set + } + set[value] = struct{}{} + + return true +} + +// CloseCommandUsageScope pops the scope identified by scope and returns its +// accumulated attributes as deterministically sorted string slices. It returns +// an error when the handle does not identify the current top scope, which +// covers a repeated or out-of-order close. On error the stack is left +// unchanged so a balanced close by the true owner still succeeds. +func CloseCommandUsageScope(scope CommandUsageScope) ([]attribute.KeyValue, error) { + commandUsage.mu.Lock() + defer commandUsage.mu.Unlock() + + n := len(commandUsage.scopes) + if n == 0 { + return nil, fmt.Errorf("no active command usage scope to close") + } + + top := commandUsage.scopes[n-1] + if top.id != scope.id { + return nil, fmt.Errorf( + "command usage scope is not the current scope; expected %d, got %d", + top.id, scope.id, + ) + } + + commandUsage.scopes = commandUsage.scopes[:n-1] + + keys := make([]attribute.Key, 0, len(top.values)) + for k := range top.values { + keys = append(keys, k) + } + slices.SortFunc(keys, func(a, b attribute.Key) int { + return strings.Compare(string(a), string(b)) + }) + + attrs := make([]attribute.KeyValue, 0, len(keys)) + for _, k := range keys { + valuesSet := top.values[k] + values := make([]string, 0, len(valuesSet)) + for v := range valuesSet { + values = append(values, v) + } + slices.Sort(values) + attrs = append(attrs, k.StringSlice(values)) + } + + return attrs, nil +} + +// ResetCommandUsageForTest clears all command usage scopes. Command usage state +// is process-global; tests that rely on it must not run with t.Parallel(). +func ResetCommandUsageForTest() { + commandUsage.mu.Lock() + defer commandUsage.mu.Unlock() + + commandUsage.scopes = nil + commandUsage.nextID = 0 +} diff --git a/cli/azd/internal/tracing/command_usage_test.go b/cli/azd/internal/tracing/command_usage_test.go new file mode 100644 index 00000000000..195bf71e149 --- /dev/null +++ b/cli/azd/internal/tracing/command_usage_test.go @@ -0,0 +1,222 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package tracing + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" +) + +const testUsageKey = attribute.Key("agent.deploy.mode") + +func testEligibleEvents() map[string]struct{} { + return map[string]struct{}{ + "cmd.deploy": {}, + "cmd.up": {}, + } +} + +// closeValues closes the scope and returns the string-slice value for the +// telemetry key, or nil when the key was not recorded. +func closeValues(t *testing.T, scope CommandUsageScope) []string { + t.Helper() + + attrs, err := CloseCommandUsageScope(scope) + require.NoError(t, err) + + for _, attr := range attrs { + if attr.Key == testUsageKey { + return attr.Value.AsStringSlice() + } + } + + return nil +} + +func TestCommandUsage_NoActiveScope(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + ok := TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code") + require.False(t, ok) +} + +func TestCommandUsage_EligibleDeployScope(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.deploy") + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + require.Equal(t, []string{"code"}, closeValues(t, scope)) +} + +func TestCommandUsage_EligibleUpScope(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.up") + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "container")) + require.Equal(t, []string{"container"}, closeValues(t, scope)) +} + +func TestCommandUsage_DuplicateValueCollapses(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.deploy") + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + require.Equal(t, []string{"code"}, closeValues(t, scope)) +} + +func TestCommandUsage_TwoValuesSortedUnique(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.up") + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "container")) + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + // Sorted regardless of insertion order. + require.Equal(t, []string{"code", "container"}, closeValues(t, scope)) +} + +func TestCommandUsage_ConcurrentValues(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.up") + + modes := []string{"code", "container", "byo_image"} + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + mode := modes[i%len(modes)] + wg.Add(1) + go func() { + defer wg.Done() + TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, mode) + }() + } + wg.Wait() + + require.ElementsMatch(t, modes, closeValues(t, scope)) +} + +func TestCommandUsage_IneligiblePackageScope(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.package") + require.False(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + require.Nil(t, closeValues(t, scope)) +} + +func TestCommandUsage_NestedUpThenPackageNoFallback(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + up := BeginCommandUsageScope("cmd.up") + pkg := BeginCommandUsageScope("cmd.package") + + // A report while the ineligible child is on top must not fall back to up. + require.False(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + + require.Nil(t, closeValues(t, pkg)) + require.Nil(t, closeValues(t, up)) +} + +func TestCommandUsage_NestedUpThenDeployOwnsValue(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + up := BeginCommandUsageScope("cmd.up") + deploy := BeginCommandUsageScope("cmd.deploy") + + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "byo_image")) + + // The value lands only on the child deploy, never the parent up. + require.Equal(t, []string{"byo_image"}, closeValues(t, deploy)) + require.Nil(t, closeValues(t, up)) +} + +func TestCommandUsage_ChildCloseRestoresParentAsCurrent(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + up := BeginCommandUsageScope("cmd.up") + deploy := BeginCommandUsageScope("cmd.deploy") + _, err := CloseCommandUsageScope(deploy) + require.NoError(t, err) + + // After the child closes, up is current again and eligible. + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + require.Equal(t, []string{"code"}, closeValues(t, up)) +} + +func TestCommandUsage_CloseAfterActionErrorRemovesScope(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + // Simulate a command action that errored: the deferred close still runs. + scope := BeginCommandUsageScope("cmd.deploy") + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + _, err := CloseCommandUsageScope(scope) + require.NoError(t, err) + + // The next command sees no active scope. + require.False(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) +} + +func TestCommandUsage_RepeatedCloseErrors(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + scope := BeginCommandUsageScope("cmd.deploy") + _, err := CloseCommandUsageScope(scope) + require.NoError(t, err) + + _, err = CloseCommandUsageScope(scope) + require.Error(t, err) +} + +func TestCommandUsage_OutOfOrderCloseErrorsAndPreservesState(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + up := BeginCommandUsageScope("cmd.up") + deploy := BeginCommandUsageScope("cmd.deploy") + + // Closing the parent while the child is on top is rejected. + _, err := CloseCommandUsageScope(up) + require.Error(t, err) + + // State is intact: the child is still current and closes cleanly, then up. + _, err = CloseCommandUsageScope(deploy) + require.NoError(t, err) + _, err = CloseCommandUsageScope(up) + require.NoError(t, err) +} + +func TestCommandUsage_NextCommandHasNoLeakedValue(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + first := BeginCommandUsageScope("cmd.deploy") + require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) + require.Equal(t, []string{"code"}, closeValues(t, first)) + + // A subsequent unrelated command starts empty. + second := BeginCommandUsageScope("cmd.deploy") + require.Nil(t, closeValues(t, second)) +} + +func TestCommandUsage_CloseWithNoScopeErrors(t *testing.T) { + ResetCommandUsageForTest() + t.Cleanup(ResetCommandUsageForTest) + + _, err := CloseCommandUsageScope(CommandUsageScope{id: 999}) + require.Error(t, err) +} diff --git a/cli/azd/internal/tracing/fields/fields.go b/cli/azd/internal/tracing/fields/fields.go index de360f23974..7da1ce090e6 100644 --- a/cli/azd/internal/tracing/fields/fields.go +++ b/cli/azd/internal/tracing/fields/fields.go @@ -8,6 +8,8 @@ import ( "github.com/microsoft/ApplicationInsights-Go/appinsights/contracts" "go.opentelemetry.io/otel/attribute" semconv "go.opentelemetry.io/otel/semconv/v1.39.0" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" ) // AttributeKey represents an attribute key with additional metadata. @@ -261,6 +263,17 @@ var ( // Deployment attributes var ( + // AgentDeploymentModeKey records the de-duplicated set of hosted agent + // deployment modes selected during a command. Values are a fixed enum + // (code, container, byo_image) contributed by an authenticated extension + // through the host-validated telemetry service, so the field never carries + // user content, resource names, image references, paths, or identifiers. + AgentDeploymentModeKey = AttributeKey{ + Key: attribute.Key(telemetry.AgentDeploymentModeAttribute), + Classification: SystemMetadata, + Purpose: FeatureInsight, + } + // DeployAttemptKey tracks the retry attempt number for App Service zip deployments. DeployAttemptKey = AttributeKey{ Key: attribute.Key("deploy.appservice.attempt"), diff --git a/cli/azd/pkg/azdext/artifact_metadata.go b/cli/azd/pkg/azdext/artifact_metadata.go new file mode 100644 index 00000000000..55a09245054 --- /dev/null +++ b/cli/azd/pkg/azdext/artifact_metadata.go @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azdext + +// ArtifactMetadataKeyFromPackage marks an artifact that was supplied through +// the --from-package option rather than produced by azd. This is deployment +// payload provenance, not telemetry. +const ArtifactMetadataKeyFromPackage = "azd.fromPackage" diff --git a/cli/azd/pkg/azdext/azd_client.go b/cli/azd/pkg/azdext/azd_client.go index a2f6fb9031b..21907a09257 100644 --- a/cli/azd/pkg/azdext/azd_client.go +++ b/cli/azd/pkg/azdext/azd_client.go @@ -262,3 +262,14 @@ func (c *AzdClient) Validation() ValidationServiceClient { return c.validationClient } + +// Telemetry returns the telemetry service client used to contribute +// host-validated command usage attributes. +// +// A fresh client is returned on each call rather than caching it on the +// AzdClient struct. Service target providers can deploy services concurrently, +// so an unsynchronized lazily-written cache field could race on first use. The +// generated client wrapper is cheap and shares the existing connection. +func (c *AzdClient) Telemetry() TelemetryServiceClient { + return NewTelemetryServiceClient(c.connection) +} diff --git a/cli/azd/pkg/azdext/telemetry.pb.go b/cli/azd/pkg/azdext/telemetry.pb.go new file mode 100644 index 00000000000..c2a2c3a87aa --- /dev/null +++ b/cli/azd/pkg/azdext/telemetry.pb.go @@ -0,0 +1,189 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.10 +// protoc v7.35.0 +// source: telemetry.proto + +package azdext + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" + unsafe "unsafe" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +type AddCommandUsageAttributeRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Key must be registered and owned by the host. + Key string `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"` + // Value must be in the host-owned allowed set for key. + Value string `protobuf:"bytes,2,opt,name=value,proto3" json:"value,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *AddCommandUsageAttributeRequest) Reset() { + *x = AddCommandUsageAttributeRequest{} + mi := &file_telemetry_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *AddCommandUsageAttributeRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*AddCommandUsageAttributeRequest) ProtoMessage() {} + +func (x *AddCommandUsageAttributeRequest) ProtoReflect() protoreflect.Message { + mi := &file_telemetry_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use AddCommandUsageAttributeRequest.ProtoReflect.Descriptor instead. +func (*AddCommandUsageAttributeRequest) Descriptor() ([]byte, []int) { + return file_telemetry_proto_rawDescGZIP(), []int{0} +} + +func (x *AddCommandUsageAttributeRequest) GetKey() string { + if x != nil { + return x.Key + } + return "" +} + +func (x *AddCommandUsageAttributeRequest) GetValue() string { + if x != nil { + return x.Value + } + return "" +} + +type AddCommandUsageAttributeResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Accepted is false when the value was rejected because no eligible command + // scope is currently active. A false response is not an error. + Accepted bool `protobuf:"varint,1,opt,name=accepted,proto3" json:"accepted,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *AddCommandUsageAttributeResponse) Reset() { + *x = AddCommandUsageAttributeResponse{} + mi := &file_telemetry_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *AddCommandUsageAttributeResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*AddCommandUsageAttributeResponse) ProtoMessage() {} + +func (x *AddCommandUsageAttributeResponse) ProtoReflect() protoreflect.Message { + mi := &file_telemetry_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use AddCommandUsageAttributeResponse.ProtoReflect.Descriptor instead. +func (*AddCommandUsageAttributeResponse) Descriptor() ([]byte, []int) { + return file_telemetry_proto_rawDescGZIP(), []int{1} +} + +func (x *AddCommandUsageAttributeResponse) GetAccepted() bool { + if x != nil { + return x.Accepted + } + return false +} + +var File_telemetry_proto protoreflect.FileDescriptor + +const file_telemetry_proto_rawDesc = "" + + "\n" + + "\x0ftelemetry.proto\x12\x06azdext\"I\n" + + "\x1fAddCommandUsageAttributeRequest\x12\x10\n" + + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + + "\x05value\x18\x02 \x01(\tR\x05value\">\n" + + " AddCommandUsageAttributeResponse\x12\x1a\n" + + "\baccepted\x18\x01 \x01(\bR\baccepted2\x81\x01\n" + + "\x10TelemetryService\x12m\n" + + "\x18AddCommandUsageAttribute\x12'.azdext.AddCommandUsageAttributeRequest\x1a(.azdext.AddCommandUsageAttributeResponseB/Z-github.com/azure/azure-dev/cli/azd/pkg/azdextb\x06proto3" + +var ( + file_telemetry_proto_rawDescOnce sync.Once + file_telemetry_proto_rawDescData []byte +) + +func file_telemetry_proto_rawDescGZIP() []byte { + file_telemetry_proto_rawDescOnce.Do(func() { + file_telemetry_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_telemetry_proto_rawDesc), len(file_telemetry_proto_rawDesc))) + }) + return file_telemetry_proto_rawDescData +} + +var file_telemetry_proto_msgTypes = make([]protoimpl.MessageInfo, 2) +var file_telemetry_proto_goTypes = []any{ + (*AddCommandUsageAttributeRequest)(nil), // 0: azdext.AddCommandUsageAttributeRequest + (*AddCommandUsageAttributeResponse)(nil), // 1: azdext.AddCommandUsageAttributeResponse +} +var file_telemetry_proto_depIdxs = []int32{ + 0, // 0: azdext.TelemetryService.AddCommandUsageAttribute:input_type -> azdext.AddCommandUsageAttributeRequest + 1, // 1: azdext.TelemetryService.AddCommandUsageAttribute:output_type -> azdext.AddCommandUsageAttributeResponse + 1, // [1:2] is the sub-list for method output_type + 0, // [0:1] is the sub-list for method input_type + 0, // [0:0] is the sub-list for extension type_name + 0, // [0:0] is the sub-list for extension extendee + 0, // [0:0] is the sub-list for field type_name +} + +func init() { file_telemetry_proto_init() } +func file_telemetry_proto_init() { + if File_telemetry_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_telemetry_proto_rawDesc), len(file_telemetry_proto_rawDesc)), + NumEnums: 0, + NumMessages: 2, + NumExtensions: 0, + NumServices: 1, + }, + GoTypes: file_telemetry_proto_goTypes, + DependencyIndexes: file_telemetry_proto_depIdxs, + MessageInfos: file_telemetry_proto_msgTypes, + }.Build() + File_telemetry_proto = out.File + file_telemetry_proto_goTypes = nil + file_telemetry_proto_depIdxs = nil +} diff --git a/cli/azd/pkg/azdext/telemetry/attributes.go b/cli/azd/pkg/azdext/telemetry/attributes.go new file mode 100644 index 00000000000..47f748d8895 --- /dev/null +++ b/cli/azd/pkg/azdext/telemetry/attributes.go @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// Package telemetry defines the extension telemetry contract values shared by +// the azd host and extensions. Keeping the key and the allowed enum values in +// one dependency-free package prevents core and extension code from duplicating +// telemetry string literals. +package telemetry + +// AgentDeploymentModeAttribute is the command-level usage attribute that +// records the set of hosted agent deployment modes selected during a command. +// Its value is a de-duplicated string slice. +const AgentDeploymentModeAttribute = "agent.deploy.mode" + +// AgentDeploymentMode identifies the selected hosted agent deployment source. +type AgentDeploymentMode string + +const ( + // AgentDeploymentModeCode deploys a source archive. + AgentDeploymentModeCode AgentDeploymentMode = "code" + + // AgentDeploymentModeContainer deploys a container image that azd builds + // and publishes from the project source. + AgentDeploymentModeContainer AgentDeploymentMode = "container" + + // AgentDeploymentModeByoImage deploys a caller-supplied image. + AgentDeploymentModeByoImage AgentDeploymentMode = "byo_image" +) diff --git a/cli/azd/pkg/azdext/telemetry/attributes_test.go b/cli/azd/pkg/azdext/telemetry/attributes_test.go new file mode 100644 index 00000000000..e0ca411e0ab --- /dev/null +++ b/cli/azd/pkg/azdext/telemetry/attributes_test.go @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package telemetry + +import "testing" + +// TestContractValues locks the public wire values. Downstream telemetry +// pipelines and the host allowlist depend on these exact strings, so a change +// here is a breaking telemetry contract change and must be intentional. +func TestContractValues(t *testing.T) { + t.Parallel() + + if AgentDeploymentModeAttribute != "agent.deploy.mode" { + t.Fatalf("unexpected attribute key: %q", AgentDeploymentModeAttribute) + } + + cases := map[AgentDeploymentMode]string{ + AgentDeploymentModeCode: "code", + AgentDeploymentModeContainer: "container", + AgentDeploymentModeByoImage: "byo_image", + } + for mode, want := range cases { + if string(mode) != want { + t.Errorf("unexpected mode value: got %q, want %q", string(mode), want) + } + } +} diff --git a/cli/azd/pkg/azdext/telemetry_grpc.pb.go b/cli/azd/pkg/azdext/telemetry_grpc.pb.go new file mode 100644 index 00000000000..d93398ee252 --- /dev/null +++ b/cli/azd/pkg/azdext/telemetry_grpc.pb.go @@ -0,0 +1,140 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// Code generated by protoc-gen-go-grpc. DO NOT EDIT. +// versions: +// - protoc-gen-go-grpc v1.5.1 +// - protoc v7.35.0 +// source: telemetry.proto + +package azdext + +import ( + context "context" + grpc "google.golang.org/grpc" + codes "google.golang.org/grpc/codes" + status "google.golang.org/grpc/status" +) + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the grpc package it is being compiled against. +// Requires gRPC-Go v1.64.0 or later. +const _ = grpc.SupportPackageIsVersion9 + +const ( + TelemetryService_AddCommandUsageAttribute_FullMethodName = "/azdext.TelemetryService/AddCommandUsageAttribute" +) + +// TelemetryServiceClient is the client API for TelemetryService service. +// +// For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. +// +// TelemetryService accepts reviewed, host-owned command usage attribute values +// from authenticated extensions. The host validates the key and value against +// an allowlist and attaches accepted values to the current command telemetry +// event. Extensions cannot choose classification, purpose, hashing, or +// aggregation. +type TelemetryServiceClient interface { + // AddCommandUsageAttribute adds one bounded value to a host-owned command + // usage attribute. Duplicate values are collapsed by the host. + AddCommandUsageAttribute(ctx context.Context, in *AddCommandUsageAttributeRequest, opts ...grpc.CallOption) (*AddCommandUsageAttributeResponse, error) +} + +type telemetryServiceClient struct { + cc grpc.ClientConnInterface +} + +func NewTelemetryServiceClient(cc grpc.ClientConnInterface) TelemetryServiceClient { + return &telemetryServiceClient{cc} +} + +func (c *telemetryServiceClient) AddCommandUsageAttribute(ctx context.Context, in *AddCommandUsageAttributeRequest, opts ...grpc.CallOption) (*AddCommandUsageAttributeResponse, error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + out := new(AddCommandUsageAttributeResponse) + err := c.cc.Invoke(ctx, TelemetryService_AddCommandUsageAttribute_FullMethodName, in, out, cOpts...) + if err != nil { + return nil, err + } + return out, nil +} + +// TelemetryServiceServer is the server API for TelemetryService service. +// All implementations must embed UnimplementedTelemetryServiceServer +// for forward compatibility. +// +// TelemetryService accepts reviewed, host-owned command usage attribute values +// from authenticated extensions. The host validates the key and value against +// an allowlist and attaches accepted values to the current command telemetry +// event. Extensions cannot choose classification, purpose, hashing, or +// aggregation. +type TelemetryServiceServer interface { + // AddCommandUsageAttribute adds one bounded value to a host-owned command + // usage attribute. Duplicate values are collapsed by the host. + AddCommandUsageAttribute(context.Context, *AddCommandUsageAttributeRequest) (*AddCommandUsageAttributeResponse, error) + mustEmbedUnimplementedTelemetryServiceServer() +} + +// UnimplementedTelemetryServiceServer must be embedded to have +// forward compatible implementations. +// +// NOTE: this should be embedded by value instead of pointer to avoid a nil +// pointer dereference when methods are called. +type UnimplementedTelemetryServiceServer struct{} + +func (UnimplementedTelemetryServiceServer) AddCommandUsageAttribute(context.Context, *AddCommandUsageAttributeRequest) (*AddCommandUsageAttributeResponse, error) { + return nil, status.Errorf(codes.Unimplemented, "method AddCommandUsageAttribute not implemented") +} +func (UnimplementedTelemetryServiceServer) mustEmbedUnimplementedTelemetryServiceServer() {} +func (UnimplementedTelemetryServiceServer) testEmbeddedByValue() {} + +// UnsafeTelemetryServiceServer may be embedded to opt out of forward compatibility for this service. +// Use of this interface is not recommended, as added methods to TelemetryServiceServer will +// result in compilation errors. +type UnsafeTelemetryServiceServer interface { + mustEmbedUnimplementedTelemetryServiceServer() +} + +func RegisterTelemetryServiceServer(s grpc.ServiceRegistrar, srv TelemetryServiceServer) { + // If the following call pancis, it indicates UnimplementedTelemetryServiceServer was + // embedded by pointer and is nil. This will cause panics if an + // unimplemented method is ever invoked, so we test this at initialization + // time to prevent it from happening at runtime later due to I/O. + if t, ok := srv.(interface{ testEmbeddedByValue() }); ok { + t.testEmbeddedByValue() + } + s.RegisterService(&TelemetryService_ServiceDesc, srv) +} + +func _TelemetryService_AddCommandUsageAttribute_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(AddCommandUsageAttributeRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(TelemetryServiceServer).AddCommandUsageAttribute(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: TelemetryService_AddCommandUsageAttribute_FullMethodName, + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(TelemetryServiceServer).AddCommandUsageAttribute(ctx, req.(*AddCommandUsageAttributeRequest)) + } + return interceptor(ctx, in, info, handler) +} + +// TelemetryService_ServiceDesc is the grpc.ServiceDesc for TelemetryService service. +// It's only intended for direct use with grpc.RegisterService, +// and not to be introspected or modified (even as a copy) +var TelemetryService_ServiceDesc = grpc.ServiceDesc{ + ServiceName: "azdext.TelemetryService", + HandlerType: (*TelemetryServiceServer)(nil), + Methods: []grpc.MethodDesc{ + { + MethodName: "AddCommandUsageAttribute", + Handler: _TelemetryService_AddCommandUsageAttribute_Handler, + }, + }, + Streams: []grpc.StreamDesc{}, + Metadata: "telemetry.proto", +} diff --git a/cli/azd/pkg/project/artifact.go b/cli/azd/pkg/project/artifact.go index a365c629296..8303e9113f6 100644 --- a/cli/azd/pkg/project/artifact.go +++ b/cli/azd/pkg/project/artifact.go @@ -11,6 +11,7 @@ import ( "slices" "strings" + "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/output" ) @@ -22,6 +23,12 @@ const ( // MetadataKeyNote adds a note line below the artifact output. MetadataKeyNote = "note" + + // MetadataKeyFromPackage marks an artifact supplied through the + // --from-package option rather than produced by azd. The canonical value + // lives in the SDK so core and extensions share a single definition. This + // is deployment payload provenance, not telemetry. + MetadataKeyFromPackage = azdext.ArtifactMetadataKeyFromPackage ) // ArtifactKind represents well-known artifact types in the Azure Developer CLI From 38fd61a2b615926b6f7edc7a60f15ec4ae4b147c Mon Sep 17 00:00:00 2001 From: huimiu Date: Wed, 22 Jul 2026 16:56:33 +0800 Subject: [PATCH 02/12] test: add telemetry serialization test and docs --- .../extensions/extension-sdk-reference.md | 16 ++++++++++++++++ .../span_to_envelope_test.go | 19 +++++++++++++++++++ docs/specs/metrics-audit/telemetry-schema.md | 1 + 3 files changed, 36 insertions(+) diff --git a/cli/azd/docs/extensions/extension-sdk-reference.md b/cli/azd/docs/extensions/extension-sdk-reference.md index ba91ef45e3a..eded8ce2a66 100644 --- a/cli/azd/docs/extensions/extension-sdk-reference.md +++ b/cli/azd/docs/extensions/extension-sdk-reference.md @@ -519,9 +519,25 @@ gRPC client connecting to the azd framework. Auto-discovers the socket via | `Extension()` | `ExtensionServiceClient` | | `Account()` | `AccountServiceClient` | | `Ai()` | `AiModelServiceClient` | +| `Telemetry()` | `TelemetryServiceClient` | Always call `defer client.Close()` after creation. +#### TelemetryService + +`Telemetry().AddCommandUsageAttribute(ctx, &azdext.AddCommandUsageAttributeRequest{Key, Value})` +lets an authenticated extension contribute a host-validated command usage +attribute. The host owns the allowlist: it validates the key and value, checks +the extension's declared capabilities, and, when accepted, attaches the value +to the current command telemetry event (for example `cmd.deploy` or `cmd.up`) +as a de-duplicated string slice. Extensions cannot choose classification, +purpose, hashing, or aggregation, and unknown keys or values are rejected. + +The call is best-effort. It returns `Accepted=false` (not an error) when no +eligible command is active, and an older azd host returns `Unimplemented`. +Treat any failure as a no-op and never let it change command behavior. Report +a value as soon as it is known so a later failure still retains it. + ### ConfigHelper ```go diff --git a/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go b/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go index 3bf8c727722..610ee15b09b 100644 --- a/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go +++ b/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go @@ -169,6 +169,25 @@ func assertAttributeInPropertiesOrMeasurement( } } +// TestSpanToEnvelope_AgentDeployMode locks the App Insights representation of +// the agent.deploy.mode field. The logical OTel value is a string slice; the +// exporter stores it as JSON-encoded text in the Properties bag, so downstream +// Kusto must parse it as a dynamic array. +func TestSpanToEnvelope_AgentDeployMode(t *testing.T) { + t.Parallel() + + stub := getDefaultSpanStub() + stub.Attributes = []attribute.KeyValue{ + attribute.StringSlice("agent.deploy.mode", []string{"code", "container"}), + } + span := stub.Snapshot() + + envelope := SpanToEnvelope(span) + data := envelope.Data.(*contracts.Data).BaseData.(*contracts.RequestData) + + assert.Equal(t, `["code","container"]`, data.Properties["agent.deploy.mode"]) +} + func getDefaultSpanStub() tracetest.SpanStub { traceId, _ := trace.TraceIDFromHex("68f1c4f4ef5346e69d7f196761d10c68") spanId, _ := trace.SpanIDFromHex("7fbdc197a52f4825877ddd46e4ec7f6c") diff --git a/docs/specs/metrics-audit/telemetry-schema.md b/docs/specs/metrics-audit/telemetry-schema.md index d4bd317c0a2..e218838cee0 100644 --- a/docs/specs/metrics-audit/telemetry-schema.md +++ b/docs/specs/metrics-audit/telemetry-schema.md @@ -88,6 +88,7 @@ These are set once at process startup via `resource.New()` and attached to every | Service languages | `project.service.languages` | SystemMetadata | FeatureInsight | List of languages | | Service language | `project.service.language` | SystemMetadata | PerformanceAndHealth | Single service language | | Platform type | `platform.type` | SystemMetadata | FeatureInsight | e.g. `aca`, `aks` | +| 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 | ### Config and Environment From fbf94b55b03aeee635359d528ddee2f4d591d77f Mon Sep 17 00:00:00 2001 From: huimiu Date: Wed, 22 Jul 2026 18:06:16 +0800 Subject: [PATCH 03/12] test: modernize concurrency loops in telemetry tests --- cli/azd/internal/grpcserver/telemetry_service_test.go | 10 ++++------ cli/azd/internal/tracing/command_usage_test.go | 8 +++----- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/cli/azd/internal/grpcserver/telemetry_service_test.go b/cli/azd/internal/grpcserver/telemetry_service_test.go index abea91e2b6c..e2cfe77f10e 100644 --- a/cli/azd/internal/grpcserver/telemetry_service_test.go +++ b/cli/azd/internal/grpcserver/telemetry_service_test.go @@ -182,7 +182,7 @@ func TestTelemetryService_DuplicateCollapses(t *testing.T) { svc := NewTelemetryService() ctx := serviceTargetClaims() - for i := 0; i < 3; i++ { + for range 3 { resp, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) require.NoError(t, err) require.True(t, resp.Accepted) @@ -204,14 +204,12 @@ func TestTelemetryService_ConcurrentReports(t *testing.T) { modes := []string{"code", "container", "byo_image"} var wg sync.WaitGroup - for i := 0; i < 60; i++ { + for i := range 60 { value := modes[i%len(modes)] - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { _, err := svc.AddCommandUsageAttribute(ctx, validRequest(value)) require.NoError(t, err) - }() + }) } wg.Wait() diff --git a/cli/azd/internal/tracing/command_usage_test.go b/cli/azd/internal/tracing/command_usage_test.go index 195bf71e149..52d8f4cfce7 100644 --- a/cli/azd/internal/tracing/command_usage_test.go +++ b/cli/azd/internal/tracing/command_usage_test.go @@ -92,13 +92,11 @@ func TestCommandUsage_ConcurrentValues(t *testing.T) { modes := []string{"code", "container", "byo_image"} var wg sync.WaitGroup - for i := 0; i < 50; i++ { + for i := range 50 { mode := modes[i%len(modes)] - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, mode) - }() + }) } wg.Wait() From 898b2f94a6027664c01c30e9144816e00f60c3fb Mon Sep 17 00:00:00 2001 From: huimiu Date: Wed, 22 Jul 2026 20:59:27 +0800 Subject: [PATCH 04/12] fix: remove unrelated archive classification changes --- cli/azd/internal/cmd/from_package_test.go | 33 ----------------------- cli/azd/internal/cmd/publish.go | 21 +++++---------- 2 files changed, 6 insertions(+), 48 deletions(-) diff --git a/cli/azd/internal/cmd/from_package_test.go b/cli/azd/internal/cmd/from_package_test.go index 2f0e528a544..dcf87ba5ed4 100644 --- a/cli/azd/internal/cmd/from_package_test.go +++ b/cli/azd/internal/cmd/from_package_test.go @@ -4,8 +4,6 @@ package cmd import ( - "os" - "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -14,37 +12,6 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/project" ) -// TestDetermineArtifactKind locks --from-package classification, including the -// compound archive suffixes that filepath.Ext cannot recognize. -func TestDetermineArtifactKind(t *testing.T) { - t.Parallel() - dir := t.TempDir() - - archiveNames := []string{ - "pkg.zip", "pkg.tar", "pkg.tgz", "pkg.txz", "pkg.tbz2", - "pkg.tar.gz", "pkg.tar.bz2", "pkg.tar.xz", - "PKG.TAR.GZ", // classification is case-insensitive - } - for _, name := range archiveNames { - p := filepath.Join(dir, name) - require.NoError(t, os.WriteFile(p, []byte("x"), 0o600)) - require.Equalf(t, project.ArtifactKindArchive, determineArtifactKind(p), - "expected archive for %q", name) - } - - // An existing directory is a directory artifact. - require.Equal(t, project.ArtifactKindDirectory, determineArtifactKind(dir)) - - // A path that does not exist is treated as a container image reference. - require.Equal(t, project.ArtifactKindContainer, - determineArtifactKind("myregistry.azurecr.io/app:latest")) - - // An existing non-archive file is also treated as a container reference. - other := filepath.Join(dir, "notanarchive.bin") - require.NoError(t, os.WriteFile(other, []byte("x"), 0o600)) - require.Equal(t, project.ArtifactKindContainer, determineArtifactKind(other)) -} - // TestFromPackageMetadataConstant keeps the core alias and the SDK constant in // sync so core and extensions agree on the provenance marker. func TestFromPackageMetadataConstant(t *testing.T) { diff --git a/cli/azd/internal/cmd/publish.go b/cli/azd/internal/cmd/publish.go index bdae32d64dc..23174b5b073 100644 --- a/cli/azd/internal/cmd/publish.go +++ b/cli/azd/internal/cmd/publish.go @@ -9,6 +9,7 @@ import ( "io" "log" "os" + "path/filepath" "slices" "strings" "time" @@ -415,23 +416,13 @@ func (pa *PublishAction) supportsPublish(ctx context.Context, serviceConfig *pro // 1. Archive (zip file) - checks if it exists and matches popular archive extensions // 2. Directory - checks if it is an existing directory (absolute or relative) // 3. Container - otherwise, it's likely a local container reference -// archiveSuffixes are the file suffixes treated as a code archive for -// --from-package classification. Compound suffixes such as ".tar.gz" are -// matched with strings.HasSuffix because filepath.Ext only returns the final -// segment (".gz") and would misclassify them as a container reference. -var archiveSuffixes = []string{ - ".zip", ".tar", ".tar.gz", ".tgz", ".tar.bz2", ".tbz2", ".tar.xz", ".txz", -} - func determineArtifactKind(fromPackage string) project.ArtifactKind { - lower := strings.ToLower(fromPackage) - - // Check if it's an existing file with an archive suffix. + // Check if it's an existing file with archive extension if info, err := os.Stat(fromPackage); err == nil && !info.IsDir() { - for _, suffix := range archiveSuffixes { - if strings.HasSuffix(lower, suffix) { - return project.ArtifactKindArchive - } + ext := strings.ToLower(filepath.Ext(fromPackage)) + switch ext { + case ".zip", ".tar", ".tar.gz", ".tgz", ".tar.bz2", ".tbz2", ".tar.xz", ".txz": + return project.ArtifactKindArchive } } From c1d93755f447b8bca76ec151d9f7c38d876920e8 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 18:17:05 +0800 Subject: [PATCH 05/12] feat: declare extension telemetry fields in the azd registry --- cli/azd/cmd/middleware/telemetry.go | 16 - cli/azd/cmd/telemetry_test.go | 13 - .../docs/extensions/extension-framework.md | 1 + .../extensions/extension-sdk-reference.md | 50 ++- .../extensions/extension-telemetry-fields.md | 106 ++++++ cli/azd/extensions/extension.schema.json | 14 +- cli/azd/extensions/registry.schema.json | 39 +- cli/azd/grpc/proto/telemetry.proto | 28 +- cli/azd/internal/cmd/from_package_test.go | 21 -- cli/azd/internal/cmd/publish.go | 7 +- cli/azd/internal/cmd/service_graph.go | 7 +- .../internal/grpcserver/extension_claims.go | 1 + cli/azd/internal/grpcserver/server.go | 11 +- cli/azd/internal/grpcserver/server_test.go | 79 ++-- .../internal/grpcserver/telemetry_service.go | 143 ++++---- .../grpcserver/telemetry_service_test.go | 343 +++++++++--------- cli/azd/internal/grpcserver/trace_context.go | 70 ++++ .../internal/grpcserver/trace_context_test.go | 64 ++++ .../span_to_envelope_test.go | 19 - cli/azd/internal/tracing/command_usage.go | 147 -------- .../internal/tracing/command_usage_test.go | 220 ----------- cli/azd/internal/tracing/events/events.go | 4 + cli/azd/internal/tracing/fields/fields.go | 13 - cli/azd/pkg/azdext/artifact_metadata.go | 9 - cli/azd/pkg/azdext/azd_client.go | 50 ++- cli/azd/pkg/azdext/azd_client_test.go | 57 +++ cli/azd/pkg/azdext/telemetry.pb.go | 67 ++-- cli/azd/pkg/azdext/telemetry/attributes.go | 28 -- .../pkg/azdext/telemetry/attributes_test.go | 28 -- cli/azd/pkg/azdext/telemetry_grpc.pb.go | 56 ++- cli/azd/pkg/extensions/claims.go | 3 + cli/azd/pkg/extensions/extension.go | 3 + cli/azd/pkg/extensions/manager.go | 1 + cli/azd/pkg/extensions/registry.go | 7 + .../pkg/extensions/telemetry_declaration.go | 163 +++++++++ .../extensions/telemetry_declaration_test.go | 198 ++++++++++ cli/azd/pkg/extensions/validate_registry.go | 22 +- cli/azd/pkg/project/artifact.go | 7 - docs/README.md | 1 + ...dr-001-extension-telemetry-declarations.md | 94 +++++ docs/architecture/extension-framework.md | 1 + docs/architecture/telemetry.md | 6 + docs/concepts/glossary.md | 6 +- docs/specs/metrics-audit/telemetry-schema.md | 30 +- 44 files changed, 1326 insertions(+), 927 deletions(-) create mode 100644 cli/azd/docs/extensions/extension-telemetry-fields.md delete mode 100644 cli/azd/internal/cmd/from_package_test.go create mode 100644 cli/azd/internal/grpcserver/trace_context.go create mode 100644 cli/azd/internal/grpcserver/trace_context_test.go delete mode 100644 cli/azd/internal/tracing/command_usage.go delete mode 100644 cli/azd/internal/tracing/command_usage_test.go delete mode 100644 cli/azd/pkg/azdext/artifact_metadata.go delete mode 100644 cli/azd/pkg/azdext/telemetry/attributes.go delete mode 100644 cli/azd/pkg/azdext/telemetry/attributes_test.go create mode 100644 cli/azd/pkg/extensions/telemetry_declaration.go create mode 100644 cli/azd/pkg/extensions/telemetry_declaration_test.go create mode 100644 docs/architecture/adr-001-extension-telemetry-declarations.md diff --git a/cli/azd/cmd/middleware/telemetry.go b/cli/azd/cmd/middleware/telemetry.go index 7033343dd7b..1a105878c47 100644 --- a/cli/azd/cmd/middleware/telemetry.go +++ b/cli/azd/cmd/middleware/telemetry.go @@ -65,13 +65,6 @@ func (m *TelemetryMiddleware) Run(ctx context.Context, next NextFn) (*actions.Ac spanCtx, span := tracing.Start(ctx, eventName) - // Begin a command usage scope keyed by this exact event name. Extensions - // contribute host-validated usage attributes through the telemetry gRPC - // 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) - log.Printf("TraceID: %s", span.SpanContext().TraceID()) if !IsChildAction(ctx) { @@ -110,15 +103,6 @@ func (m *TelemetryMiddleware) Run(ctx context.Context, next NextFn) (*actions.Ac m.setInstalledExtensionsAttributes(span) defer func() { - // Attach any command-scoped usage attributes reported by extensions, - // then close the scope. A close error is unexpected and only logged; - // it never changes the command result. - if commandAttrs, closeErr := tracing.CloseCommandUsageScope(usageScope); closeErr != nil { - log.Printf("closing command usage scope: %v", closeErr) - } else { - span.SetAttributes(commandAttrs...) - } - // Include any usage attributes set span.SetAttributes(tracing.GetUsageAttributes()...) span.SetAttributes(fields.PerfInteractTime.Int64(tracing.InteractTimeMs.Load())) diff --git a/cli/azd/cmd/telemetry_test.go b/cli/azd/cmd/telemetry_test.go index 54b92b6d854..18328955e78 100644 --- a/cli/azd/cmd/telemetry_test.go +++ b/cli/azd/cmd/telemetry_test.go @@ -83,19 +83,6 @@ func TestTelemetryFieldConstants(t *testing.T) { } }) - // Agent deployment mode telemetry field (contributed by extensions through - // the host-validated telemetry service). - t.Run("AgentDeploymentModeField", func(t *testing.T) { - t.Parallel() - require.Equal(t, "agent.deploy.mode", string(fields.AgentDeploymentModeKey.Key)) - require.Equal(t, fields.SystemMetadata, fields.AgentDeploymentModeKey.Classification) - require.Equal(t, fields.FeatureInsight, fields.AgentDeploymentModeKey.Purpose) - require.False(t, fields.AgentDeploymentModeKey.IsMeasurement) - - kv := fields.AgentDeploymentModeKey.StringSlice([]string{"code", "container", "byo_image"}) - require.Equal(t, []string{"code", "container", "byo_image"}, kv.Value.AsStringSlice()) - }) - // Tool command telemetry fields t.Run("ToolFields", func(t *testing.T) { t.Parallel() diff --git a/cli/azd/docs/extensions/extension-framework.md b/cli/azd/docs/extensions/extension-framework.md index 60f550d995f..7bbac014c92 100644 --- a/cli/azd/docs/extensions/extension-framework.md +++ b/cli/azd/docs/extensions/extension-framework.md @@ -1104,6 +1104,7 @@ Extensions can declare the following capabilities in their manifest: - **`provisioning-provider`**: Provide a custom infrastructure provisioning experience (alternative to Bicep / Terraform) - **`validation-provider`**: Contribute validation checks to azd's provision validation and future validation pipelines - **`metadata`**: Provide comprehensive metadata about commands and configuration schemas +- **`telemetry`**: Report usage attributes declared in the extension's official registry entry (see [Extension Telemetry Fields](./extension-telemetry-fields.md)) #### Complete Extension Manifest Example diff --git a/cli/azd/docs/extensions/extension-sdk-reference.md b/cli/azd/docs/extensions/extension-sdk-reference.md index eded8ce2a66..64e805f7396 100644 --- a/cli/azd/docs/extensions/extension-sdk-reference.md +++ b/cli/azd/docs/extensions/extension-sdk-reference.md @@ -525,18 +525,44 @@ Always call `defer client.Close()` after creation. #### TelemetryService -`Telemetry().AddCommandUsageAttribute(ctx, &azdext.AddCommandUsageAttributeRequest{Key, Value})` -lets an authenticated extension contribute a host-validated command usage -attribute. The host owns the allowlist: it validates the key and value, checks -the extension's declared capabilities, and, when accepted, attaches the value -to the current command telemetry event (for example `cmd.deploy` or `cmd.up`) -as a de-duplicated string slice. Extensions cannot choose classification, -purpose, hashing, or aggregation, and unknown keys or values are rejected. - -The call is best-effort. It returns `Accepted=false` (not an error) when no -eligible command is active, and an older azd host returns `Unimplemented`. -Treat any failure as a no-op and never let it change command behavior. Report -a value as soon as it is known so a later failure still retains it. +`Telemetry().ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{Key, Value})` +lets an authenticated extension report one usage attribute value. The +extension declares the key and its closed value set in its entry in the +official azd registry; `azd` core owns no product-specific fields, so adding a +new field is a registry change rather than a core release. + +The host validates every call and fails closed. It requires the extension to +be installed from the official `azd` registry, to carry the `telemetry` +capability, and to have declared both the key and the exact value. Accepted +values are recorded on a dedicated `ext.usage` span that shares the command's +trace, so downstream queries join it to the originating command on +`operation_Id`. Extensions cannot choose the span, classification, purpose, +hashing, or aggregation. + +Declare fields in the registry entry alongside the capability: + +```json +{ + "version": "1.0.0", + "capabilities": ["telemetry"], + "telemetry": [ + { + "key": "ext.contoso.tools.deploy.mode", + "allowedValues": ["code", "container"] + } + ] +} +``` + +Keys must be namespaced as `ext..`, and values are +limited to lowercase alphanumerics with `_`, `-`, and `.` so registry names, +URLs, and paths cannot be smuggled through a value. See +[Extension Telemetry Fields](./extension-telemetry-fields.md) for the full +declaration rules and review process. + +The call is best-effort. An older azd host returns `Unimplemented`. Treat any +failure as a no-op and never let it change command behavior. Report a value as +soon as it is known so a later failure still retains it. ### ConfigHelper diff --git a/cli/azd/docs/extensions/extension-telemetry-fields.md b/cli/azd/docs/extensions/extension-telemetry-fields.md new file mode 100644 index 00000000000..426c1b5855e --- /dev/null +++ b/cli/azd/docs/extensions/extension-telemetry-fields.md @@ -0,0 +1,106 @@ +# Extension Telemetry Fields + +This guide is for extension authors who need `azd` to record a bounded usage +signal on their behalf — for example, which deployment mode a user picked. + +`azd` core owns no product-specific telemetry fields. Every field an extension +can report is declared in that extension's entry in the official `azd` +registry, and `azd` only enforces the declaration. Adding a field is therefore +a registry pull request in this repository, not a core release. + +See [ADR-001](../../../../docs/architecture/adr-001-extension-telemetry-declarations.md) +for the reasoning behind this design. + +## What you can and cannot report + +| | | +|---|---| +| You choose | The key name and the closed set of values it may take | +| `azd` chooses | Which span the value lands on, its classification, and how it is exported | +| Always rejected | Free-form values, values outside your declared set, keys you did not declare | + +Values must be a closed enum because that is what makes a privacy review +possible. A shape-only rule (length and charset) would let a value like +`byo_image:myregistry.azurecr.io/foo` through and leak a registry name, so the +charset excludes `:` and `/` and the host compares each value against your +declaration. + +## Step 1: Declare the capability + +Add `telemetry` to `capabilities` in your `extension.yaml`: + +```yaml +capabilities: + - custom-commands + - telemetry +``` + +## Step 2: Declare the fields in the registry entry + +Add a `telemetry` array to your version entry in the official registry +(`cli/azd/extensions/registry.json`). Unlike `capabilities`, this is not +copied from `extension.yaml` — it only exists in the reviewed registry entry: + +```json +{ + "version": "1.0.0", + "capabilities": ["custom-commands", "telemetry"], + "telemetry": [ + { + "key": "ext.contoso.tools.deploy.mode", + "allowedValues": ["code", "container", "unknown"] + } + ] +} +``` + +Rules enforced when the registry is validated: + +| Rule | Limit | +|---|---| +| Key namespace | Must start with `ext..` | +| Key length | 128 characters | +| Fields per version | 16 | +| Values per field | 1–32, unique | +| Value length | 64 characters | +| Value charset | Lowercase alphanumerics plus `_`, `-`, `.` | +| Capability | `telemetry` must be present when fields are declared | + +The pull request that adds these values is the privacy review. Expect +reviewers to ask what each value means and why it is needed. + +## Step 3: Report from your extension + +```go +_, err := client.Telemetry().ReportUsageAttribute( + ctx, + &azdext.ReportUsageAttributeRequest{ + Key: "ext.contoso.tools.deploy.mode", + Value: "container", + }, +) +``` + +Treat the call as best-effort. Older `azd` hosts return `Unimplemented`, and +every rejection is a plain error. Never let the result change command +behavior, and never retry. Report the value as soon as it is known so a later +failure in your command still keeps the signal. + +## Why a call might be rejected + +| Status | Cause | +|---|---| +| `Unauthenticated` | The request did not carry the host-issued extension token | +| `PermissionDenied` | The extension was not installed from the official `azd` registry, is missing the `telemetry` capability, or is not installed | +| `FailedPrecondition` | The stored declaration no longer passes validation | +| `InvalidArgument` | The key was not declared, or the value is not in the declared set | + +Error messages never echo the key or value you sent, so use the status code +plus your own declaration to diagnose. + +## Where the data lands + +Each accepted value becomes an `ext.usage` span carrying `extension.id`, +`extension.version`, and your attribute. The span shares the command's trace, +so it joins to the originating command on `operation_Id` in Application +Insights. diff --git a/cli/azd/extensions/extension.schema.json b/cli/azd/extensions/extension.schema.json index 2a8340e59ff..27e5f048585 100644 --- a/cli/azd/extensions/extension.schema.json +++ b/cli/azd/extensions/extension.schema.json @@ -112,7 +112,7 @@ "capabilities": { "type": "array", "title": "Capabilities", - "description": "List of capabilities provided by the extension. Supported values: custom-commands, lifecycle-events, mcp-server, service-target-provider, framework-service-provider, provisioning-provider, validation-provider, metadata. Select one or more from the allowed list. Each value must be unique. Not required for extension packs, which declare dependencies instead and have no executable.", + "description": "List of capabilities provided by the extension. Supported values: custom-commands, lifecycle-events, mcp-server, service-target-provider, framework-service-provider, provisioning-provider, validation-provider, metadata, telemetry. Select one or more from the allowed list. Each value must be unique. Not required for extension packs, which declare dependencies instead and have no executable.", "minItems": 1, "uniqueItems": true, "items": { @@ -159,11 +159,17 @@ "title": "Validation Provider", "description": "Validation provider enables extensions to contribute checks to azd validation pipelines." }, - { - "type": "string", - "const": "metadata", + { + "type": "string", + "const": "metadata", "title": "Metadata", "description": "Metadata capability enables extensions to provide comprehensive metadata about their commands and capabilities via a metadata command." + }, + { + "type": "string", + "const": "telemetry", + "title": "Telemetry", + "description": "Telemetry capability enables extensions to report usage attributes declared in their official registry entry. Only extensions installed from the official azd registry may report." } ] } diff --git a/cli/azd/extensions/registry.schema.json b/cli/azd/extensions/registry.schema.json index 5ab7d22be42..2ddbd6014c1 100644 --- a/cli/azd/extensions/registry.schema.json +++ b/cli/azd/extensions/registry.schema.json @@ -81,9 +81,42 @@ "service-target-provider", "framework-service-provider", "provisioning-provider", - "validation-provider", - "metadata" - ] + "validation-provider", + "metadata", + "telemetry" + ] + } + }, + "telemetry": { + "type": "array", + "description": "Usage attributes this version may report through the telemetry service. Requires the 'telemetry' capability.", + "maxItems": 16, + "items": { + "type": "object", + "properties": { + "key": { + "type": "string", + "description": "Telemetry attribute name, namespaced as 'ext..'.", + "maxLength": 128, + "pattern": "^ext\\.[a-z0-9]([a-z0-9-]*[a-z0-9])?(\\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$" + }, + "allowedValues": { + "type": "array", + "description": "Closed set of values that may be reported for this key. Free-form values are always rejected.", + "minItems": 1, + "maxItems": 32, + "uniqueItems": true, + "items": { + "type": "string", + "maxLength": 64, + "pattern": "^[a-z0-9]([a-z0-9_.-]*[a-z0-9])?$" + } + } + }, + "required": [ + "key", + "allowedValues" + ] } }, "usage": { diff --git a/cli/azd/grpc/proto/telemetry.proto b/cli/azd/grpc/proto/telemetry.proto index 8c6e20faa96..3b5256eaf95 100644 --- a/cli/azd/grpc/proto/telemetry.proto +++ b/cli/azd/grpc/proto/telemetry.proto @@ -6,28 +6,26 @@ package azdext; option go_package = "github.com/azure/azure-dev/cli/azd/pkg/azdext"; -// TelemetryService accepts reviewed, host-owned command usage attribute values -// from authenticated extensions. The host validates the key and value against -// an allowlist and attaches accepted values to the current command telemetry -// event. Extensions cannot choose classification, purpose, hashing, or -// aggregation. +// TelemetryService accepts usage attribute values from authenticated +// extensions. Every key and value must appear in the telemetry declaration +// the extension published in the official azd registry, which is where those +// fields are reviewed. The host decides which telemetry span a value lands on +// and owns classification, purpose, hashing, and aggregation. service TelemetryService { - // AddCommandUsageAttribute adds one bounded value to a host-owned command - // usage attribute. Duplicate values are collapsed by the host. - rpc AddCommandUsageAttribute(AddCommandUsageAttributeRequest) - returns (AddCommandUsageAttributeResponse); + // ReportUsageAttribute reports one declared usage attribute value. + rpc ReportUsageAttribute(ReportUsageAttributeRequest) + returns (ReportUsageAttributeResponse); } -message AddCommandUsageAttributeRequest { - // Key must be registered and owned by the host. +message ReportUsageAttributeRequest { + // Key must match a telemetry field the extension declared in the registry. string key = 1; - // Value must be in the host-owned allowed set for key. + // Value must be in the allowed set the extension declared for key. string value = 2; } -message AddCommandUsageAttributeResponse { - // Accepted is false when the value was rejected because no eligible command - // scope is currently active. A false response is not an error. +message ReportUsageAttributeResponse { + // Accepted reports whether the host recorded the value. bool accepted = 1; } diff --git a/cli/azd/internal/cmd/from_package_test.go b/cli/azd/internal/cmd/from_package_test.go deleted file mode 100644 index dcf87ba5ed4..00000000000 --- a/cli/azd/internal/cmd/from_package_test.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package cmd - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/azure/azure-dev/cli/azd/pkg/project" -) - -// TestFromPackageMetadataConstant keeps the core alias and the SDK constant in -// sync so core and extensions agree on the provenance marker. -func TestFromPackageMetadataConstant(t *testing.T) { - t.Parallel() - require.Equal(t, "azd.fromPackage", project.MetadataKeyFromPackage) - require.Equal(t, azdext.ArtifactMetadataKeyFromPackage, project.MetadataKeyFromPackage) -} diff --git a/cli/azd/internal/cmd/publish.go b/cli/azd/internal/cmd/publish.go index 23174b5b073..4d109640e1c 100644 --- a/cli/azd/internal/cmd/publish.go +++ b/cli/azd/internal/cmd/publish.go @@ -286,16 +286,11 @@ func (pa *PublishAction) Run(ctx context.Context) (*actions.ActionResult, error) serviceContext := &project.ServiceContext{} if pa.flags.FromPackage != "" { - // --from-package set, skip packaging and create package artifact. - // Mark it with generic provenance so a service target can tell a - // caller-supplied payload from one azd produced. + // --from-package set, skip packaging and create package artifact err = serviceContext.Package.Add(&project.Artifact{ Kind: determineArtifactKind(pa.flags.FromPackage), Location: pa.flags.FromPackage, LocationKind: project.LocationKindLocal, - Metadata: map[string]string{ - project.MetadataKeyFromPackage: "true", - }, }) if err != nil { diff --git a/cli/azd/internal/cmd/service_graph.go b/cli/azd/internal/cmd/service_graph.go index 0e228159b11..f8adcbfa3aa 100644 --- a/cli/azd/internal/cmd/service_graph.go +++ b/cli/azd/internal/cmd/service_graph.go @@ -373,16 +373,11 @@ func addServiceStepsToGraph(g *exegraph.Graph, opts serviceGraphOptions) (*servi if opts.fromPackage != "" { // --from-package bypasses the packager and wraps the - // user-supplied artifact directly. Mark it with generic - // provenance so a service target can distinguish a - // caller-supplied payload from one azd produced. + // user-supplied artifact directly. if pkgErr := sc.Package.Add(&project.Artifact{ Kind: determineArtifactKind(opts.fromPackage), Location: opts.fromPackage, LocationKind: project.LocationKindLocal, - Metadata: map[string]string{ - project.MetadataKeyFromPackage: "true", - }, }); pkgErr != nil { return fmt.Errorf("packaging service %s: %w", pkgSvc.Name, pkgErr) } diff --git a/cli/azd/internal/grpcserver/extension_claims.go b/cli/azd/internal/grpcserver/extension_claims.go index a4273c57a36..6a8e166c04d 100644 --- a/cli/azd/internal/grpcserver/extension_claims.go +++ b/cli/azd/internal/grpcserver/extension_claims.go @@ -23,6 +23,7 @@ func GenerateExtensionToken(extension *extensions.Extension, serverInfo *ServerI ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour * 1)), }, Capabilities: extension.Capabilities, + Source: extension.Source, } jwtToken, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(serverInfo.SigningKey) diff --git a/cli/azd/internal/grpcserver/server.go b/cli/azd/internal/grpcserver/server.go index 7215343dc70..03abc800c5c 100644 --- a/cli/azd/internal/grpcserver/server.go +++ b/cli/azd/internal/grpcserver/server.go @@ -100,10 +100,12 @@ func (s *Server) Start() (*ServerInfo, error) { s.grpcServer = grpc.NewServer( grpc.ChainUnaryInterceptor( s.errorWrappingInterceptor(), + s.traceContextInterceptor(), s.tokenAuthInterceptor(&serverInfo), ), grpc.ChainStreamInterceptor( s.errorWrappingStreamInterceptor(), + s.traceContextStreamInterceptor(), s.tokenAuthStreamInterceptor(&serverInfo), ), ) @@ -252,7 +254,7 @@ func (s *Server) tokenAuthStreamInterceptor(serverInfo *ServerInfo) grpc.StreamS } // Wrap the stream to inject validated claims into its context - wrappedStream := &authenticatedStream{ + wrappedStream := &contextStream{ ServerStream: ss, ctx: ctx, } @@ -261,13 +263,14 @@ func (s *Server) tokenAuthStreamInterceptor(serverInfo *ServerInfo) grpc.StreamS } } -// authenticatedStream wraps a grpc.ServerStream to provide a context with validated claims. -type authenticatedStream struct { +// contextStream wraps a grpc.ServerStream to override the context seen by the +// handler, for example to carry validated claims or trace context. +type contextStream struct { grpc.ServerStream ctx context.Context } -func (s *authenticatedStream) Context() context.Context { +func (s *contextStream) Context() context.Context { return s.ctx } diff --git a/cli/azd/internal/grpcserver/server_test.go b/cli/azd/internal/grpcserver/server_test.go index 43951327022..439c7481ca8 100644 --- a/cli/azd/internal/grpcserver/server_test.go +++ b/cli/azd/internal/grpcserver/server_test.go @@ -23,12 +23,10 @@ import ( "google.golang.org/protobuf/proto" "github.com/azure/azure-dev/cli/azd/internal" - "github.com/azure/azure-dev/cli/azd/internal/tracing" "github.com/azure/azure-dev/cli/azd/pkg/account" "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/azapi" "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/azure/azure-dev/cli/azd/pkg/prompt" @@ -37,6 +35,24 @@ import ( // Test_Server_Start validates the start and stop flows of the gRPC server, // and confirms the expected behavior for authenticated and unauthenticated requests. func Test_Server_Start(t *testing.T) { + // The reporting extension is installed from the official registry and + // declares one bounded field, which is what the host validates against. + reportingExtension := &extensions.Extension{ + Id: "azd.internal.telemetry", + Version: "1.0.0", + Source: extensions.MainRegistryName, + Capabilities: []extensions.CapabilityType{ + extensions.TelemetryCapability, + }, + Telemetry: []extensions.TelemetryFieldDeclaration{ + { + Key: "ext.azd.internal.telemetry.deploy.mode", + AllowedValues: []string{"code", "container"}, + }, + }, + Namespace: "test", + } + server := NewServer( azdext.UnimplementedProjectServiceServer{}, azdext.UnimplementedEnvironmentServiceServer{}, @@ -55,7 +71,7 @@ func Test_Server_Start(t *testing.T) { azdext.UnimplementedCopilotServiceServer{}, azdext.UnimplementedProvisioningServiceServer{}, azdext.UnimplementedValidationServiceServer{}, - NewTelemetryService(), + newTelemetryService(stubExtensionLookup{reportingExtension}), ) serverInfo, err := server.Start() @@ -124,37 +140,36 @@ func Test_Server_Start(t *testing.T) { }) t.Run("TelemetryAccepted", func(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) - - stExtension := &extensions.Extension{ - Id: "azd.internal.telemetry", - Capabilities: []extensions.CapabilityType{ - extensions.ServiceTargetProviderCapability, - }, - Namespace: "test", - } - accessToken, err := GenerateExtensionToken(stExtension, serverInfo) + accessToken, err := GenerateExtensionToken(reportingExtension, serverInfo) require.NoError(t, err) ctx := azdext.WithAccessToken(t.Context(), accessToken) client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) require.NoError(t, err) - // The server runs in-process, so the handler writes to this scope. - scope := tracing.BeginCommandUsageScope("cmd.deploy") - - resp, err := client.Telemetry().AddCommandUsageAttribute(ctx, &azdext.AddCommandUsageAttributeRequest{ - Key: telemetry.AgentDeploymentModeAttribute, - Value: string(telemetry.AgentDeploymentModeCode), + resp, err := client.Telemetry().ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{ + Key: "ext.azd.internal.telemetry.deploy.mode", + Value: "code", }) require.NoError(t, err) require.True(t, resp.Accepted) + }) - attrs, err := tracing.CloseCommandUsageScope(scope) + t.Run("TelemetryUndeclaredValue", func(t *testing.T) { + accessToken, err := GenerateExtensionToken(reportingExtension, serverInfo) + require.NoError(t, err) + + ctx := azdext.WithAccessToken(t.Context(), accessToken) + client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) require.NoError(t, err) - require.Len(t, attrs, 1) - require.Equal(t, []string{"code"}, attrs[0].Value.AsStringSlice()) + + _, err = client.Telemetry().ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{ + Key: "ext.azd.internal.telemetry.deploy.mode", + Value: "byo_image", + }) + st, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, codes.InvalidArgument, st.Code()) }) t.Run("TelemetryMissingCapability", func(t *testing.T) { @@ -166,9 +181,9 @@ func Test_Server_Start(t *testing.T) { client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) require.NoError(t, err) - _, err = client.Telemetry().AddCommandUsageAttribute(ctx, &azdext.AddCommandUsageAttributeRequest{ - Key: telemetry.AgentDeploymentModeAttribute, - Value: string(telemetry.AgentDeploymentModeCode), + _, err = client.Telemetry().ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{ + Key: "ext.azd.internal.telemetry.deploy.mode", + Value: "code", }) st, ok := status.FromError(err) require.True(t, ok) @@ -179,11 +194,11 @@ func Test_Server_Start(t *testing.T) { client, err := azdext.NewAzdClient(azdext.WithAddress(serverInfo.Address)) require.NoError(t, err) - _, err = client.Telemetry().AddCommandUsageAttribute( + _, err = client.Telemetry().ReportUsageAttribute( t.Context(), - &azdext.AddCommandUsageAttributeRequest{ - Key: telemetry.AgentDeploymentModeAttribute, - Value: string(telemetry.AgentDeploymentModeCode), + &azdext.ReportUsageAttributeRequest{ + Key: "ext.azd.internal.telemetry.deploy.mode", + Value: "code", }, ) st, ok := status.FromError(err) @@ -470,11 +485,11 @@ func requireAuthErrorInfo(t *testing.T, st *status.Status) *errdetails.ErrorInfo return nil } -func TestAuthenticatedStream_Context(t *testing.T) { +func TestContextStream_Context(t *testing.T) { t.Parallel() ctx := context.WithValue(t.Context(), struct{ key string }{key: "test"}, "value") - stream := &authenticatedStream{ + stream := &contextStream{ ctx: ctx, } diff --git a/cli/azd/internal/grpcserver/telemetry_service.go b/cli/azd/internal/grpcserver/telemetry_service.go index e96499c223b..7fda0442de1 100644 --- a/cli/azd/internal/grpcserver/telemetry_service.go +++ b/cli/azd/internal/grpcserver/telemetry_service.go @@ -6,81 +6,53 @@ package grpcserver import ( "context" "slices" + "strings" "github.com/azure/azure-dev/cli/azd/internal/tracing" "github.com/azure/azure-dev/cli/azd/internal/tracing/events" "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" "github.com/azure/azure-dev/cli/azd/pkg/extensions" + "go.opentelemetry.io/otel/attribute" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -// maxTelemetryFieldLength bounds the accepted key and value length. This is a -// defensive limit against oversized input, not a privacy control. The value -// allowlist is the privacy control. -const maxTelemetryFieldLength = 128 - -// commandUsageFieldPolicy is the host-owned policy for one extension-settable -// command usage attribute. -type commandUsageFieldPolicy struct { - key fields.AttributeKey - allowedValues map[string]struct{} - eligibleEvents map[string]struct{} - requiredCapabilities map[extensions.CapabilityType]struct{} -} - -// extensionUsageFields is the allowlist of command usage attributes an -// authenticated extension may contribute. The host owns every field's key, -// classification, allowed values, eligible command events, and required -// capabilities. No extension identifier is hardcoded: eligibility is expressed -// through signed extension capabilities and the fixed value set. -var extensionUsageFields = map[string]commandUsageFieldPolicy{ - telemetry.AgentDeploymentModeAttribute: { - key: fields.AgentDeploymentModeKey, - allowedValues: map[string]struct{}{ - string(telemetry.AgentDeploymentModeCode): {}, - string(telemetry.AgentDeploymentModeContainer): {}, - string(telemetry.AgentDeploymentModeByoImage): {}, - }, - eligibleEvents: map[string]struct{}{ - events.GetCommandEventName("azd deploy"): {}, - events.GetCommandEventName("azd up"): {}, - }, - requiredCapabilities: map[extensions.CapabilityType]struct{}{ - extensions.ServiceTargetProviderCapability: {}, - }, - }, +// installedExtensionLookup resolves the installed extension record for a +// signed extension id. *extensions.Manager satisfies it. +type installedExtensionLookup interface { + GetInstalled(options extensions.FilterOptions) (*extensions.Extension, error) } // telemetryService implements azdext.TelemetryServiceServer. type telemetryService struct { azdext.UnimplementedTelemetryServiceServer - fields map[string]commandUsageFieldPolicy + extensions installedExtensionLookup } -// NewTelemetryService creates the telemetry gRPC service. It holds no injected -// state; the handler reaches the process-global command usage store through the -// tracing package. Returning the interface type lets the IoC container satisfy -// the azdext.TelemetryServiceServer parameter on NewServer without an adapter. -func NewTelemetryService() azdext.TelemetryServiceServer { - return newTelemetryService(extensionUsageFields) +// NewTelemetryService creates the telemetry gRPC service. The extension +// manager supplies the telemetry declarations an extension published in the +// registry it was installed from; the host owns no product-specific fields. +// Returning the interface type lets the IoC container satisfy the +// azdext.TelemetryServiceServer parameter on NewServer without an adapter. +func NewTelemetryService(manager *extensions.Manager) azdext.TelemetryServiceServer { + return newTelemetryService(manager) } -func newTelemetryService(fieldPolicies map[string]commandUsageFieldPolicy) *telemetryService { - return &telemetryService{fields: fieldPolicies} +func newTelemetryService(lookup installedExtensionLookup) *telemetryService { + return &telemetryService{extensions: lookup} } -// AddCommandUsageAttribute validates an authenticated extension's request -// against the host allowlist and, when valid, appends the value to the current -// command usage scope. It fails closed: unknown keys, missing capabilities, -// invalid policies, and disallowed values are all rejected before anything is -// recorded. Rejected caller text is never echoed into the returned error. -func (s *telemetryService) AddCommandUsageAttribute( +// ReportUsageAttribute records one usage attribute value that the calling +// extension declared in the official azd registry. It fails closed: callers +// without validated claims, extensions installed from any other source, +// extensions without the telemetry capability, undeclared keys, and values +// outside the declared set are all rejected before anything is recorded. +// Rejected caller text is never echoed into the returned error. +func (s *telemetryService) ReportUsageAttribute( ctx context.Context, - req *azdext.AddCommandUsageAttributeRequest, -) (*azdext.AddCommandUsageAttributeResponse, error) { + req *azdext.ReportUsageAttributeRequest, +) (*azdext.ReportUsageAttributeResponse, error) { claims, err := extensions.GetClaimsFromContext(ctx) if err != nil { return nil, status.Error(codes.Unauthenticated, "validated extension claims are required") @@ -88,44 +60,65 @@ func (s *telemetryService) AddCommandUsageAttribute( if req == nil || req.Key == "" || req.Value == "" || - len(req.Key) > maxTelemetryFieldLength || len(req.Value) > maxTelemetryFieldLength { + len(req.Key) > extensions.MaxTelemetryKeyLength || + len(req.Value) > extensions.MaxTelemetryValueLength { return nil, status.Error(codes.InvalidArgument, "telemetry key and value are required") } - policy, ok := s.fields[req.Key] - if !ok { - return nil, status.Error(codes.InvalidArgument, "telemetry key is not registered") + // Only extensions published through the official registry may report + // telemetry, because that registry is where declared fields are reviewed. + // Source is host-signed, so an extension cannot claim a better origin. + if !strings.EqualFold(claims.Source, extensions.MainRegistryName) { + return nil, status.Error(codes.PermissionDenied, + "telemetry requires an extension installed from the official registry") } - if !hasRequiredCapability(claims, policy.requiredCapabilities) { - return nil, status.Error(codes.PermissionDenied, "extension lacks the required capability") + if !slices.Contains(claims.Capabilities, extensions.TelemetryCapability) { + return nil, status.Error(codes.PermissionDenied, "extension lacks the telemetry capability") } - if len(policy.allowedValues) == 0 || len(policy.eligibleEvents) == 0 { - return nil, status.Error(codes.Internal, "telemetry field policy is invalid") + extension, err := s.extensions.GetInstalled(extensions.FilterOptions{Id: claims.Subject}) + if err != nil { + return nil, status.Error(codes.PermissionDenied, "extension is not installed") } - if _, ok := policy.allowedValues[req.Value]; !ok { - return nil, status.Error(codes.InvalidArgument, "telemetry value is not allowed") + // Re-validate the stored declaration. The installed record lives in user + // config, so shape is enforced again here rather than trusted from disk. + if issues := extensions.ValidateTelemetryDeclarations( + extension.Id, extension.Telemetry); len(issues) > 0 { + return nil, status.Error(codes.FailedPrecondition, "telemetry declarations are invalid") } - accepted := tracing.TryAppendCommandUsageUnique(policy.eligibleEvents, policy.key.Key, req.Value) + if !isDeclaredValue(extension.Telemetry, req.Key, req.Value) { + return nil, status.Error(codes.InvalidArgument, "telemetry value is not declared") + } - return &azdext.AddCommandUsageAttributeResponse{Accepted: accepted}, nil + // Record a dedicated span rather than augmenting the command span. The + // extension's trace context arrives over gRPC metadata, so this span + // shares the command's trace and joins on operation_Id downstream. + _, span := tracing.Start(ctx, events.ExtensionUsageEvent) + span.SetAttributes( + fields.ExtensionId.String(extension.Id), + fields.ExtensionVersion.String(extension.Version), + attribute.String(req.Key, req.Value), + ) + span.End() + + return &azdext.ReportUsageAttributeResponse{Accepted: true}, nil } -// hasRequiredCapability reports whether the signed extension claims carry every -// required capability. Capabilities are part of the host-signed token, so they -// are trustworthy without a separate manager lookup. -func hasRequiredCapability( - claims *extensions.ExtensionClaims, - required map[extensions.CapabilityType]struct{}, +// isDeclaredValue reports whether key is declared and value is in that key's +// declared closed set. +func isDeclaredValue( + declarations []extensions.TelemetryFieldDeclaration, + key string, + value string, ) bool { - for capability := range required { - if !slices.Contains(claims.Capabilities, capability) { - return false + for _, declaration := range declarations { + if declaration.Key == key { + return slices.Contains(declaration.AllowedValues, value) } } - return true + return false } diff --git a/cli/azd/internal/grpcserver/telemetry_service_test.go b/cli/azd/internal/grpcserver/telemetry_service_test.go index e2cfe77f10e..fc33c1f0499 100644 --- a/cli/azd/internal/grpcserver/telemetry_service_test.go +++ b/cli/azd/internal/grpcserver/telemetry_service_test.go @@ -4,245 +4,230 @@ package grpcserver import ( - "context" "strings" - "sync" "testing" + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - - "github.com/azure/azure-dev/cli/azd/internal/tracing" - "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" - "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" - "github.com/azure/azure-dev/cli/azd/pkg/extensions" ) -func claimsContext(caps ...extensions.CapabilityType) context.Context { - claims := &extensions.ExtensionClaims{ - RegisteredClaims: jwt.RegisteredClaims{Subject: "test.extension"}, - Capabilities: caps, - } - return extensions.WithClaimsContext(context.Background(), claims) +const testTelemetryKey = "ext.azd.internal.telemetry.deploy.mode" + +// stubExtensionLookup returns a fixed installed extension record, or a +// not-found error when the requested id does not match. +type stubExtensionLookup struct { + extension *extensions.Extension } -func serviceTargetClaims() context.Context { - return claimsContext(extensions.ServiceTargetProviderCapability) +func (s stubExtensionLookup) GetInstalled( + options extensions.FilterOptions, +) (*extensions.Extension, error) { + if s.extension == nil || s.extension.Id != options.Id { + return nil, extensions.ErrInstalledExtensionNotFound + } + + return s.extension, nil } -func validRequest(value string) *azdext.AddCommandUsageAttributeRequest { - return &azdext.AddCommandUsageAttributeRequest{ - Key: telemetry.AgentDeploymentModeAttribute, - Value: value, +func testExtension() *extensions.Extension { + return &extensions.Extension{ + Id: "azd.internal.telemetry", + Version: "1.0.0", + Source: extensions.MainRegistryName, + Capabilities: []extensions.CapabilityType{ + extensions.TelemetryCapability, + }, + Telemetry: []extensions.TelemetryFieldDeclaration{ + { + Key: testTelemetryKey, + AllowedValues: []string{"code", "container"}, + }, + }, } } -func TestTelemetryService_MissingClaims(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) +// callWith runs the handler as the given extension, mirroring how the auth +// interceptor injects host-signed claims. +func callWith( + t *testing.T, + extension *extensions.Extension, + req *azdext.ReportUsageAttributeRequest, +) (*azdext.ReportUsageAttributeResponse, error) { + t.Helper() + + service := newTelemetryService(stubExtensionLookup{extension}) + ctx := extensions.WithClaimsContext(t.Context(), &extensions.ExtensionClaims{ + RegisteredClaims: jwt.RegisteredClaims{Subject: extension.Id}, + Capabilities: extension.Capabilities, + Source: extension.Source, + }) + + return service.ReportUsageAttribute(ctx, req) +} + +func requireCode(t *testing.T, err error, expected codes.Code) { + t.Helper() - svc := NewTelemetryService() - _, err := svc.AddCommandUsageAttribute(context.Background(), validRequest("code")) - require.Equal(t, codes.Unauthenticated, status.Code(err)) + st, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, expected, st.Code()) } -func TestTelemetryService_MissingCapability(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) +func Test_TelemetryService_AcceptsDeclaredValue(t *testing.T) { + t.Parallel() + + resp, err := callWith(t, testExtension(), &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "container", + }) + + require.NoError(t, err) + require.True(t, resp.Accepted) +} + +func Test_TelemetryService_RequiresClaims(t *testing.T) { + t.Parallel() - scope := tracing.BeginCommandUsageScope("cmd.deploy") - t.Cleanup(func() { _, _ = tracing.CloseCommandUsageScope(scope) }) + service := newTelemetryService(stubExtensionLookup{testExtension()}) + _, err := service.ReportUsageAttribute(t.Context(), &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "code", + }) - svc := NewTelemetryService() - // Authenticated but without the service-target-provider capability. - _, err := svc.AddCommandUsageAttribute(claimsContext(), validRequest("code")) - require.Equal(t, codes.PermissionDenied, status.Code(err)) + requireCode(t, err, codes.Unauthenticated) } -func TestTelemetryService_InvalidArguments(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) - - svc := NewTelemetryService() - ctx := serviceTargetClaims() - - cases := map[string]*azdext.AddCommandUsageAttributeRequest{ - "nil request": nil, - "empty key": {Key: "", Value: "code"}, - "empty value": {Key: telemetry.AgentDeploymentModeAttribute, Value: ""}, - "oversize key": {Key: strings.Repeat("k", maxTelemetryFieldLength+1), Value: "code"}, - "oversize value": { - Key: telemetry.AgentDeploymentModeAttribute, - Value: strings.Repeat("v", maxTelemetryFieldLength+1), +func Test_TelemetryService_RejectsInvalidRequests(t *testing.T) { + t.Parallel() + + tests := map[string]*azdext.ReportUsageAttributeRequest{ + "nil": nil, + "empty key": {Key: "", Value: "code"}, + "empty value": {Key: testTelemetryKey, Value: ""}, + "long key": { + Key: strings.Repeat("k", extensions.MaxTelemetryKeyLength+1), + Value: "code", + }, + "long value": { + Key: testTelemetryKey, + Value: strings.Repeat("v", extensions.MaxTelemetryValueLength+1), }, - "unknown key": {Key: "some.other.key", Value: "code"}, - "invalid enum": validRequest("bogus"), } - for name, req := range cases { + for name, req := range tests { t.Run(name, func(t *testing.T) { - _, err := svc.AddCommandUsageAttribute(ctx, req) - require.Equal(t, codes.InvalidArgument, status.Code(err), "case %q", name) + t.Parallel() + + _, err := callWith(t, testExtension(), req) + requireCode(t, err, codes.InvalidArgument) }) } } -func TestTelemetryService_InvalidPolicyIsInternal(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) +func Test_TelemetryService_RejectsUnofficialSource(t *testing.T) { + t.Parallel() - ctx := serviceTargetClaims() - capSet := map[extensions.CapabilityType]struct{}{ - extensions.ServiceTargetProviderCapability: {}, - } + extension := testExtension() + extension.Source = "dev" - t.Run("empty allowed values", func(t *testing.T) { - svc := newTelemetryService(map[string]commandUsageFieldPolicy{ - telemetry.AgentDeploymentModeAttribute: { - key: fields.AgentDeploymentModeKey, - allowedValues: map[string]struct{}{}, - eligibleEvents: map[string]struct{}{"cmd.deploy": {}}, - requiredCapabilities: capSet, - }, - }) - _, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) - require.Equal(t, codes.Internal, status.Code(err)) + _, err := callWith(t, extension, &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "code", }) - t.Run("empty eligible events", func(t *testing.T) { - svc := newTelemetryService(map[string]commandUsageFieldPolicy{ - telemetry.AgentDeploymentModeAttribute: { - key: fields.AgentDeploymentModeKey, - allowedValues: map[string]struct{}{"code": {}}, - eligibleEvents: map[string]struct{}{}, - requiredCapabilities: capSet, - }, - }) - _, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) - require.Equal(t, codes.Internal, status.Code(err)) - }) + requireCode(t, err, codes.PermissionDenied) } -func TestTelemetryService_EligibleScopesAccept(t *testing.T) { - for _, eventName := range []string{"cmd.deploy", "cmd.up"} { - t.Run(eventName, func(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) +func Test_TelemetryService_RejectsMissingSource(t *testing.T) { + t.Parallel() - scope := tracing.BeginCommandUsageScope(eventName) - svc := NewTelemetryService() + // An install predating source tracking must fail closed rather than be + // treated as coming from the official registry. + extension := testExtension() + extension.Source = "" - resp, err := svc.AddCommandUsageAttribute(serviceTargetClaims(), validRequest("code")) - require.NoError(t, err) - require.True(t, resp.Accepted) + _, err := callWith(t, extension, &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "code", + }) - attrs, err := tracing.CloseCommandUsageScope(scope) - require.NoError(t, err) - require.Len(t, attrs, 1) - require.Equal(t, telemetry.AgentDeploymentModeAttribute, string(attrs[0].Key)) - require.Equal(t, []string{"code"}, attrs[0].Value.AsStringSlice()) - }) - } + requireCode(t, err, codes.PermissionDenied) } -func TestTelemetryService_IneligibleScopeNotAccepted(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) +func Test_TelemetryService_RejectsMissingCapability(t *testing.T) { + t.Parallel() - scope := tracing.BeginCommandUsageScope("cmd.package") - svc := NewTelemetryService() + extension := testExtension() + extension.Capabilities = []extensions.CapabilityType{ + extensions.CustomCommandCapability, + } - resp, err := svc.AddCommandUsageAttribute(serviceTargetClaims(), validRequest("code")) - require.NoError(t, err) - require.False(t, resp.Accepted) + _, err := callWith(t, extension, &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "code", + }) - attrs, err := tracing.CloseCommandUsageScope(scope) - require.NoError(t, err) - require.Empty(t, attrs) + requireCode(t, err, codes.PermissionDenied) } -func TestTelemetryService_NoActiveScopeNotAccepted(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) +func Test_TelemetryService_RejectsUninstalledExtension(t *testing.T) { + t.Parallel() - svc := NewTelemetryService() - resp, err := svc.AddCommandUsageAttribute(serviceTargetClaims(), validRequest("code")) - require.NoError(t, err) - require.False(t, resp.Accepted) -} + service := newTelemetryService(stubExtensionLookup{}) + ctx := extensions.WithClaimsContext(t.Context(), &extensions.ExtensionClaims{ + RegisteredClaims: jwt.RegisteredClaims{Subject: "azd.internal.telemetry"}, + Capabilities: []extensions.CapabilityType{extensions.TelemetryCapability}, + Source: extensions.MainRegistryName, + }) + + _, err := service.ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "code", + }) -func TestTelemetryService_DuplicateCollapses(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) + requireCode(t, err, codes.PermissionDenied) +} - scope := tracing.BeginCommandUsageScope("cmd.deploy") - svc := NewTelemetryService() - ctx := serviceTargetClaims() +func Test_TelemetryService_RejectsUndeclaredKeyOrValue(t *testing.T) { + t.Parallel() - for range 3 { - resp, err := svc.AddCommandUsageAttribute(ctx, validRequest("code")) - require.NoError(t, err) - require.True(t, resp.Accepted) + tests := map[string]*azdext.ReportUsageAttributeRequest{ + "undeclared key": {Key: "ext.azd.internal.telemetry.other", Value: "code"}, + "undeclared value": {Key: testTelemetryKey, Value: "byo_image"}, + "core key": {Key: "agent.deploy.mode", Value: "code"}, } - attrs, err := tracing.CloseCommandUsageScope(scope) - require.NoError(t, err) - require.Len(t, attrs, 1) - require.Equal(t, []string{"code"}, attrs[0].Value.AsStringSlice()) -} + for name, req := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() -func TestTelemetryService_ConcurrentReports(t *testing.T) { - tracing.ResetCommandUsageForTest() - t.Cleanup(tracing.ResetCommandUsageForTest) - - scope := tracing.BeginCommandUsageScope("cmd.up") - svc := NewTelemetryService() - ctx := serviceTargetClaims() - - modes := []string{"code", "container", "byo_image"} - var wg sync.WaitGroup - for i := range 60 { - value := modes[i%len(modes)] - wg.Go(func() { - _, err := svc.AddCommandUsageAttribute(ctx, validRequest(value)) - require.NoError(t, err) + _, err := callWith(t, testExtension(), req) + requireCode(t, err, codes.InvalidArgument) }) } - wg.Wait() - - attrs, err := tracing.CloseCommandUsageScope(scope) - require.NoError(t, err) - require.Len(t, attrs, 1) - require.ElementsMatch(t, modes, attrs[0].Value.AsStringSlice()) } -// TestExtensionUsageFieldsInvariants guards the production allowlist so a future -// field cannot accidentally open a free-form or unauthenticated path. -func TestExtensionUsageFieldsInvariants(t *testing.T) { +func Test_TelemetryService_RejectsTamperedDeclaration(t *testing.T) { t.Parallel() - require.NotEmpty(t, extensionUsageFields) - - for registryKey, policy := range extensionUsageFields { - require.NotEmpty(t, registryKey) - require.Equal(t, registryKey, string(policy.key.Key), - "policy key must match its registry key") - require.NotEmpty(t, string(policy.key.Classification), "classification must be set") - require.NotEmpty(t, string(policy.key.Purpose), "purpose must be set") - - require.NotEmpty(t, policy.allowedValues, "allowed values must be set") - for value := range policy.allowedValues { - require.NotEmpty(t, value, "allowed value must not be empty") - } + // The installed record lives in user config, so a locally widened + // declaration must be rejected instead of trusted. + extension := testExtension() + extension.Telemetry = []extensions.TelemetryFieldDeclaration{ + { + Key: "agent.deploy.mode", + AllowedValues: []string{"code"}, + }, + } - require.NotEmpty(t, policy.eligibleEvents, "eligible events must be set") - for event := range policy.eligibleEvents { - require.NotEmpty(t, event, "eligible event must not be empty") - } + _, err := callWith(t, extension, &azdext.ReportUsageAttributeRequest{ + Key: "agent.deploy.mode", + Value: "code", + }) - require.NotEmpty(t, policy.requiredCapabilities, "required capabilities must be set") - } + requireCode(t, err, codes.FailedPrecondition) } diff --git a/cli/azd/internal/grpcserver/trace_context.go b/cli/azd/internal/grpcserver/trace_context.go new file mode 100644 index 00000000000..b130b48fc15 --- /dev/null +++ b/cli/azd/internal/grpcserver/trace_context.go @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package grpcserver + +import ( + "context" + + "go.opentelemetry.io/otel/propagation" + "google.golang.org/grpc" + "google.golang.org/grpc/metadata" +) + +// W3C trace context metadata keys sent by the extension SDK. +const ( + traceparentHeader = "traceparent" + tracestateHeader = "tracestate" +) + +// traceContextInterceptor restores the caller's W3C trace context so spans the +// host records while serving an extension call join the command's trace +// instead of starting an unrelated root span. +func (s *Server) traceContextInterceptor() grpc.UnaryServerInterceptor { + return func( + ctx context.Context, + req any, + info *grpc.UnaryServerInfo, + handler grpc.UnaryHandler, + ) (any, error) { + return handler(withIncomingTraceContext(ctx), req) + } +} + +// traceContextStreamInterceptor is the streaming counterpart of +// traceContextInterceptor. +func (s *Server) traceContextStreamInterceptor() grpc.StreamServerInterceptor { + return func( + srv any, + ss grpc.ServerStream, + info *grpc.StreamServerInfo, + handler grpc.StreamHandler, + ) error { + return handler(srv, &contextStream{ + ServerStream: ss, + ctx: withIncomingTraceContext(ss.Context()), + }) + } +} + +// withIncomingTraceContext extracts W3C trace context from gRPC metadata. +// A missing or malformed traceparent leaves the context untouched, so calls +// from older extensions keep working. +func withIncomingTraceContext(ctx context.Context) context.Context { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return ctx + } + + parent := md.Get(traceparentHeader) + if len(parent) == 0 || parent[0] == "" { + return ctx + } + + carrier := propagation.MapCarrier{traceparentHeader: parent[0]} + if state := md.Get(tracestateHeader); len(state) > 0 { + carrier[tracestateHeader] = state[0] + } + + return propagation.TraceContext{}.Extract(ctx, carrier) +} diff --git a/cli/azd/internal/grpcserver/trace_context_test.go b/cli/azd/internal/grpcserver/trace_context_test.go new file mode 100644 index 00000000000..7ed8644a90e --- /dev/null +++ b/cli/azd/internal/grpcserver/trace_context_test.go @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package grpcserver + +import ( + "testing" + + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace" + "google.golang.org/grpc/metadata" +) + +const ( + testTraceId = "68f1c4f4ef5346e69d7f196761d10c68" + testSpanId = "7fbdc197a52f4825" +) + +func testTraceparent() string { + return "00-" + testTraceId + "-" + testSpanId + "-01" +} + +func Test_WithIncomingTraceContext_RestoresCallerTrace(t *testing.T) { + t.Parallel() + + md := metadata.Pairs( + traceparentHeader, testTraceparent(), + tracestateHeader, "vendor=value", + ) + ctx := metadata.NewIncomingContext(t.Context(), md) + + spanContext := trace.SpanContextFromContext(withIncomingTraceContext(ctx)) + + require.True(t, spanContext.IsValid()) + require.Equal(t, testTraceId, spanContext.TraceID().String()) + require.Equal(t, testSpanId, spanContext.SpanID().String()) + require.Equal(t, "vendor=value", spanContext.TraceState().String()) +} + +func Test_WithIncomingTraceContext_LeavesContextAlone(t *testing.T) { + t.Parallel() + + tests := map[string]metadata.MD{ + "no metadata": nil, + "no traceparent": metadata.Pairs("authorization", "token"), + "empty traceparent": metadata.Pairs(traceparentHeader, ""), + "malformed": metadata.Pairs(traceparentHeader, "not-a-traceparent"), + "unsupported format": metadata.Pairs(traceparentHeader, "99-"+testTraceId), + } + + for name, md := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + if md != nil { + ctx = metadata.NewIncomingContext(ctx, md) + } + + require.False(t, trace.SpanContextFromContext( + withIncomingTraceContext(ctx)).IsValid()) + }) + } +} diff --git a/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go b/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go index 610ee15b09b..3bf8c727722 100644 --- a/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go +++ b/cli/azd/internal/telemetry/appinsights-exporter/span_to_envelope_test.go @@ -169,25 +169,6 @@ func assertAttributeInPropertiesOrMeasurement( } } -// TestSpanToEnvelope_AgentDeployMode locks the App Insights representation of -// the agent.deploy.mode field. The logical OTel value is a string slice; the -// exporter stores it as JSON-encoded text in the Properties bag, so downstream -// Kusto must parse it as a dynamic array. -func TestSpanToEnvelope_AgentDeployMode(t *testing.T) { - t.Parallel() - - stub := getDefaultSpanStub() - stub.Attributes = []attribute.KeyValue{ - attribute.StringSlice("agent.deploy.mode", []string{"code", "container"}), - } - span := stub.Snapshot() - - envelope := SpanToEnvelope(span) - data := envelope.Data.(*contracts.Data).BaseData.(*contracts.RequestData) - - assert.Equal(t, `["code","container"]`, data.Properties["agent.deploy.mode"]) -} - func getDefaultSpanStub() tracetest.SpanStub { traceId, _ := trace.TraceIDFromHex("68f1c4f4ef5346e69d7f196761d10c68") spanId, _ := trace.SpanIDFromHex("7fbdc197a52f4825877ddd46e4ec7f6c") diff --git a/cli/azd/internal/tracing/command_usage.go b/cli/azd/internal/tracing/command_usage.go deleted file mode 100644 index 67b4bbfe33c..00000000000 --- a/cli/azd/internal/tracing/command_usage.go +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package tracing - -import ( - "fmt" - "slices" - "strings" - "sync" - - "go.opentelemetry.io/otel/attribute" -) - -// commandUsage is the process-global command usage scope stack. It mirrors the -// package-global usageVal store: a single process-wide instance reachable from -// the command middleware and from any gRPC handler goroutine without dependency -// injection. Extension-contributed command usage attributes are held here and -// attached to the owning command span when its scope closes. -var commandUsage = commandUsageStore{} - -type commandUsageStore struct { - mu sync.Mutex - nextID uint64 - scopes []*commandUsageScope -} - -type commandUsageScope struct { - id uint64 - eventName string - values map[attribute.Key]map[string]struct{} -} - -// CommandUsageScope is an opaque handle to a pushed command usage scope. The -// command middleware holds it between BeginCommandUsageScope and -// CloseCommandUsageScope. -type CommandUsageScope struct { - id uint64 -} - -// BeginCommandUsageScope pushes a new command usage scope for eventName and -// returns its handle. Every command telemetry middleware invocation begins -// exactly one scope, including nested child actions, so the top of the stack -// always identifies the command that owns any values reported while it runs. -func BeginCommandUsageScope(eventName string) CommandUsageScope { - commandUsage.mu.Lock() - defer commandUsage.mu.Unlock() - - commandUsage.nextID++ - id := commandUsage.nextID - commandUsage.scopes = append(commandUsage.scopes, &commandUsageScope{ - id: id, - eventName: eventName, - values: map[attribute.Key]map[string]struct{}{}, - }) - - return CommandUsageScope{id: id} -} - -// TryAppendCommandUsageUnique appends value to the current (top) scope when its -// exact event name is present in eligibleEvents. It returns false when no scope -// is active or the current scope is not eligible. It never falls back to an -// ancestor scope, so a value reported while an ineligible command (for example -// a synthetic child cmd.package) is on top is discarded rather than attributed -// to a parent command. -func TryAppendCommandUsageUnique( - eligibleEvents map[string]struct{}, - key attribute.Key, - value string, -) bool { - commandUsage.mu.Lock() - defer commandUsage.mu.Unlock() - - if len(commandUsage.scopes) == 0 { - return false - } - - top := commandUsage.scopes[len(commandUsage.scopes)-1] - if _, ok := eligibleEvents[top.eventName]; !ok { - return false - } - - set, ok := top.values[key] - if !ok { - set = map[string]struct{}{} - top.values[key] = set - } - set[value] = struct{}{} - - return true -} - -// CloseCommandUsageScope pops the scope identified by scope and returns its -// accumulated attributes as deterministically sorted string slices. It returns -// an error when the handle does not identify the current top scope, which -// covers a repeated or out-of-order close. On error the stack is left -// unchanged so a balanced close by the true owner still succeeds. -func CloseCommandUsageScope(scope CommandUsageScope) ([]attribute.KeyValue, error) { - commandUsage.mu.Lock() - defer commandUsage.mu.Unlock() - - n := len(commandUsage.scopes) - if n == 0 { - return nil, fmt.Errorf("no active command usage scope to close") - } - - top := commandUsage.scopes[n-1] - if top.id != scope.id { - return nil, fmt.Errorf( - "command usage scope is not the current scope; expected %d, got %d", - top.id, scope.id, - ) - } - - commandUsage.scopes = commandUsage.scopes[:n-1] - - keys := make([]attribute.Key, 0, len(top.values)) - for k := range top.values { - keys = append(keys, k) - } - slices.SortFunc(keys, func(a, b attribute.Key) int { - return strings.Compare(string(a), string(b)) - }) - - attrs := make([]attribute.KeyValue, 0, len(keys)) - for _, k := range keys { - valuesSet := top.values[k] - values := make([]string, 0, len(valuesSet)) - for v := range valuesSet { - values = append(values, v) - } - slices.Sort(values) - attrs = append(attrs, k.StringSlice(values)) - } - - return attrs, nil -} - -// ResetCommandUsageForTest clears all command usage scopes. Command usage state -// is process-global; tests that rely on it must not run with t.Parallel(). -func ResetCommandUsageForTest() { - commandUsage.mu.Lock() - defer commandUsage.mu.Unlock() - - commandUsage.scopes = nil - commandUsage.nextID = 0 -} diff --git a/cli/azd/internal/tracing/command_usage_test.go b/cli/azd/internal/tracing/command_usage_test.go deleted file mode 100644 index 52d8f4cfce7..00000000000 --- a/cli/azd/internal/tracing/command_usage_test.go +++ /dev/null @@ -1,220 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package tracing - -import ( - "sync" - "testing" - - "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel/attribute" -) - -const testUsageKey = attribute.Key("agent.deploy.mode") - -func testEligibleEvents() map[string]struct{} { - return map[string]struct{}{ - "cmd.deploy": {}, - "cmd.up": {}, - } -} - -// closeValues closes the scope and returns the string-slice value for the -// telemetry key, or nil when the key was not recorded. -func closeValues(t *testing.T, scope CommandUsageScope) []string { - t.Helper() - - attrs, err := CloseCommandUsageScope(scope) - require.NoError(t, err) - - for _, attr := range attrs { - if attr.Key == testUsageKey { - return attr.Value.AsStringSlice() - } - } - - return nil -} - -func TestCommandUsage_NoActiveScope(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - ok := TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code") - require.False(t, ok) -} - -func TestCommandUsage_EligibleDeployScope(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.deploy") - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - require.Equal(t, []string{"code"}, closeValues(t, scope)) -} - -func TestCommandUsage_EligibleUpScope(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.up") - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "container")) - require.Equal(t, []string{"container"}, closeValues(t, scope)) -} - -func TestCommandUsage_DuplicateValueCollapses(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.deploy") - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - require.Equal(t, []string{"code"}, closeValues(t, scope)) -} - -func TestCommandUsage_TwoValuesSortedUnique(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.up") - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "container")) - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - // Sorted regardless of insertion order. - require.Equal(t, []string{"code", "container"}, closeValues(t, scope)) -} - -func TestCommandUsage_ConcurrentValues(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.up") - - modes := []string{"code", "container", "byo_image"} - var wg sync.WaitGroup - for i := range 50 { - mode := modes[i%len(modes)] - wg.Go(func() { - TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, mode) - }) - } - wg.Wait() - - require.ElementsMatch(t, modes, closeValues(t, scope)) -} - -func TestCommandUsage_IneligiblePackageScope(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.package") - require.False(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - require.Nil(t, closeValues(t, scope)) -} - -func TestCommandUsage_NestedUpThenPackageNoFallback(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - up := BeginCommandUsageScope("cmd.up") - pkg := BeginCommandUsageScope("cmd.package") - - // A report while the ineligible child is on top must not fall back to up. - require.False(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - - require.Nil(t, closeValues(t, pkg)) - require.Nil(t, closeValues(t, up)) -} - -func TestCommandUsage_NestedUpThenDeployOwnsValue(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - up := BeginCommandUsageScope("cmd.up") - deploy := BeginCommandUsageScope("cmd.deploy") - - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "byo_image")) - - // The value lands only on the child deploy, never the parent up. - require.Equal(t, []string{"byo_image"}, closeValues(t, deploy)) - require.Nil(t, closeValues(t, up)) -} - -func TestCommandUsage_ChildCloseRestoresParentAsCurrent(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - up := BeginCommandUsageScope("cmd.up") - deploy := BeginCommandUsageScope("cmd.deploy") - _, err := CloseCommandUsageScope(deploy) - require.NoError(t, err) - - // After the child closes, up is current again and eligible. - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - require.Equal(t, []string{"code"}, closeValues(t, up)) -} - -func TestCommandUsage_CloseAfterActionErrorRemovesScope(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - // Simulate a command action that errored: the deferred close still runs. - scope := BeginCommandUsageScope("cmd.deploy") - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - _, err := CloseCommandUsageScope(scope) - require.NoError(t, err) - - // The next command sees no active scope. - require.False(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) -} - -func TestCommandUsage_RepeatedCloseErrors(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - scope := BeginCommandUsageScope("cmd.deploy") - _, err := CloseCommandUsageScope(scope) - require.NoError(t, err) - - _, err = CloseCommandUsageScope(scope) - require.Error(t, err) -} - -func TestCommandUsage_OutOfOrderCloseErrorsAndPreservesState(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - up := BeginCommandUsageScope("cmd.up") - deploy := BeginCommandUsageScope("cmd.deploy") - - // Closing the parent while the child is on top is rejected. - _, err := CloseCommandUsageScope(up) - require.Error(t, err) - - // State is intact: the child is still current and closes cleanly, then up. - _, err = CloseCommandUsageScope(deploy) - require.NoError(t, err) - _, err = CloseCommandUsageScope(up) - require.NoError(t, err) -} - -func TestCommandUsage_NextCommandHasNoLeakedValue(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - first := BeginCommandUsageScope("cmd.deploy") - require.True(t, TryAppendCommandUsageUnique(testEligibleEvents(), testUsageKey, "code")) - require.Equal(t, []string{"code"}, closeValues(t, first)) - - // A subsequent unrelated command starts empty. - second := BeginCommandUsageScope("cmd.deploy") - require.Nil(t, closeValues(t, second)) -} - -func TestCommandUsage_CloseWithNoScopeErrors(t *testing.T) { - ResetCommandUsageForTest() - t.Cleanup(ResetCommandUsageForTest) - - _, err := CloseCommandUsageScope(CommandUsageScope{id: 999}) - require.Error(t, err) -} diff --git a/cli/azd/internal/tracing/events/events.go b/cli/azd/internal/tracing/events/events.go index 5e89fb7cbf7..8c36bb2059b 100644 --- a/cli/azd/internal/tracing/events/events.go +++ b/cli/azd/internal/tracing/events/events.go @@ -32,6 +32,10 @@ const ( ExtensionUpgradeEvent = "ext.upgrade" // ExtensionPromoteEvent tracks a registry promotion (e.g., dev → main). ExtensionPromoteEvent = "ext.promote" + // ExtensionUsageEvent carries one usage attribute an extension reported + // through the telemetry service. The attribute key and value come from + // the extension's registry declaration, validated by the host. + ExtensionUsageEvent = "ext.usage" ) // Copilot agent related events. diff --git a/cli/azd/internal/tracing/fields/fields.go b/cli/azd/internal/tracing/fields/fields.go index 7da1ce090e6..de360f23974 100644 --- a/cli/azd/internal/tracing/fields/fields.go +++ b/cli/azd/internal/tracing/fields/fields.go @@ -8,8 +8,6 @@ import ( "github.com/microsoft/ApplicationInsights-Go/appinsights/contracts" "go.opentelemetry.io/otel/attribute" semconv "go.opentelemetry.io/otel/semconv/v1.39.0" - - "github.com/azure/azure-dev/cli/azd/pkg/azdext/telemetry" ) // AttributeKey represents an attribute key with additional metadata. @@ -263,17 +261,6 @@ var ( // Deployment attributes var ( - // AgentDeploymentModeKey records the de-duplicated set of hosted agent - // deployment modes selected during a command. Values are a fixed enum - // (code, container, byo_image) contributed by an authenticated extension - // through the host-validated telemetry service, so the field never carries - // user content, resource names, image references, paths, or identifiers. - AgentDeploymentModeKey = AttributeKey{ - Key: attribute.Key(telemetry.AgentDeploymentModeAttribute), - Classification: SystemMetadata, - Purpose: FeatureInsight, - } - // DeployAttemptKey tracks the retry attempt number for App Service zip deployments. DeployAttemptKey = AttributeKey{ Key: attribute.Key("deploy.appservice.attempt"), diff --git a/cli/azd/pkg/azdext/artifact_metadata.go b/cli/azd/pkg/azdext/artifact_metadata.go deleted file mode 100644 index 55a09245054..00000000000 --- a/cli/azd/pkg/azdext/artifact_metadata.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package azdext - -// ArtifactMetadataKeyFromPackage marks an artifact that was supplied through -// the --from-package option rather than produced by azd. This is deployment -// payload provenance, not telemetry. -const ArtifactMetadataKeyFromPackage = "azd.fromPackage" diff --git a/cli/azd/pkg/azdext/azd_client.go b/cli/azd/pkg/azdext/azd_client.go index 21907a09257..140b45ac2c1 100644 --- a/cli/azd/pkg/azdext/azd_client.go +++ b/cli/azd/pkg/azdext/azd_client.go @@ -9,6 +9,7 @@ import ( "os" "strings" + "go.opentelemetry.io/otel/propagation" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" @@ -16,6 +17,17 @@ import ( "google.golang.org/grpc/metadata" ) +// W3C trace context carriers. The gRPC metadata keys match the HTTP header +// names; the environment variables are what azd sets on the extension +// process. +const ( + traceparentHeader = "traceparent" + tracestateHeader = "tracestate" + + traceparentEnv = "TRACEPARENT" + tracestateEnv = "TRACESTATE" +) + type AzdClientOption func(*AzdClient) error // AzdClient is the client for the `azd` gRPC server. @@ -82,15 +94,47 @@ func isLocalhostAddress(address string) bool { return ip != nil && ip.IsLoopback() } -// WithAccessToken sets the access token for the `azd` client into a new Go context. +// WithAccessToken sets the access token for the `azd` client into a new Go +// context. It also forwards the W3C trace context so telemetry the host +// records while serving the call joins the azd command's trace instead of +// starting an unrelated one. func WithAccessToken(ctx context.Context, params ...string) context.Context { tokenValue := strings.Join(params, "") if tokenValue == "" { tokenValue = os.Getenv("AZD_ACCESS_TOKEN") } - md := metadata.Pairs("authorization", tokenValue) - return metadata.NewOutgoingContext(ctx, md) + pairs := append([]string{"authorization", tokenValue}, traceContextPairs(ctx)...) + + return metadata.NewOutgoingContext(ctx, metadata.Pairs(pairs...)) +} + +// traceContextPairs returns W3C trace context metadata pairs. A span already +// on ctx wins; otherwise the TRACEPARENT/TRACESTATE variables azd sets on the +// extension process are used, which is the common case because extensions +// build their context from scratch. +func traceContextPairs(ctx context.Context) []string { + carrier := propagation.MapCarrier{} + propagation.TraceContext{}.Inject(ctx, carrier) + + parent := carrier.Get(traceparentHeader) + state := carrier.Get(tracestateHeader) + + if parent == "" { + parent = os.Getenv(traceparentEnv) + state = os.Getenv(tracestateEnv) + } + + if parent == "" { + return nil + } + + pairs := []string{traceparentHeader, parent} + if state != "" { + pairs = append(pairs, tracestateHeader, state) + } + + return pairs } // NewAzdClient creates a new `azd` client. diff --git a/cli/azd/pkg/azdext/azd_client_test.go b/cli/azd/pkg/azdext/azd_client_test.go index 06bd5c46981..eb4b486a3f5 100644 --- a/cli/azd/pkg/azdext/azd_client_test.go +++ b/cli/azd/pkg/azdext/azd_client_test.go @@ -7,8 +7,65 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace" + "google.golang.org/grpc/metadata" ) +const ( + testTraceId = "68f1c4f4ef5346e69d7f196761d10c68" + testSpanId = "7fbdc197a52f4825" +) + +func testTraceparent() string { + return "00-" + testTraceId + "-" + testSpanId + "-01" +} + +func Test_WithAccessToken_PropagatesEnvTraceContext(t *testing.T) { + t.Setenv(traceparentEnv, testTraceparent()) + t.Setenv(tracestateEnv, "vendor=value") + + md, ok := metadata.FromOutgoingContext(WithAccessToken(t.Context(), "token")) + + require.True(t, ok) + require.Equal(t, []string{"token"}, md.Get("authorization")) + require.Equal(t, []string{testTraceparent()}, md.Get(traceparentHeader)) + require.Equal(t, []string{"vendor=value"}, md.Get(tracestateHeader)) +} + +func Test_WithAccessToken_PrefersContextSpan(t *testing.T) { + t.Setenv(traceparentEnv, testTraceparent()) + + traceId, err := trace.TraceIDFromHex("11111111111111111111111111111111") + require.NoError(t, err) + spanId, err := trace.SpanIDFromHex("2222222222222222") + require.NoError(t, err) + + ctx := trace.ContextWithSpanContext(t.Context(), trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceId, + SpanID: spanId, + TraceFlags: trace.FlagsSampled, + })) + + md, ok := metadata.FromOutgoingContext(WithAccessToken(ctx, "token")) + + require.True(t, ok) + require.Equal(t, []string{ + "00-11111111111111111111111111111111-2222222222222222-01", + }, md.Get(traceparentHeader)) +} + +func Test_WithAccessToken_OmitsMissingTraceContext(t *testing.T) { + t.Setenv(traceparentEnv, "") + t.Setenv(tracestateEnv, "") + + md, ok := metadata.FromOutgoingContext(WithAccessToken(t.Context(), "token")) + + require.True(t, ok) + require.Equal(t, []string{"token"}, md.Get("authorization")) + require.Empty(t, md.Get(traceparentHeader)) + require.Empty(t, md.Get(tracestateHeader)) +} + func Test_IsLocalhostAddress(t *testing.T) { tests := []struct { name string diff --git a/cli/azd/pkg/azdext/telemetry.pb.go b/cli/azd/pkg/azdext/telemetry.pb.go index c2a2c3a87aa..b6f655f1c60 100644 --- a/cli/azd/pkg/azdext/telemetry.pb.go +++ b/cli/azd/pkg/azdext/telemetry.pb.go @@ -24,30 +24,30 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) -type AddCommandUsageAttributeRequest struct { +type ReportUsageAttributeRequest struct { state protoimpl.MessageState `protogen:"open.v1"` - // Key must be registered and owned by the host. + // Key must match a telemetry field the extension declared in the registry. Key string `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"` - // Value must be in the host-owned allowed set for key. + // Value must be in the allowed set the extension declared for key. Value string `protobuf:"bytes,2,opt,name=value,proto3" json:"value,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } -func (x *AddCommandUsageAttributeRequest) Reset() { - *x = AddCommandUsageAttributeRequest{} +func (x *ReportUsageAttributeRequest) Reset() { + *x = ReportUsageAttributeRequest{} mi := &file_telemetry_proto_msgTypes[0] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } -func (x *AddCommandUsageAttributeRequest) String() string { +func (x *ReportUsageAttributeRequest) String() string { return protoimpl.X.MessageStringOf(x) } -func (*AddCommandUsageAttributeRequest) ProtoMessage() {} +func (*ReportUsageAttributeRequest) ProtoMessage() {} -func (x *AddCommandUsageAttributeRequest) ProtoReflect() protoreflect.Message { +func (x *ReportUsageAttributeRequest) ProtoReflect() protoreflect.Message { mi := &file_telemetry_proto_msgTypes[0] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) @@ -59,48 +59,47 @@ func (x *AddCommandUsageAttributeRequest) ProtoReflect() protoreflect.Message { return mi.MessageOf(x) } -// Deprecated: Use AddCommandUsageAttributeRequest.ProtoReflect.Descriptor instead. -func (*AddCommandUsageAttributeRequest) Descriptor() ([]byte, []int) { +// Deprecated: Use ReportUsageAttributeRequest.ProtoReflect.Descriptor instead. +func (*ReportUsageAttributeRequest) Descriptor() ([]byte, []int) { return file_telemetry_proto_rawDescGZIP(), []int{0} } -func (x *AddCommandUsageAttributeRequest) GetKey() string { +func (x *ReportUsageAttributeRequest) GetKey() string { if x != nil { return x.Key } return "" } -func (x *AddCommandUsageAttributeRequest) GetValue() string { +func (x *ReportUsageAttributeRequest) GetValue() string { if x != nil { return x.Value } return "" } -type AddCommandUsageAttributeResponse struct { +type ReportUsageAttributeResponse struct { state protoimpl.MessageState `protogen:"open.v1"` - // Accepted is false when the value was rejected because no eligible command - // scope is currently active. A false response is not an error. + // Accepted reports whether the host recorded the value. Accepted bool `protobuf:"varint,1,opt,name=accepted,proto3" json:"accepted,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } -func (x *AddCommandUsageAttributeResponse) Reset() { - *x = AddCommandUsageAttributeResponse{} +func (x *ReportUsageAttributeResponse) Reset() { + *x = ReportUsageAttributeResponse{} mi := &file_telemetry_proto_msgTypes[1] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } -func (x *AddCommandUsageAttributeResponse) String() string { +func (x *ReportUsageAttributeResponse) String() string { return protoimpl.X.MessageStringOf(x) } -func (*AddCommandUsageAttributeResponse) ProtoMessage() {} +func (*ReportUsageAttributeResponse) ProtoMessage() {} -func (x *AddCommandUsageAttributeResponse) ProtoReflect() protoreflect.Message { +func (x *ReportUsageAttributeResponse) ProtoReflect() protoreflect.Message { mi := &file_telemetry_proto_msgTypes[1] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) @@ -112,12 +111,12 @@ func (x *AddCommandUsageAttributeResponse) ProtoReflect() protoreflect.Message { return mi.MessageOf(x) } -// Deprecated: Use AddCommandUsageAttributeResponse.ProtoReflect.Descriptor instead. -func (*AddCommandUsageAttributeResponse) Descriptor() ([]byte, []int) { +// Deprecated: Use ReportUsageAttributeResponse.ProtoReflect.Descriptor instead. +func (*ReportUsageAttributeResponse) Descriptor() ([]byte, []int) { return file_telemetry_proto_rawDescGZIP(), []int{1} } -func (x *AddCommandUsageAttributeResponse) GetAccepted() bool { +func (x *ReportUsageAttributeResponse) GetAccepted() bool { if x != nil { return x.Accepted } @@ -128,14 +127,14 @@ var File_telemetry_proto protoreflect.FileDescriptor const file_telemetry_proto_rawDesc = "" + "\n" + - "\x0ftelemetry.proto\x12\x06azdext\"I\n" + - "\x1fAddCommandUsageAttributeRequest\x12\x10\n" + + "\x0ftelemetry.proto\x12\x06azdext\"E\n" + + "\x1bReportUsageAttributeRequest\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value\">\n" + - " AddCommandUsageAttributeResponse\x12\x1a\n" + - "\baccepted\x18\x01 \x01(\bR\baccepted2\x81\x01\n" + - "\x10TelemetryService\x12m\n" + - "\x18AddCommandUsageAttribute\x12'.azdext.AddCommandUsageAttributeRequest\x1a(.azdext.AddCommandUsageAttributeResponseB/Z-github.com/azure/azure-dev/cli/azd/pkg/azdextb\x06proto3" + "\x05value\x18\x02 \x01(\tR\x05value\":\n" + + "\x1cReportUsageAttributeResponse\x12\x1a\n" + + "\baccepted\x18\x01 \x01(\bR\baccepted2u\n" + + "\x10TelemetryService\x12a\n" + + "\x14ReportUsageAttribute\x12#.azdext.ReportUsageAttributeRequest\x1a$.azdext.ReportUsageAttributeResponseB/Z-github.com/azure/azure-dev/cli/azd/pkg/azdextb\x06proto3" var ( file_telemetry_proto_rawDescOnce sync.Once @@ -151,12 +150,12 @@ func file_telemetry_proto_rawDescGZIP() []byte { var file_telemetry_proto_msgTypes = make([]protoimpl.MessageInfo, 2) var file_telemetry_proto_goTypes = []any{ - (*AddCommandUsageAttributeRequest)(nil), // 0: azdext.AddCommandUsageAttributeRequest - (*AddCommandUsageAttributeResponse)(nil), // 1: azdext.AddCommandUsageAttributeResponse + (*ReportUsageAttributeRequest)(nil), // 0: azdext.ReportUsageAttributeRequest + (*ReportUsageAttributeResponse)(nil), // 1: azdext.ReportUsageAttributeResponse } var file_telemetry_proto_depIdxs = []int32{ - 0, // 0: azdext.TelemetryService.AddCommandUsageAttribute:input_type -> azdext.AddCommandUsageAttributeRequest - 1, // 1: azdext.TelemetryService.AddCommandUsageAttribute:output_type -> azdext.AddCommandUsageAttributeResponse + 0, // 0: azdext.TelemetryService.ReportUsageAttribute:input_type -> azdext.ReportUsageAttributeRequest + 1, // 1: azdext.TelemetryService.ReportUsageAttribute:output_type -> azdext.ReportUsageAttributeResponse 1, // [1:2] is the sub-list for method output_type 0, // [0:1] is the sub-list for method input_type 0, // [0:0] is the sub-list for extension type_name diff --git a/cli/azd/pkg/azdext/telemetry/attributes.go b/cli/azd/pkg/azdext/telemetry/attributes.go deleted file mode 100644 index 47f748d8895..00000000000 --- a/cli/azd/pkg/azdext/telemetry/attributes.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -// Package telemetry defines the extension telemetry contract values shared by -// the azd host and extensions. Keeping the key and the allowed enum values in -// one dependency-free package prevents core and extension code from duplicating -// telemetry string literals. -package telemetry - -// AgentDeploymentModeAttribute is the command-level usage attribute that -// records the set of hosted agent deployment modes selected during a command. -// Its value is a de-duplicated string slice. -const AgentDeploymentModeAttribute = "agent.deploy.mode" - -// AgentDeploymentMode identifies the selected hosted agent deployment source. -type AgentDeploymentMode string - -const ( - // AgentDeploymentModeCode deploys a source archive. - AgentDeploymentModeCode AgentDeploymentMode = "code" - - // AgentDeploymentModeContainer deploys a container image that azd builds - // and publishes from the project source. - AgentDeploymentModeContainer AgentDeploymentMode = "container" - - // AgentDeploymentModeByoImage deploys a caller-supplied image. - AgentDeploymentModeByoImage AgentDeploymentMode = "byo_image" -) diff --git a/cli/azd/pkg/azdext/telemetry/attributes_test.go b/cli/azd/pkg/azdext/telemetry/attributes_test.go deleted file mode 100644 index e0ca411e0ab..00000000000 --- a/cli/azd/pkg/azdext/telemetry/attributes_test.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package telemetry - -import "testing" - -// TestContractValues locks the public wire values. Downstream telemetry -// pipelines and the host allowlist depend on these exact strings, so a change -// here is a breaking telemetry contract change and must be intentional. -func TestContractValues(t *testing.T) { - t.Parallel() - - if AgentDeploymentModeAttribute != "agent.deploy.mode" { - t.Fatalf("unexpected attribute key: %q", AgentDeploymentModeAttribute) - } - - cases := map[AgentDeploymentMode]string{ - AgentDeploymentModeCode: "code", - AgentDeploymentModeContainer: "container", - AgentDeploymentModeByoImage: "byo_image", - } - for mode, want := range cases { - if string(mode) != want { - t.Errorf("unexpected mode value: got %q, want %q", string(mode), want) - } - } -} diff --git a/cli/azd/pkg/azdext/telemetry_grpc.pb.go b/cli/azd/pkg/azdext/telemetry_grpc.pb.go index d93398ee252..75851425422 100644 --- a/cli/azd/pkg/azdext/telemetry_grpc.pb.go +++ b/cli/azd/pkg/azdext/telemetry_grpc.pb.go @@ -22,22 +22,21 @@ import ( const _ = grpc.SupportPackageIsVersion9 const ( - TelemetryService_AddCommandUsageAttribute_FullMethodName = "/azdext.TelemetryService/AddCommandUsageAttribute" + TelemetryService_ReportUsageAttribute_FullMethodName = "/azdext.TelemetryService/ReportUsageAttribute" ) // TelemetryServiceClient is the client API for TelemetryService service. // // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. // -// TelemetryService accepts reviewed, host-owned command usage attribute values -// from authenticated extensions. The host validates the key and value against -// an allowlist and attaches accepted values to the current command telemetry -// event. Extensions cannot choose classification, purpose, hashing, or -// aggregation. +// TelemetryService accepts usage attribute values from authenticated +// extensions. Every key and value must appear in the telemetry declaration +// the extension published in the official azd registry, which is where those +// fields are reviewed. The host decides which telemetry span a value lands on +// and owns classification, purpose, hashing, and aggregation. type TelemetryServiceClient interface { - // AddCommandUsageAttribute adds one bounded value to a host-owned command - // usage attribute. Duplicate values are collapsed by the host. - AddCommandUsageAttribute(ctx context.Context, in *AddCommandUsageAttributeRequest, opts ...grpc.CallOption) (*AddCommandUsageAttributeResponse, error) + // ReportUsageAttribute reports one declared usage attribute value. + ReportUsageAttribute(ctx context.Context, in *ReportUsageAttributeRequest, opts ...grpc.CallOption) (*ReportUsageAttributeResponse, error) } type telemetryServiceClient struct { @@ -48,10 +47,10 @@ func NewTelemetryServiceClient(cc grpc.ClientConnInterface) TelemetryServiceClie return &telemetryServiceClient{cc} } -func (c *telemetryServiceClient) AddCommandUsageAttribute(ctx context.Context, in *AddCommandUsageAttributeRequest, opts ...grpc.CallOption) (*AddCommandUsageAttributeResponse, error) { +func (c *telemetryServiceClient) ReportUsageAttribute(ctx context.Context, in *ReportUsageAttributeRequest, opts ...grpc.CallOption) (*ReportUsageAttributeResponse, error) { cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) - out := new(AddCommandUsageAttributeResponse) - err := c.cc.Invoke(ctx, TelemetryService_AddCommandUsageAttribute_FullMethodName, in, out, cOpts...) + out := new(ReportUsageAttributeResponse) + err := c.cc.Invoke(ctx, TelemetryService_ReportUsageAttribute_FullMethodName, in, out, cOpts...) if err != nil { return nil, err } @@ -62,15 +61,14 @@ func (c *telemetryServiceClient) AddCommandUsageAttribute(ctx context.Context, i // All implementations must embed UnimplementedTelemetryServiceServer // for forward compatibility. // -// TelemetryService accepts reviewed, host-owned command usage attribute values -// from authenticated extensions. The host validates the key and value against -// an allowlist and attaches accepted values to the current command telemetry -// event. Extensions cannot choose classification, purpose, hashing, or -// aggregation. +// TelemetryService accepts usage attribute values from authenticated +// extensions. Every key and value must appear in the telemetry declaration +// the extension published in the official azd registry, which is where those +// fields are reviewed. The host decides which telemetry span a value lands on +// and owns classification, purpose, hashing, and aggregation. type TelemetryServiceServer interface { - // AddCommandUsageAttribute adds one bounded value to a host-owned command - // usage attribute. Duplicate values are collapsed by the host. - AddCommandUsageAttribute(context.Context, *AddCommandUsageAttributeRequest) (*AddCommandUsageAttributeResponse, error) + // ReportUsageAttribute reports one declared usage attribute value. + ReportUsageAttribute(context.Context, *ReportUsageAttributeRequest) (*ReportUsageAttributeResponse, error) mustEmbedUnimplementedTelemetryServiceServer() } @@ -81,8 +79,8 @@ type TelemetryServiceServer interface { // pointer dereference when methods are called. type UnimplementedTelemetryServiceServer struct{} -func (UnimplementedTelemetryServiceServer) AddCommandUsageAttribute(context.Context, *AddCommandUsageAttributeRequest) (*AddCommandUsageAttributeResponse, error) { - return nil, status.Errorf(codes.Unimplemented, "method AddCommandUsageAttribute not implemented") +func (UnimplementedTelemetryServiceServer) ReportUsageAttribute(context.Context, *ReportUsageAttributeRequest) (*ReportUsageAttributeResponse, error) { + return nil, status.Errorf(codes.Unimplemented, "method ReportUsageAttribute not implemented") } func (UnimplementedTelemetryServiceServer) mustEmbedUnimplementedTelemetryServiceServer() {} func (UnimplementedTelemetryServiceServer) testEmbeddedByValue() {} @@ -105,20 +103,20 @@ func RegisterTelemetryServiceServer(s grpc.ServiceRegistrar, srv TelemetryServic s.RegisterService(&TelemetryService_ServiceDesc, srv) } -func _TelemetryService_AddCommandUsageAttribute_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { - in := new(AddCommandUsageAttributeRequest) +func _TelemetryService_ReportUsageAttribute_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(ReportUsageAttributeRequest) if err := dec(in); err != nil { return nil, err } if interceptor == nil { - return srv.(TelemetryServiceServer).AddCommandUsageAttribute(ctx, in) + return srv.(TelemetryServiceServer).ReportUsageAttribute(ctx, in) } info := &grpc.UnaryServerInfo{ Server: srv, - FullMethod: TelemetryService_AddCommandUsageAttribute_FullMethodName, + FullMethod: TelemetryService_ReportUsageAttribute_FullMethodName, } handler := func(ctx context.Context, req interface{}) (interface{}, error) { - return srv.(TelemetryServiceServer).AddCommandUsageAttribute(ctx, req.(*AddCommandUsageAttributeRequest)) + return srv.(TelemetryServiceServer).ReportUsageAttribute(ctx, req.(*ReportUsageAttributeRequest)) } return interceptor(ctx, in, info, handler) } @@ -131,8 +129,8 @@ var TelemetryService_ServiceDesc = grpc.ServiceDesc{ HandlerType: (*TelemetryServiceServer)(nil), Methods: []grpc.MethodDesc{ { - MethodName: "AddCommandUsageAttribute", - Handler: _TelemetryService_AddCommandUsageAttribute_Handler, + MethodName: "ReportUsageAttribute", + Handler: _TelemetryService_ReportUsageAttribute_Handler, }, }, Streams: []grpc.StreamDesc{}, diff --git a/cli/azd/pkg/extensions/claims.go b/cli/azd/pkg/extensions/claims.go index 1d302388d8a..2e0b69d5106 100644 --- a/cli/azd/pkg/extensions/claims.go +++ b/cli/azd/pkg/extensions/claims.go @@ -14,6 +14,9 @@ import ( type ExtensionClaims struct { jwt.RegisteredClaims Capabilities []CapabilityType `json:"cap,omitempty"` + // Source is the registry the extension was installed from. It is signed by + // the host so an extension cannot claim a more trusted origin than it has. + Source string `json:"src,omitempty"` } // extensionClaimsKeyType is the context key for storing validated extension claims. diff --git a/cli/azd/pkg/extensions/extension.go b/cli/azd/pkg/extensions/extension.go index 4fa288bf957..19113ff24af 100644 --- a/cli/azd/pkg/extensions/extension.go +++ b/cli/azd/pkg/extensions/extension.go @@ -27,6 +27,9 @@ type Extension struct { Providers []Provider `json:"providers,omitempty"` McpConfig *McpConfig `json:"mcp,omitempty"` LastUpdateWarning string `json:"lastUpdateWarning,omitempty"` + // Telemetry are the usage attributes this version declared in the registry + // it was installed from. azd validates every reported value against it. + Telemetry []TelemetryFieldDeclaration `json:"telemetry,omitempty"` stdin *bytes.Buffer stdout *output.DynamicMultiWriter diff --git a/cli/azd/pkg/extensions/manager.go b/cli/azd/pkg/extensions/manager.go index 65fae091afc..0c799e32358 100644 --- a/cli/azd/pkg/extensions/manager.go +++ b/cli/azd/pkg/extensions/manager.go @@ -752,6 +752,7 @@ func (m *Manager) installInternal( Source: extension.Source, Providers: selectedVersion.Providers, McpConfig: selectedVersion.McpConfig, + Telemetry: selectedVersion.Telemetry, } if err := m.userConfig.Set(installedConfigKey, extensions); err != nil { diff --git a/cli/azd/pkg/extensions/registry.go b/cli/azd/pkg/extensions/registry.go index 18427236f68..bc9b9d96042 100644 --- a/cli/azd/pkg/extensions/registry.go +++ b/cli/azd/pkg/extensions/registry.go @@ -57,6 +57,10 @@ const ( // Validation provider enables extensions to contribute validation checks // to azd's validation pipeline (e.g. provision checks during provisioning) ValidationProviderCapability CapabilityType = "validation-provider" + // Telemetry enables extensions to report the usage attributes they declare + // in the official registry. azd validates every reported key and value + // against that declaration. + TelemetryCapability CapabilityType = "telemetry" ) type ProviderType string @@ -139,6 +143,9 @@ type ExtensionVersion struct { EntryPoint string `json:"entryPoint,omitempty"` // McpConfig is the MCP server configuration for this extension version McpConfig *McpConfig `json:"mcp,omitempty"` + // Telemetry declares the usage attributes this version may report through + // the telemetry service. Requires the telemetry capability. + Telemetry []TelemetryFieldDeclaration `json:"telemetry,omitempty"` } // ExtensionArtifact represents the artifact information of an extension diff --git a/cli/azd/pkg/extensions/telemetry_declaration.go b/cli/azd/pkg/extensions/telemetry_declaration.go new file mode 100644 index 00000000000..411af698522 --- /dev/null +++ b/cli/azd/pkg/extensions/telemetry_declaration.go @@ -0,0 +1,163 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package extensions + +import ( + "fmt" + "regexp" + "strings" +) + +// Bounds for extension-declared telemetry fields. They keep registry entries +// small and are re-checked at runtime so a tampered local install cannot +// widen them. +const ( + // MaxTelemetryFields is the number of fields one extension version may + // declare. + MaxTelemetryFields = 16 + // MaxTelemetryKeyLength bounds a declared attribute key. + MaxTelemetryKeyLength = 128 + // MaxTelemetryAllowedValues bounds the closed value set of one field. + MaxTelemetryAllowedValues = 32 + // MaxTelemetryValueLength bounds a single declared value. + MaxTelemetryValueLength = 64 +) + +// TelemetryKeyPrefix namespaces every extension-declared telemetry key so +// extension values can never collide with, or masquerade as, a core field. +const TelemetryKeyPrefix = "ext." + +// telemetryKeySegmentRegex matches one dot-separated segment of a key. +var telemetryKeySegmentRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`) + +// telemetryValueRegex bounds declared values to a conservative charset. It +// deliberately excludes ':' and '/' so a declaration cannot smuggle registry +// names, URLs, or paths into a value. +var telemetryValueRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9_.-]*[a-z0-9])?$`) + +// TelemetryFieldDeclaration declares one usage attribute that an extension +// version may report at runtime. The registry entry is the source of truth, +// so azd core carries no product-specific telemetry semantics: adding a field +// is a registry change, not a core release. +type TelemetryFieldDeclaration struct { + // Key is the telemetry attribute name. It must be "ext." followed by the + // declaring extension id and at least one more dot-separated segment. + Key string `json:"key"` + // AllowedValues is the closed set of values the extension may report for + // Key. Free-form values are never accepted. + AllowedValues []string `json:"allowedValues"` +} + +// TelemetryKeyPrefixFor returns the key prefix an extension must use for +// every telemetry field it declares. +func TelemetryKeyPrefixFor(extensionId string) string { + return TelemetryKeyPrefix + extensionId + "." +} + +// ValidateTelemetryDeclarations reports every way declarations violate the +// host telemetry contract: namespaced keys, bounded closed value sets, and a +// conservative charset. It runs at publish time against the registry and +// again at runtime against the installed record, so it must stay free of any +// product-specific knowledge. +func ValidateTelemetryDeclarations( + extensionId string, + declarations []TelemetryFieldDeclaration, +) []string { + if len(declarations) == 0 { + return nil + } + + var issues []string + + if len(declarations) > MaxTelemetryFields { + issues = append(issues, fmt.Sprintf( + "telemetry declares %d fields (max %d)", len(declarations), MaxTelemetryFields)) + } + + prefix := TelemetryKeyPrefixFor(extensionId) + seenKeys := map[string]struct{}{} + + for i, declaration := range declarations { + at := fmt.Sprintf("telemetry[%d]", i) + + if _, duplicate := seenKeys[declaration.Key]; duplicate { + issues = append(issues, fmt.Sprintf("%s: duplicate key '%s'", at, declaration.Key)) + } + seenKeys[declaration.Key] = struct{}{} + + issues = append(issues, validateTelemetryKey(at, prefix, declaration.Key)...) + issues = append(issues, validateTelemetryValues(at, declaration.AllowedValues)...) + } + + return issues +} + +func validateTelemetryKey(at string, prefix string, key string) []string { + if key == "" { + return []string{fmt.Sprintf("%s: missing required field 'key'", at)} + } + + if len(key) > MaxTelemetryKeyLength { + return []string{fmt.Sprintf( + "%s: key exceeds %d characters", at, MaxTelemetryKeyLength)} + } + + if !strings.HasPrefix(key, prefix) { + return []string{fmt.Sprintf( + "%s: key '%s' must start with '%s'", at, key, prefix)} + } + + suffix := strings.TrimPrefix(key, prefix) + if suffix == "" { + return []string{fmt.Sprintf( + "%s: key '%s' needs at least one segment after '%s'", at, key, prefix)} + } + + for segment := range strings.SplitSeq(suffix, ".") { + if !telemetryKeySegmentRegex.MatchString(segment) { + return []string{fmt.Sprintf( + "%s: key '%s' has invalid segment '%s' "+ + "(lowercase alphanumeric and hyphens)", at, key, segment)} + } + } + + return nil +} + +func validateTelemetryValues(at string, values []string) []string { + if len(values) == 0 { + return []string{fmt.Sprintf("%s: allowedValues must declare at least one value", at)} + } + + var issues []string + + if len(values) > MaxTelemetryAllowedValues { + issues = append(issues, fmt.Sprintf( + "%s: declares %d allowed values (max %d)", + at, len(values), MaxTelemetryAllowedValues)) + } + + seen := map[string]struct{}{} + + for _, value := range values { + if _, duplicate := seen[value]; duplicate { + issues = append(issues, fmt.Sprintf("%s: duplicate allowed value '%s'", at, value)) + } + seen[value] = struct{}{} + + if len(value) > MaxTelemetryValueLength { + issues = append(issues, fmt.Sprintf( + "%s: allowed value exceeds %d characters", at, MaxTelemetryValueLength)) + continue + } + + if !telemetryValueRegex.MatchString(value) { + issues = append(issues, fmt.Sprintf( + "%s: invalid allowed value '%s' "+ + "(lowercase alphanumeric, '_', '-', and '.')", at, value)) + } + } + + return issues +} diff --git a/cli/azd/pkg/extensions/telemetry_declaration_test.go b/cli/azd/pkg/extensions/telemetry_declaration_test.go new file mode 100644 index 00000000000..0b25cbc72f8 --- /dev/null +++ b/cli/azd/pkg/extensions/telemetry_declaration_test.go @@ -0,0 +1,198 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package extensions + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +const testExtensionId = "azure.ai.agents" + +func Test_ValidateTelemetryDeclarations_Valid(t *testing.T) { + t.Parallel() + + issues := ValidateTelemetryDeclarations(testExtensionId, []TelemetryFieldDeclaration{ + { + Key: "ext.azure.ai.agents.deploy.mode", + AllowedValues: []string{"code", "container", "byo_image"}, + }, + { + Key: "ext.azure.ai.agents.deploy-target", + AllowedValues: []string{"aca"}, + }, + }) + + require.Empty(t, issues) +} + +func Test_ValidateTelemetryDeclarations_Empty(t *testing.T) { + t.Parallel() + + require.Empty(t, ValidateTelemetryDeclarations(testExtensionId, nil)) +} + +func Test_ValidateTelemetryDeclarations_RejectsBadKeys(t *testing.T) { + t.Parallel() + + tests := map[string]string{ + "missing key": "", + "core namespace": "agent.deploy.mode", + "other extension": "ext.contoso.tools.deploy.mode", + "prefix only": "ext.azure.ai.agents.", + "uppercase segment": "ext.azure.ai.agents.Mode", + "empty segment": "ext.azure.ai.agents..mode", + "too long": "ext.azure.ai.agents." + strings.Repeat("m", MaxTelemetryKeyLength), + } + + for name, key := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + issues := ValidateTelemetryDeclarations(testExtensionId, []TelemetryFieldDeclaration{ + {Key: key, AllowedValues: []string{"code"}}, + }) + + require.NotEmpty(t, issues) + }) + } +} + +func Test_ValidateTelemetryDeclarations_RejectsBadValues(t *testing.T) { + t.Parallel() + + tests := map[string][]string{ + "no values": {}, + "empty value": {""}, + "registry name": {"byo_image:myregistry.azurecr.io/foo"}, + "path": {"images/foo"}, + "uppercase": {"Code"}, + "duplicate": {"code", "code"}, + "too long": {strings.Repeat("v", MaxTelemetryValueLength+1)}, + "too many entries": make([]string, MaxTelemetryAllowedValues+1), + } + + for name, values := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + issues := ValidateTelemetryDeclarations(testExtensionId, []TelemetryFieldDeclaration{ + {Key: "ext.azure.ai.agents.deploy.mode", AllowedValues: values}, + }) + + require.NotEmpty(t, issues) + }) + } +} + +func Test_ValidateTelemetryDeclarations_RejectsOversizedSets(t *testing.T) { + t.Parallel() + + declarations := make([]TelemetryFieldDeclaration, 0, MaxTelemetryFields+1) + for i := range MaxTelemetryFields + 1 { + declarations = append(declarations, TelemetryFieldDeclaration{ + Key: "ext.azure.ai.agents.field" + string(rune('a'+i)), + AllowedValues: []string{"code"}, + }) + } + + require.NotEmpty(t, ValidateTelemetryDeclarations(testExtensionId, declarations)) +} + +func Test_ValidateTelemetryDeclarations_RejectsDuplicateKeys(t *testing.T) { + t.Parallel() + + issues := ValidateTelemetryDeclarations(testExtensionId, []TelemetryFieldDeclaration{ + {Key: "ext.azure.ai.agents.deploy.mode", AllowedValues: []string{"code"}}, + {Key: "ext.azure.ai.agents.deploy.mode", AllowedValues: []string{"container"}}, + }) + + require.NotEmpty(t, issues) +} + +func Test_ValidateRegistry_TelemetryRequiresCapability(t *testing.T) { + t.Parallel() + + metadata := []*ExtensionMetadata{ + { + Id: testExtensionId, + DisplayName: "Agents", + Description: "Test extension", + Versions: []ExtensionVersion{ + { + Version: "1.0.0", + Telemetry: []TelemetryFieldDeclaration{ + { + Key: "ext.azure.ai.agents.deploy.mode", + AllowedValues: []string{"code"}, + }, + }, + Artifacts: map[string]ExtensionArtifact{ + "linux/amd64": { + URL: "https://example.com/agents.zip", + Checksum: ExtensionChecksum{Algorithm: "sha256", Value: "abc"}, + }, + }, + }, + }, + }, + } + + result := ValidateExtensions(metadata, false) + + require.False(t, result.Valid) + require.Contains(t, strings.Join(collectMessages(result), "\n"), "telemetry") +} + +func Test_ValidateRegistry_TelemetryValidWithCapability(t *testing.T) { + t.Parallel() + + metadata := []*ExtensionMetadata{ + { + Id: testExtensionId, + DisplayName: "Agents", + Description: "Test extension", + Versions: []ExtensionVersion{ + { + Version: "1.0.0", + Capabilities: []CapabilityType{TelemetryCapability}, + Telemetry: []TelemetryFieldDeclaration{ + { + Key: "ext.azure.ai.agents.deploy.mode", + AllowedValues: []string{"code"}, + }, + }, + Artifacts: map[string]ExtensionArtifact{ + "linux/amd64": { + URL: "https://example.com/agents.zip", + Checksum: ExtensionChecksum{Algorithm: "sha256", Value: "abc"}, + }, + }, + }, + }, + }, + } + + result := ValidateExtensions(metadata, false) + + require.True(t, result.Valid, strings.Join(collectMessages(result), "\n")) +} + +func collectMessages(result *RegistryValidationResult) []string { + messages := []string{} + + for _, issue := range result.Issues { + messages = append(messages, issue.Message) + } + + for _, extension := range result.Extensions { + for _, issue := range extension.Issues { + messages = append(messages, issue.Message) + } + } + + return messages +} diff --git a/cli/azd/pkg/extensions/validate_registry.go b/cli/azd/pkg/extensions/validate_registry.go index ed00edda141..985f129f7d8 100644 --- a/cli/azd/pkg/extensions/validate_registry.go +++ b/cli/azd/pkg/extensions/validate_registry.go @@ -32,6 +32,7 @@ var ValidCapabilities = []CapabilityType{ MetadataCapability, ProvisioningProviderCapability, ValidationProviderCapability, + TelemetryCapability, } // validChecksumAlgorithms defines the supported checksum algorithms. @@ -218,7 +219,7 @@ func validateExtension(ext *ExtensionMetadata, strict bool) ExtensionValidationR // Validate each version for i, ver := range ext.Versions { - validateVersion(&result, i, &ver, strict) + validateVersion(&result, ext.Id, i, &ver, strict) } // Find latest version using semver ordering @@ -262,7 +263,13 @@ func findLatestVersion(versions []ExtensionVersion) *ExtensionVersion { } // validateVersion validates a single version entry within an extension. -func validateVersion(result *ExtensionValidationResult, index int, ver *ExtensionVersion, strict bool) { +func validateVersion( + result *ExtensionValidationResult, + extensionId string, + index int, + ver *ExtensionVersion, + strict bool, +) { prefix := fmt.Sprintf("versions[%d]", index) // Validate semver format using the semver package @@ -281,6 +288,17 @@ func validateVersion(result *ExtensionValidationResult, index int, ver *Extensio } } + // Validate telemetry declarations. Every field an extension may report at + // runtime is declared here and reviewed as part of the registry change. + for _, issue := range ValidateTelemetryDeclarations(extensionId, ver.Telemetry) { + result.addError(fmt.Sprintf("%s: %s", prefix, issue)) + } + + if len(ver.Telemetry) > 0 && !slices.Contains(ver.Capabilities, TelemetryCapability) { + result.addError(fmt.Sprintf("%s: telemetry declarations require the '%s' capability", + prefix, TelemetryCapability)) + } + // Enforce that each version has at least one artifact or dependency hasArtifacts := len(ver.Artifacts) > 0 hasDependencies := len(ver.Dependencies) > 0 diff --git a/cli/azd/pkg/project/artifact.go b/cli/azd/pkg/project/artifact.go index 8303e9113f6..a365c629296 100644 --- a/cli/azd/pkg/project/artifact.go +++ b/cli/azd/pkg/project/artifact.go @@ -11,7 +11,6 @@ import ( "slices" "strings" - "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/output" ) @@ -23,12 +22,6 @@ const ( // MetadataKeyNote adds a note line below the artifact output. MetadataKeyNote = "note" - - // MetadataKeyFromPackage marks an artifact supplied through the - // --from-package option rather than produced by azd. The canonical value - // lives in the SDK so core and extensions share a single definition. This - // is deployment payload provenance, not telemetry. - MetadataKeyFromPackage = azdext.ArtifactMetadataKeyFromPackage ) // ArtifactKind represents well-known artifact types in the Azure Developer CLI diff --git a/docs/README.md b/docs/README.md index a36a0eff4fe..29a470ac2be 100644 --- a/docs/README.md +++ b/docs/README.md @@ -43,6 +43,7 @@ System overviews, design context, and decision records. - [Extension Framework](architecture/extension-framework.md) — gRPC-based extension system architecture - [Provisioning Pipeline](architecture/provisioning-pipeline.md) — How infrastructure provisioning works - [Telemetry Architecture](architecture/telemetry.md) — How azd collects and exports telemetry +- [ADR-001: Extension Telemetry Declarations](architecture/adr-001-extension-telemetry-declarations.md) — Why extension telemetry fields live in the registry - [ADR Template](architecture/adr-template.md) — Template for lightweight architecture decision records --- diff --git a/docs/architecture/adr-001-extension-telemetry-declarations.md b/docs/architecture/adr-001-extension-telemetry-declarations.md new file mode 100644 index 00000000000..e3d339ff83e --- /dev/null +++ b/docs/architecture/adr-001-extension-telemetry-declarations.md @@ -0,0 +1,94 @@ +# ADR-001: Extension telemetry is declared in the registry, not in azd core + +**Status:** Proposed + +**Date:** 2026-07-27 + +## Context + +Extensions need to report a small number of bounded usage signals so the team +can answer product questions like which deployment mode people actually pick. +Two things made this awkward: + +1. The first design put an allowlist of product fields (key, allowed values, + eligible commands, required capability) inside `azd` core. That means every + new field an extension wants is a core PR, a core release, and a wait for + users to upgrade `azd` before any data arrives. It also puts product + semantics from one Foundry product into the CLI that hosts all of them. +2. Extensions cannot simply be trusted to send arbitrary telemetry. The values + need a privacy review, and a bounded set is what makes that review possible. + +The constraint is therefore: reviewable and bounded, but not on the core +release path, and with no product-specific knowledge in core. + +## Decision + +**Field declarations move to the extension registry entry; `azd` core only +enforces them.** + +- `ExtensionVersion` gains `telemetry: [{ key, allowedValues }]`, following the + same "registry declares, core enforces" pattern already used for + `capabilities`, `providers`, and `mcp`. +- Publish-time validation (`ValidateTelemetryDeclarations`) enforces the shape: + keys namespaced as `ext..`, a non-empty closed value + set, and a charset that excludes `:` and `/` so registry names, URLs, and + paths cannot be smuggled through a value. Bounds cap the number of fields, + values, and lengths. +- A dedicated `telemetry` capability gates the feature, and the extension's + install `Source` is carried in the host-signed JWT claims. Only extensions + installed from the official `azd` registry are allowed to report. +- At runtime the host re-validates the stored declaration before use, because + the installed record lives in user-writable config. +- Accepted values are recorded on a new `ext.usage` span rather than being + appended to the command span. +- The RPC is named `ReportUsageAttribute`, deliberately free of any span + wording, so the host can change where values land without a breaking SDK + change. + +The privacy gate does not disappear: the official registry lives in this +repository, so adding a field is still a reviewed PR by the same people. It +just no longer occupies a core release slot. + +## Consequences + +**Easier** + +- Adding a telemetry field is a registry PR. Users get it by upgrading the + extension; `azd` core does not ship. +- `azd` core contains zero product semantics for extension telemetry. The + schema documents one class of field rather than one row per product concept. +- The trust boundary is explicit and signed: capability plus official-registry + source, both host-issued rather than self-reported. + +**More difficult** + +- Reviewers must read registry changes as telemetry changes. This needs to be + part of the registry review checklist, not just tribal knowledge. +- The new span does not sit on the same row as the command in App Insights. + Queries must join on `operation_Id`. This is reliable because `azd` does not + sample and the exporter writes the trace ID to `operation_Id`. +- Trace context has to cross the gRPC boundary for that join to work, so the + extension SDK now forwards the W3C trace context headers and the server + extracts them. + +## Alternatives Considered + +**Keep the allowlist in core.** Simplest to review, and it keeps every value in +one Go file. Rejected because it puts one product's vocabulary in the CLI that +hosts all products, and because it forces a core release for each new field — +the maintenance overhead this design was asked to remove. + +**Let extensions send free-form key/value pairs, validated only by shape** +(length, charset, cardinality). Rejected: an extension could pack a container +image reference into a value and leak a private registry name. A closed, +declared value set is what makes the privacy review meaningful. + +**Put declarations in the signed JWT claims instead of looking them up.** +Rejected: the token travels on every RPC, so declarations would add constant +per-call overhead for a feature used a handful of times per command. + +**Augment the existing command span instead of creating `ext.usage`.** +Rejected: it requires a process-global command-usage scope stack that has to be +opened and closed by middleware and kept correct across nested and concurrent +commands. Supporting both models would mean maintaining that machinery *and* +the new span path. diff --git a/docs/architecture/extension-framework.md b/docs/architecture/extension-framework.md index 6f961f5430a..0e5ce7ad5de 100644 --- a/docs/architecture/extension-framework.md +++ b/docs/architecture/extension-framework.md @@ -58,6 +58,7 @@ Extensions declare their capabilities in `extension.yaml`: | `service-target-provider` | Add deployment support for new hosting targets | | `provisioning-provider` | Add custom infrastructure provisioning support | | `metadata` | Provide metadata about commands and capabilities | +| `telemetry` | Report usage attributes declared in the extension's registry entry | ## Available gRPC Services diff --git a/docs/architecture/telemetry.md b/docs/architecture/telemetry.md index bee6971d45f..a5482f4ea9d 100644 --- a/docs/architecture/telemetry.md +++ b/docs/architecture/telemetry.md @@ -183,6 +183,12 @@ flowchart LR - Auth: `ext.auth.*` - Dependency: `ext.dependency.*` - Extension lifecycle events: `ext.install`, `ext.upgrade`, `ext.promote` +- Extensions installed from the official registry and carrying the `telemetry` + capability can report **declared usage attributes** via + `TelemetryService.ReportUsageAttribute`. Each accepted value becomes an + `ext.usage` span sharing the command's trace. The key and its allowed values + come from the extension's registry entry, not from azd core — see + [ADR-001](./adr-001-extension-telemetry-declarations.md). ## Consent & Privacy diff --git a/docs/concepts/glossary.md b/docs/concepts/glossary.md index 86836ad5275..8dec4393c55 100644 --- a/docs/concepts/glossary.md +++ b/docs/concepts/glossary.md @@ -62,7 +62,11 @@ A JSON manifest that lists available extensions, their versions, capabilities, a ### Extension Capabilities -The set of features an extension provides. Valid capabilities are: `custom-commands`, `lifecycle-events`, `mcp-server`, `service-target-provider`, `framework-service-provider`, `provisioning-provider`, and `metadata`. See [Extension Framework](../architecture/extension-framework.md) for details. +The set of features an extension provides. Valid capabilities are: `custom-commands`, `lifecycle-events`, `mcp-server`, `service-target-provider`, `framework-service-provider`, `provisioning-provider`, `validation-provider`, `metadata`, and `telemetry`. See [Extension Framework](../architecture/extension-framework.md) for details. + +### Telemetry Declaration + +The `telemetry` array in an extension's registry entry, listing each usage attribute key the extension may report and the closed set of values allowed for it. `azd` core owns no product-specific telemetry fields; it only enforces what the registry declares. See [ADR-001](../architecture/adr-001-extension-telemetry-declarations.md). ### Provisioning Provider diff --git a/docs/specs/metrics-audit/telemetry-schema.md b/docs/specs/metrics-audit/telemetry-schema.md index e218838cee0..85883bcc404 100644 --- a/docs/specs/metrics-audit/telemetry-schema.md +++ b/docs/specs/metrics-audit/telemetry-schema.md @@ -19,6 +19,7 @@ OpenTelemetry span name or event name. | `ExtensionInstallEvent` | `ext.install` | Extension install/upgrade event | | `ExtensionUpgradeEvent` | `ext.upgrade` | Single extension upgrade attempt | | `ExtensionPromoteEvent` | `ext.promote` | Extension registry promotion (e.g., dev → main) | +| `ExtensionUsageEvent` | `ext.usage` | One usage attribute reported by an extension through the telemetry service | | `CopilotInitializeEvent` | `copilot.initialize` | Copilot initialization event | | `CopilotSessionEvent` | `copilot.session` | Copilot session lifecycle event | | `ProvisionValidationEvent` | `validation.provision` | Local provision validation outcome | @@ -88,7 +89,6 @@ These are set once at process startup via `resource.New()` and attached to every | Service languages | `project.service.languages` | SystemMetadata | FeatureInsight | List of languages | | Service language | `project.service.language` | SystemMetadata | PerformanceAndHealth | Single service language | | Platform type | `platform.type` | SystemMetadata | FeatureInsight | e.g. `aca`, `aks` | -| 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 | ### Config and Environment @@ -216,6 +216,34 @@ not emitted by azd spans. | Dependency of | `extension.dependency_of` | SystemMetadata | FeatureInsight | Parent extension for a dependency upgrade | | Dependency upgrade count | `extension.dependency_upgrade_count` | SystemMetadata | FeatureInsight | Recursive dependency upgrade count | +#### Extension-contributed usage attributes + +Extensions do not have individual fields listed in this document. Instead they +declare the attributes they may report in their entry in the official azd +extension registry, and `azd` records each accepted value on an `ext.usage` +span alongside `extension.id` and `extension.version`. + +`azd` core carries no product-specific telemetry semantics for these fields. +The following rules are enforced by the host and are what this schema +guarantees about the whole class: + +| Rule | Enforcement | +|------|-------------| +| Key namespace | Must be `ext..[.…]`, lowercase alphanumeric and hyphens | +| Values | Must be one of the closed set declared for that key in the registry; free-form values are always rejected | +| Value charset | Lowercase alphanumeric with `_`, `-`, and `.`; `:` and `/` are excluded so registry names, URLs, and paths cannot be smuggled through | +| Classification | Always `SystemMetadata` | +| Purpose | Always `FeatureInsight` | +| Trust | Only extensions installed from the official `azd` registry, carrying the `telemetry` capability, may report values | +| Review | Adding or changing a declared field is a change to the official registry in this repository and is reviewed like any other telemetry change | + +Because `ext.usage` spans share the command's trace, they join the originating +command in Kusto on `operation_Id`. See +[ADR-001](../../architecture/adr-001-extension-telemetry-declarations.md) for +the design rationale and +[Extension Telemetry Fields](../../../cli/azd/docs/extensions/extension-telemetry-fields.md) +for the author-facing declaration rules. + ### Update | Field | OTel Key | Classification | Purpose | From 4fd8e4c71cca81b60e8bee19bca856b7eff83dca Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 19:03:37 +0800 Subject: [PATCH 06/12] ci: reword telemetry doc example to pass spell check --- cli/azd/docs/extensions/extension-telemetry-fields.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/azd/docs/extensions/extension-telemetry-fields.md b/cli/azd/docs/extensions/extension-telemetry-fields.md index 426c1b5855e..9206b61cc83 100644 --- a/cli/azd/docs/extensions/extension-telemetry-fields.md +++ b/cli/azd/docs/extensions/extension-telemetry-fields.md @@ -20,10 +20,10 @@ for the reasoning behind this design. | Always rejected | Free-form values, values outside your declared set, keys you did not declare | Values must be a closed enum because that is what makes a privacy review -possible. A shape-only rule (length and charset) would let a value like -`byo_image:myregistry.azurecr.io/foo` through and leak a registry name, so the -charset excludes `:` and `/` and the host compares each value against your -declaration. +possible. A shape-only rule (length and charset) would let someone pack a +container image reference into a value and leak a private registry name, so +the charset excludes `:` and `/` and the host compares each value against +your declaration. ## Step 1: Declare the capability From b1051bc0da45f39e7806048d9acf6440f695f68f Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 19:03:38 +0800 Subject: [PATCH 07/12] feat: document the ext.usage event in telemetry references --- .../adr-001-extension-telemetry-declarations.md | 9 +++++++-- docs/reference/telemetry-data.md | 1 + docs/specs/metrics-audit/feature-telemetry-matrix.md | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr-001-extension-telemetry-declarations.md b/docs/architecture/adr-001-extension-telemetry-declarations.md index e3d339ff83e..381c4a9b3d9 100644 --- a/docs/architecture/adr-001-extension-telemetry-declarations.md +++ b/docs/architecture/adr-001-extension-telemetry-declarations.md @@ -57,8 +57,9 @@ just no longer occupies a core release slot. extension; `azd` core does not ship. - `azd` core contains zero product semantics for extension telemetry. The schema documents one class of field rather than one row per product concept. -- The trust boundary is explicit and signed: capability plus official-registry - source, both host-issued rather than self-reported. +- The trust boundary is explicit: the capability and the install `Source` are + carried in host-signed claims, so an extension cannot assert either of them + on the request itself. **More difficult** @@ -70,6 +71,10 @@ just no longer occupies a core release slot. - Trace context has to cross the gRPC boundary for that join to work, so the extension SDK now forwards the W3C trace context headers and the server extracts them. +- Those claims are signed from the installed record, which lives in + user-writable config. This is the same trust level every existing capability + gate already depends on, so the feature does not weaken it, but it is not + proof of provenance. Hardening the installed record is tracked separately. ## Alternatives Considered diff --git a/docs/reference/telemetry-data.md b/docs/reference/telemetry-data.md index 47f001122e0..698682f8ea4 100644 --- a/docs/reference/telemetry-data.md +++ b/docs/reference/telemetry-data.md @@ -74,6 +74,7 @@ Commands follow the pattern `cmd.` where spaces become dots. | `ext.install` | Extension installation | | `ext.upgrade` | Extension upgrade attempt | | `ext.promote` | Registry promotion (e.g., dev → main) | +| `ext.usage` | Usage attribute reported by an extension, limited to the values declared in its official registry entry | ### Agent & Copilot Events diff --git a/docs/specs/metrics-audit/feature-telemetry-matrix.md b/docs/specs/metrics-audit/feature-telemetry-matrix.md index 5e9fea1358a..548f4f96fb0 100644 --- a/docs/specs/metrics-audit/feature-telemetry-matrix.md +++ b/docs/specs/metrics-audit/feature-telemetry-matrix.md @@ -166,3 +166,4 @@ privacy review covers every emission point. | **Agent troubleshoot middleware** | Triggered on command failure when troubleshooting is engaged | `agent.troubleshoot` | Error chain attributes, hashed error fields | Emitted from `cmd/middleware/error.go` | | **Up-graph performance** | `up` (graph execution) | (none — enriches the `up` command span) | `perf.provision_duration_ms`, `perf.deploy_duration_ms`, `perf.total_duration_ms` | Emitted from `internal/cmd/up_graph.go` after the graph completes; provision/deploy durations set only when those phases run | | **VS RPC** | `vs-server` long-running session | `vsrpc.*` (event prefix) | Per-RPC attributes documented in `telemetry-schema.md` | Long-running RPC server for VS integration | +| **Extension telemetry service** | Extension calls `ReportUsageAttribute` over the extension gRPC API | `ext.usage` | `extension.id`, `extension.version`, plus the single registry-declared attribute key/value carried by the request | Requires the `telemetry` capability and an install from the official `azd` registry. Keys and their closed value sets are declared per version in the registry, not in core; the host rejects any key outside the `ext..` namespace and any value outside the declared set | From 4a52fb68c1273932df31c910e9f49d582d3aad15 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 19:03:39 +0800 Subject: [PATCH 08/12] test: assert the ext.usage span name and attributes --- .../grpcserver/telemetry_service_test.go | 58 ++++++++++++++++++- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/cli/azd/internal/grpcserver/telemetry_service_test.go b/cli/azd/internal/grpcserver/telemetry_service_test.go index fc33c1f0499..371184de4ac 100644 --- a/cli/azd/internal/grpcserver/telemetry_service_test.go +++ b/cli/azd/internal/grpcserver/telemetry_service_test.go @@ -7,10 +7,16 @@ import ( "strings" "testing" + "github.com/azure/azure-dev/cli/azd/internal/tracing/events" + "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -78,15 +84,63 @@ func requireCode(t *testing.T, err error, expected codes.Code) { } func Test_TelemetryService_AcceptsDeclaredValue(t *testing.T) { - t.Parallel() + // Installs a global tracer provider, so this test cannot be parallel. + recorder := tracetest.NewSpanRecorder() + provider := tracesdk.NewTracerProvider(tracesdk.WithSpanProcessor(recorder)) + + previous := otel.GetTracerProvider() + otel.SetTracerProvider(provider) + t.Cleanup(func() { otel.SetTracerProvider(previous) }) + + // Start a command span first so the usage span can be checked to share + // its trace, which is what makes the operation_Id join work downstream. + ctx, command := provider.Tracer("test").Start(t.Context(), "cmd.deploy") + + extension := testExtension() + service := newTelemetryService(stubExtensionLookup{extension}) + ctx = extensions.WithClaimsContext(ctx, &extensions.ExtensionClaims{ + RegisteredClaims: jwt.RegisteredClaims{Subject: extension.Id}, + Capabilities: extension.Capabilities, + Source: extension.Source, + }) - resp, err := callWith(t, testExtension(), &azdext.ReportUsageAttributeRequest{ + resp, err := service.ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{ Key: testTelemetryKey, Value: "container", }) require.NoError(t, err) require.True(t, resp.Accepted) + + ended := recorder.Ended() + require.Len(t, ended, 1) + + usage := ended[0] + require.Equal(t, events.ExtensionUsageEvent, usage.Name()) + require.Equal(t, command.SpanContext().TraceID(), usage.SpanContext().TraceID()) + require.ElementsMatch(t, []attribute.KeyValue{ + fields.ExtensionId.String(extension.Id), + fields.ExtensionVersion.String(extension.Version), + attribute.String(testTelemetryKey, "container"), + }, usage.Attributes()) +} + +func Test_TelemetryService_RecordsNoSpanWhenRejected(t *testing.T) { + // Installs a global tracer provider, so this test cannot be parallel. + recorder := tracetest.NewSpanRecorder() + provider := tracesdk.NewTracerProvider(tracesdk.WithSpanProcessor(recorder)) + + previous := otel.GetTracerProvider() + otel.SetTracerProvider(provider) + t.Cleanup(func() { otel.SetTracerProvider(previous) }) + + _, err := callWith(t, testExtension(), &azdext.ReportUsageAttributeRequest{ + Key: testTelemetryKey, + Value: "byo_image", + }) + + requireCode(t, err, codes.InvalidArgument) + require.Empty(t, recorder.Ended()) } func Test_TelemetryService_RequiresClaims(t *testing.T) { From 6f0bc803196f87501d44a66f13f2ed82d7dbc7da Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 19:03:39 +0800 Subject: [PATCH 09/12] fix: preserve registry telemetry declarations on republish --- .../microsoft.azd.extensions/internal/cmd/publish.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go b/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go index 7fec51145e2..1373e21ae8c 100644 --- a/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go +++ b/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go @@ -474,6 +474,10 @@ func addOrUpdateExtension( Dependencies: extensionMetadata.Dependencies, Providers: extensionMetadata.Providers, Artifacts: artifacts, + // Telemetry declarations are authored directly in the + // registry and reviewed there, so carry them forward + // instead of dropping them on republish. + Telemetry: v.Telemetry, } return From ed2e698a53ae00530880837af8739849a1da8ea5 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 21:13:51 +0800 Subject: [PATCH 10/12] test: share one tracer provider across grpcserver tests --- cli/azd/internal/grpcserver/main_test.go | 29 ++++++ .../grpcserver/telemetry_service_test.go | 92 ++++++++++++------- 2 files changed, 87 insertions(+), 34 deletions(-) create mode 100644 cli/azd/internal/grpcserver/main_test.go diff --git a/cli/azd/internal/grpcserver/main_test.go b/cli/azd/internal/grpcserver/main_test.go new file mode 100644 index 00000000000..8786c36d7a0 --- /dev/null +++ b/cli/azd/internal/grpcserver/main_test.go @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package grpcserver + +import ( + "os" + "testing" + + "go.opentelemetry.io/otel" + tracesdk "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" +) + +// The OTel global provider only ever delegates to the first provider +// installed in a process, and internal/tracing resolves its tracer at +// package init. A provider installed inside a single test would therefore +// be silently ignored, so the package installs exactly one here and tests +// share the recorder. +var ( + testSpanRecorder = tracetest.NewSpanRecorder() + testTracerProvider = tracesdk.NewTracerProvider( + tracesdk.WithSpanProcessor(testSpanRecorder)) +) + +func TestMain(m *testing.M) { + otel.SetTracerProvider(testTracerProvider) + os.Exit(m.Run()) +} diff --git a/cli/azd/internal/grpcserver/telemetry_service_test.go b/cli/azd/internal/grpcserver/telemetry_service_test.go index 371184de4ac..5abc7b91fa0 100644 --- a/cli/azd/internal/grpcserver/telemetry_service_test.go +++ b/cli/azd/internal/grpcserver/telemetry_service_test.go @@ -4,6 +4,7 @@ package grpcserver import ( + "context" "strings" "testing" @@ -13,10 +14,9 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" tracesdk "go.opentelemetry.io/otel/sdk/trace" - "go.opentelemetry.io/otel/sdk/trace/tracetest" + "go.opentelemetry.io/otel/trace" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -65,8 +65,21 @@ func callWith( ) (*azdext.ReportUsageAttributeResponse, error) { t.Helper() + return callWithContext(t, t.Context(), extension, req) +} + +// callWithContext is callWith with a caller-supplied context, so a test can +// place the call inside an existing trace. +func callWithContext( + t *testing.T, + ctx context.Context, + extension *extensions.Extension, + req *azdext.ReportUsageAttributeRequest, +) (*azdext.ReportUsageAttributeResponse, error) { + t.Helper() + service := newTelemetryService(stubExtensionLookup{extension}) - ctx := extensions.WithClaimsContext(t.Context(), &extensions.ExtensionClaims{ + ctx = extensions.WithClaimsContext(ctx, &extensions.ExtensionClaims{ RegisteredClaims: jwt.RegisteredClaims{Subject: extension.Id}, Capabilities: extension.Capabilities, Source: extension.Source, @@ -75,6 +88,25 @@ func callWith( return service.ReportUsageAttribute(ctx, req) } +// usageSpansIn returns the recorded ext.usage spans belonging to traceId. +// The recorder is shared by the whole package, so filtering on the trace +// started by a single test keeps it independent of every other test. +func usageSpansIn(traceId trace.TraceID) []tracesdk.ReadOnlySpan { + matched := []tracesdk.ReadOnlySpan{} + + for _, span := range testSpanRecorder.Ended() { + if span.Name() != events.ExtensionUsageEvent { + continue + } + + if span.SpanContext().TraceID() == traceId { + matched = append(matched, span) + } + } + + return matched +} + func requireCode(t *testing.T, err error, expected codes.Code) { t.Helper() @@ -84,27 +116,15 @@ func requireCode(t *testing.T, err error, expected codes.Code) { } func Test_TelemetryService_AcceptsDeclaredValue(t *testing.T) { - // Installs a global tracer provider, so this test cannot be parallel. - recorder := tracetest.NewSpanRecorder() - provider := tracesdk.NewTracerProvider(tracesdk.WithSpanProcessor(recorder)) - - previous := otel.GetTracerProvider() - otel.SetTracerProvider(provider) - t.Cleanup(func() { otel.SetTracerProvider(previous) }) + t.Parallel() // Start a command span first so the usage span can be checked to share // its trace, which is what makes the operation_Id join work downstream. - ctx, command := provider.Tracer("test").Start(t.Context(), "cmd.deploy") + ctx, command := testTracerProvider.Tracer("test").Start(t.Context(), "cmd.deploy") + defer command.End() extension := testExtension() - service := newTelemetryService(stubExtensionLookup{extension}) - ctx = extensions.WithClaimsContext(ctx, &extensions.ExtensionClaims{ - RegisteredClaims: jwt.RegisteredClaims{Subject: extension.Id}, - Capabilities: extension.Capabilities, - Source: extension.Source, - }) - - resp, err := service.ReportUsageAttribute(ctx, &azdext.ReportUsageAttributeRequest{ + resp, err := callWithContext(t, ctx, extension, &azdext.ReportUsageAttributeRequest{ Key: testTelemetryKey, Value: "container", }) @@ -112,35 +132,39 @@ func Test_TelemetryService_AcceptsDeclaredValue(t *testing.T) { require.NoError(t, err) require.True(t, resp.Accepted) - ended := recorder.Ended() - require.Len(t, ended, 1) + recorded := usageSpansIn(command.SpanContext().TraceID()) + require.Len(t, recorded, 1) - usage := ended[0] - require.Equal(t, events.ExtensionUsageEvent, usage.Name()) - require.Equal(t, command.SpanContext().TraceID(), usage.SpanContext().TraceID()) - require.ElementsMatch(t, []attribute.KeyValue{ + // Spans also carry process-global attributes, so assert only the ones + // this feature owns rather than matching the whole set. + attributes := map[attribute.Key]attribute.Value{} + for _, attr := range recorded[0].Attributes() { + attributes[attr.Key] = attr.Value + } + + for _, expected := range []attribute.KeyValue{ fields.ExtensionId.String(extension.Id), fields.ExtensionVersion.String(extension.Version), attribute.String(testTelemetryKey, "container"), - }, usage.Attributes()) + } { + require.Contains(t, attributes, expected.Key) + require.Equal(t, expected.Value, attributes[expected.Key]) + } } func Test_TelemetryService_RecordsNoSpanWhenRejected(t *testing.T) { - // Installs a global tracer provider, so this test cannot be parallel. - recorder := tracetest.NewSpanRecorder() - provider := tracesdk.NewTracerProvider(tracesdk.WithSpanProcessor(recorder)) + t.Parallel() - previous := otel.GetTracerProvider() - otel.SetTracerProvider(provider) - t.Cleanup(func() { otel.SetTracerProvider(previous) }) + ctx, command := testTracerProvider.Tracer("test").Start(t.Context(), "cmd.deploy") + defer command.End() - _, err := callWith(t, testExtension(), &azdext.ReportUsageAttributeRequest{ + _, err := callWithContext(t, ctx, testExtension(), &azdext.ReportUsageAttributeRequest{ Key: testTelemetryKey, Value: "byo_image", }) requireCode(t, err, codes.InvalidArgument) - require.Empty(t, recorder.Ended()) + require.Empty(t, usageSpansIn(command.SpanContext().TraceID())) } func Test_TelemetryService_RequiresClaims(t *testing.T) { From fb0cede4edd1cb0ae82e3eb885f23f7baa18a9d1 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 21:13:52 +0800 Subject: [PATCH 11/12] fix: reuse exported trace context constants in azd client --- cli/azd/pkg/azdext/azd_client.go | 23 ++++++----------------- cli/azd/pkg/azdext/azd_client_test.go | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/cli/azd/pkg/azdext/azd_client.go b/cli/azd/pkg/azdext/azd_client.go index 140b45ac2c1..9d02d80141a 100644 --- a/cli/azd/pkg/azdext/azd_client.go +++ b/cli/azd/pkg/azdext/azd_client.go @@ -17,17 +17,6 @@ import ( "google.golang.org/grpc/metadata" ) -// W3C trace context carriers. The gRPC metadata keys match the HTTP header -// names; the environment variables are what azd sets on the extension -// process. -const ( - traceparentHeader = "traceparent" - tracestateHeader = "tracestate" - - traceparentEnv = "TRACEPARENT" - tracestateEnv = "TRACESTATE" -) - type AzdClientOption func(*AzdClient) error // AzdClient is the client for the `azd` gRPC server. @@ -117,21 +106,21 @@ func traceContextPairs(ctx context.Context) []string { carrier := propagation.MapCarrier{} propagation.TraceContext{}.Inject(ctx, carrier) - parent := carrier.Get(traceparentHeader) - state := carrier.Get(tracestateHeader) + parent := carrier.Get(TraceparentKey) + state := carrier.Get(TracestateKey) if parent == "" { - parent = os.Getenv(traceparentEnv) - state = os.Getenv(tracestateEnv) + parent = os.Getenv(TraceparentEnv) + state = os.Getenv(TracestateEnv) } if parent == "" { return nil } - pairs := []string{traceparentHeader, parent} + pairs := []string{TraceparentKey, parent} if state != "" { - pairs = append(pairs, tracestateHeader, state) + pairs = append(pairs, TracestateKey, state) } return pairs diff --git a/cli/azd/pkg/azdext/azd_client_test.go b/cli/azd/pkg/azdext/azd_client_test.go index eb4b486a3f5..21c3d9fa551 100644 --- a/cli/azd/pkg/azdext/azd_client_test.go +++ b/cli/azd/pkg/azdext/azd_client_test.go @@ -21,19 +21,19 @@ func testTraceparent() string { } func Test_WithAccessToken_PropagatesEnvTraceContext(t *testing.T) { - t.Setenv(traceparentEnv, testTraceparent()) - t.Setenv(tracestateEnv, "vendor=value") + t.Setenv(TraceparentEnv, testTraceparent()) + t.Setenv(TracestateEnv, "vendor=value") md, ok := metadata.FromOutgoingContext(WithAccessToken(t.Context(), "token")) require.True(t, ok) require.Equal(t, []string{"token"}, md.Get("authorization")) - require.Equal(t, []string{testTraceparent()}, md.Get(traceparentHeader)) - require.Equal(t, []string{"vendor=value"}, md.Get(tracestateHeader)) + require.Equal(t, []string{testTraceparent()}, md.Get(TraceparentKey)) + require.Equal(t, []string{"vendor=value"}, md.Get(TracestateKey)) } func Test_WithAccessToken_PrefersContextSpan(t *testing.T) { - t.Setenv(traceparentEnv, testTraceparent()) + t.Setenv(TraceparentEnv, testTraceparent()) traceId, err := trace.TraceIDFromHex("11111111111111111111111111111111") require.NoError(t, err) @@ -51,19 +51,19 @@ func Test_WithAccessToken_PrefersContextSpan(t *testing.T) { require.True(t, ok) require.Equal(t, []string{ "00-11111111111111111111111111111111-2222222222222222-01", - }, md.Get(traceparentHeader)) + }, md.Get(TraceparentKey)) } func Test_WithAccessToken_OmitsMissingTraceContext(t *testing.T) { - t.Setenv(traceparentEnv, "") - t.Setenv(tracestateEnv, "") + t.Setenv(TraceparentEnv, "") + t.Setenv(TracestateEnv, "") md, ok := metadata.FromOutgoingContext(WithAccessToken(t.Context(), "token")) require.True(t, ok) require.Equal(t, []string{"token"}, md.Get("authorization")) - require.Empty(t, md.Get(traceparentHeader)) - require.Empty(t, md.Get(tracestateHeader)) + require.Empty(t, md.Get(TraceparentKey)) + require.Empty(t, md.Get(TracestateKey)) } func Test_IsLocalhostAddress(t *testing.T) { From 212870454a0a4e33fb22836d12251b30535630e7 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 27 Jul 2026 21:13:53 +0800 Subject: [PATCH 12/12] fix: preserve registry mcp config on extension republish --- .../microsoft.azd.extensions/internal/cmd/publish.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go b/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go index 1373e21ae8c..5ef71c59cee 100644 --- a/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go +++ b/cli/azd/extensions/microsoft.azd.extensions/internal/cmd/publish.go @@ -474,10 +474,11 @@ func addOrUpdateExtension( Dependencies: extensionMetadata.Dependencies, Providers: extensionMetadata.Providers, Artifacts: artifacts, - // Telemetry declarations are authored directly in the - // registry and reviewed there, so carry them forward - // instead of dropping them on republish. + // Telemetry declarations and MCP config are authored + // directly in the registry and reviewed there, so carry + // them forward instead of dropping them on republish. Telemetry: v.Telemetry, + McpConfig: v.McpConfig, } return