From c07d8437c2829da554847ee02eb46a2545cedec5 Mon Sep 17 00:00:00 2001 From: hemarina Date: Fri, 21 Aug 2026 18:52:15 -0700 Subject: [PATCH 1/2] fix telemetry resource field metadata Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/azd-code-reviewer/reviewers.md | 10 +++++++++ cli/azd/cmd/telemetry_test.go | 18 ++++++++++++++++ cli/azd/internal/cmd/errors.go | 8 +++---- cli/azd/internal/cmd/errors_test.go | 14 ++++++------- cli/azd/internal/tracing/fields/fields.go | 21 +++++++++---------- docs/specs/metrics-audit/telemetry-schema.md | 6 +++--- 6 files changed, 52 insertions(+), 25 deletions(-) diff --git a/.github/skills/azd-code-reviewer/reviewers.md b/.github/skills/azd-code-reviewer/reviewers.md index b702de1948e..c89fc32af74 100644 --- a/.github/skills/azd-code-reviewer/reviewers.md +++ b/.github/skills/azd-code-reviewer/reviewers.md @@ -310,3 +310,13 @@ Apply your domain expertise to identify: Use the same findings format and severity levels as the fixed lenses. If there are no {domain}-specific concerns, contribute no findings. ``` + +### Observability-specific checks + +When applying the Observability Expert lens to telemetry field changes: + +- Verify every new exported, package-level `fields.AttributeKey` sets non-empty `Classification` + and `Purpose` metadata. +- Compare each runtime key string with existing `AttributeKey` declarations. A telemetry key must be + declared exactly once; reuse its canonical declaration instead of adding an alias or duplicate, + including when `fields.ErrorKey` later prefixes the emitted key with `error.`. diff --git a/cli/azd/cmd/telemetry_test.go b/cli/azd/cmd/telemetry_test.go index 5e3df5a14ca..562bbe6b1df 100644 --- a/cli/azd/cmd/telemetry_test.go +++ b/cli/azd/cmd/telemetry_test.go @@ -41,6 +41,24 @@ func TestTelemetryEventConstants(t *testing.T) { // rejected by TestNoRawTelemetryAttributes (below). func TestTelemetryFieldConstants(t *testing.T) { t.Parallel() + t.Run("ResourceFields", func(t *testing.T) { + t.Parallel() + tests := []struct { + key fields.AttributeKey + expectedName string + expectedPurpose fields.Purpose + }{ + {fields.ServiceNameKey, "service.name", fields.PerformanceAndHealth}, + {fields.ServiceVersionKey, "service.version", fields.FeatureInsight}, + {fields.OSTypeKey, "os.type", fields.FeatureInsight}, + } + for _, tt := range tests { + require.Equal(t, tt.expectedName, string(tt.key.Key)) + require.Equal(t, fields.SystemMetadata, tt.key.Classification) + require.Equal(t, tt.expectedPurpose, tt.key.Purpose) + } + }) + // Auth command telemetry fields t.Run("AuthFields", func(t *testing.T) { t.Parallel() diff --git a/cli/azd/internal/cmd/errors.go b/cli/azd/internal/cmd/errors.go index 56c544fb8db..30b065d7029 100644 --- a/cli/azd/internal/cmd/errors.go +++ b/cli/azd/internal/cmd/errors.go @@ -194,7 +194,7 @@ func classifyResponseError(respErr *azcore.ResponseError) (string, []attribute.K attrs = append(attrs, fields.ServiceHost.String(hostName), fields.ServiceMethod.String(respErr.RawResponse.Request.Method), - fields.ServiceName.String(serviceName), + fields.ServiceNameKey.String(serviceName), ) } } @@ -203,7 +203,7 @@ func classifyResponseError(respErr *azcore.ResponseError) (string, []attribute.K } func classifyArmDeployError(armDeployErr *azapi.AzureDeploymentError) (string, []attribute.KeyValue) { - attrs := []attribute.KeyValue{fields.ServiceName.String("arm")} + attrs := []attribute.KeyValue{fields.ServiceNameKey.String("arm")} codes := []*deploymentErrorCode{} var collect func(details []*azapi.DeploymentErrorLine, frame int) collect = func(details []*azapi.DeploymentErrorLine, frame int) { @@ -247,7 +247,7 @@ func classifyExtServiceError(extServiceErr *azdext.ServiceError) (string, []attr var hostDomain string serviceName, hostDomain = mapService(extServiceErr.ServiceName) attrs = append(attrs, - fields.ServiceName.String(serviceName), + fields.ServiceNameKey.String(serviceName), fields.ServiceHost.String(hostDomain), ) } @@ -282,7 +282,7 @@ func classifyExtLocalError(extLocalErr *azdext.LocalError) (string, []attribute. } func authFailedTelemetryDetails(authFailedErr *auth.AuthFailedError) []attribute.KeyValue { - errDetails := []attribute.KeyValue{fields.ServiceName.String("aad")} + errDetails := []attribute.KeyValue{fields.ServiceNameKey.String("aad")} if authFailedErr == nil || authFailedErr.Parsed == nil { return errDetails } diff --git a/cli/azd/internal/cmd/errors_test.go b/cli/azd/internal/cmd/errors_test.go index 86ecf94754d..fa7eb77dadf 100644 --- a/cli/azd/internal/cmd/errors_test.go +++ b/cli/azd/internal/cmd/errors_test.go @@ -110,7 +110,7 @@ func Test_MapError(t *testing.T) { }, wantErrReason: "service.arm.deployment.failed", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("arm"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("arm"), fields.ErrorKey(fields.ServiceErrorCode.Key).String(mustMarshalJson( []map[string]any{ { @@ -145,7 +145,7 @@ func Test_MapError(t *testing.T) { }, wantErrReason: "service.arm.validate.failed", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("arm"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("arm"), fields.ErrorKey(fields.ServiceErrorCode.Key).String(mustMarshalJson( []map[string]any{ { @@ -174,7 +174,7 @@ func Test_MapError(t *testing.T) { }, wantErrReason: "service.arm.503", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("arm"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("arm"), fields.ErrorKey(fields.ServiceHost.Key).String("management.azure.com"), fields.ErrorKey(fields.ServiceMethod.Key).String("GET"), fields.ErrorKey(fields.ServiceErrorCode.Key).String("ServiceUnavailable"), @@ -196,7 +196,7 @@ func Test_MapError(t *testing.T) { }, wantErrReason: "service.aad.failed", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("aad"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("aad"), fields.ErrorKey(fields.ServiceErrorCode.Key).String("50076,50078,50079"), fields.ErrorKey(fields.ServiceStatusCode.Key).String("invalid_grant"), fields.ErrorKey(fields.ServiceCorrelationId.Key).String("12345"), @@ -226,7 +226,7 @@ func Test_MapError(t *testing.T) { wantErrReason: "error.suggestion", wantErrDetails: []attribute.KeyValue{ fields.ErrType.String("service.aad.failed"), - fields.ErrorKey(fields.ServiceName.Key).String("aad"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("aad"), fields.ErrorKey(fields.ServiceErrorCode.Key).String("530084"), fields.ErrorKey(fields.ServiceStatusCode.Key).String("invalid_grant"), fields.ErrorKey(fields.ServiceCorrelationId.Key).String("blocked-token-protection"), @@ -250,7 +250,7 @@ func Test_MapError(t *testing.T) { }, wantErrReason: "ext.service.create_agent.ratelimitexceeded", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("openai"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("openai"), fields.ErrorKey(fields.ServiceHost.Key).String("openai.azure.com"), fields.ErrorKey(fields.ServiceStatusCode.Key).Int(429), fields.ErrorKey(fields.ServiceErrorCode.Key).String("create_agent.RateLimitExceeded"), @@ -276,7 +276,7 @@ func Test_MapError(t *testing.T) { }, wantErrReason: "ext.service.ai.500", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("ai"), + fields.ErrorKey(fields.ServiceNameKey.Key).String("ai"), fields.ErrorKey(fields.ServiceHost.Key).String("services.ai.azure.com"), fields.ErrorKey(fields.ServiceStatusCode.Key).Int(500), }, diff --git a/cli/azd/internal/tracing/fields/fields.go b/cli/azd/internal/tracing/fields/fields.go index 45c70000f7a..877e34686e5 100644 --- a/cli/azd/internal/tracing/fields/fields.go +++ b/cli/azd/internal/tracing/fields/fields.go @@ -55,19 +55,25 @@ func ExtensionUsageAttribute(key string) AttributeKey { // Application-level fields. Guaranteed to be set and available for all events. var ( - // Application name. Value is always "azd". + // Service name. The application resource uses "azd"; MapError prefixes error details as error.service.name. ServiceNameKey = AttributeKey{ - Key: semconv.ServiceNameKey, // service.name + Key: semconv.ServiceNameKey, // service.name + Classification: SystemMetadata, + Purpose: PerformanceAndHealth, } // Application version. ServiceVersionKey = AttributeKey{ - Key: semconv.ServiceVersionKey, // service.version + Key: semconv.ServiceVersionKey, // service.version + Classification: SystemMetadata, + Purpose: FeatureInsight, } // The operating system type. OSTypeKey = AttributeKey{ - Key: semconv.OSTypeKey, // os.type + Key: semconv.OSTypeKey, // os.type + Classification: SystemMetadata, + Purpose: FeatureInsight, } // The operating system version. @@ -939,13 +945,6 @@ var ( Purpose: PerformanceAndHealth, } - // Name of the service. - ServiceName = AttributeKey{ - Key: attribute.Key("service.name"), - Classification: SystemMetadata, - Purpose: PerformanceAndHealth, - } - // Status code of a response returned by the service. // For HTTP, this corresponds to the HTTP status code. ServiceStatusCode = AttributeKey{ diff --git a/docs/specs/metrics-audit/telemetry-schema.md b/docs/specs/metrics-audit/telemetry-schema.md index 5979cf35639..378aa3ad91d 100644 --- a/docs/specs/metrics-audit/telemetry-schema.md +++ b/docs/specs/metrics-audit/telemetry-schema.md @@ -52,9 +52,9 @@ These are set once at process startup via `resource.New()` and attached to every | Field | OTel Key | Classification | Purpose | Notes | |-------|----------|----------------|---------|-------| -| Service name | `service.name` | — | — | Always `"azd"` | -| Service version | `service.version` | — | — | Build version string | -| OS type | `os.type` | — | — | e.g. `linux`, `windows`, `darwin` | +| Service name | `service.name` | SystemMetadata | PerformanceAndHealth | Always `"azd"` | +| Service version | `service.version` | SystemMetadata | FeatureInsight | Build version string | +| OS type | `os.type` | SystemMetadata | FeatureInsight | e.g. `linux`, `windows`, `darwin` | | OS version | `os.version` | SystemMetadata | PerformanceAndHealth | Kernel / build version | | Host architecture | `host.arch` | SystemMetadata | PerformanceAndHealth | e.g. `amd64`, `arm64` | | Runtime version | `process.runtime.version` | SystemMetadata | PerformanceAndHealth | Go version | From 7db0fbb540db37c1c8f9b45406fe7ab1ae22cf25 Mon Sep 17 00:00:00 2001 From: hemarina Date: Fri, 21 Aug 2026 19:03:32 -0700 Subject: [PATCH 2/2] clarify telemetry metadata review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/skills/azd-code-reviewer/reviewers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/skills/azd-code-reviewer/reviewers.md b/.github/skills/azd-code-reviewer/reviewers.md index c89fc32af74..12b0edf2381 100644 --- a/.github/skills/azd-code-reviewer/reviewers.md +++ b/.github/skills/azd-code-reviewer/reviewers.md @@ -315,8 +315,8 @@ If there are no {domain}-specific concerns, contribute no findings. When applying the Observability Expert lens to telemetry field changes: -- Verify every new exported, package-level `fields.AttributeKey` sets non-empty `Classification` - and `Purpose` metadata. +- Verify every new or modified exported, package-level `fields.AttributeKey` sets non-empty + `Classification` and `Purpose` metadata. - Compare each runtime key string with existing `AttributeKey` declarations. A telemetry key must be declared exactly once; reuse its canonical declaration instead of adding an alias or duplicate, including when `fields.ErrorKey` later prefixes the emitted key with `error.`.