Skip to content

Commit 701e6b4

Browse files
spboyerCopilot
authored andcommitted
Fix auth error telemetry classification (Azure#7233) (Azure#7235)
Auth errors were classified as 'unknown' in telemetry (~610K/28d). Changes: - Add ErrCategory("auth") to ReLoginRequiredError, ErrNoCurrentUser, and azidentity.AuthenticationFailedError branches in MapError() - Move ErrNoCurrentUser from classifySentinel() into MapError() so it can set errDetails - Add azidentity.AuthenticationFailedError handler with new code "auth.identity_failed" - Update classifySuggestionType() to match Per @JeffreyCA and @vhvb1989 feedback: use error.category (not service.name) for all auth errors since these are locally generated and may come from azd or az CLI delegated auth, not from AAD directly. Fixes Azure#7233 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8415627 commit 701e6b4

2 files changed

Lines changed: 53 additions & 10 deletions

File tree

cli/azd/internal/cmd/errors.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919

2020
"github.com/AlecAivazis/survey/v2/terminal"
2121
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
22+
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
2223
"github.com/azure/azure-dev/cli/azd/internal"
2324
"github.com/azure/azure-dev/cli/azd/internal/agent/consent"
2425
"github.com/azure/azure-dev/cli/azd/internal/tracing"
@@ -48,6 +49,7 @@ func MapError(err error, span tracing.Span) {
4849
errCode = updateErr.Code
4950
} else if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
5051
errCode = "auth.login_required"
52+
errDetails = append(errDetails, fields.ErrCategory.String("auth"))
5153
} else if errWithSuggestion, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok {
5254
errCode = "error.suggestion"
5355
span.SetAttributes(fields.ErrType.String(classifySuggestionType(errWithSuggestion.Unwrap())))
@@ -184,6 +186,12 @@ func MapError(err error, span tracing.Span) {
184186
fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId))
185187
}
186188
errCode = "service.aad.failed"
189+
} else if errors.Is(err, auth.ErrNoCurrentUser) {
190+
errCode = "auth.not_logged_in"
191+
errDetails = append(errDetails, fields.ErrCategory.String("auth"))
192+
} else if _, ok := errors.AsType[*azidentity.AuthenticationFailedError](err); ok {
193+
errCode = "auth.identity_failed"
194+
errDetails = append(errDetails, fields.ErrCategory.String("auth"))
187195
} else if code := classifySentinel(err); code != "" {
188196
errCode = code
189197
} else if isNetworkError(err) {
@@ -268,8 +276,6 @@ func classifySentinel(err error) string {
268276
return "user.canceled"
269277
case errors.Is(err, context.DeadlineExceeded):
270278
return "internal.timeout"
271-
case errors.Is(err, auth.ErrNoCurrentUser):
272-
return "auth.not_logged_in"
273279
case errors.Is(err, consent.ErrToolExecutionDenied):
274280
return "user.tool_denied"
275281
case errors.Is(err, git.ErrNotRepository):
@@ -374,6 +380,14 @@ func classifySuggestionType(err error) string {
374380
return "service.aad.failed"
375381
}
376382

383+
if errors.Is(err, auth.ErrNoCurrentUser) {
384+
return "auth.not_logged_in"
385+
}
386+
387+
if _, ok := errors.AsType[*azidentity.AuthenticationFailedError](err); ok {
388+
return "auth.identity_failed"
389+
}
390+
377391
if code := classifySentinel(err); code != "" {
378392
return code
379393
}

cli/azd/internal/cmd/errors_test.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"testing"
2222

2323
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
24+
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
2425
"github.com/azure/azure-dev/cli/azd/internal"
2526
"github.com/azure/azure-dev/cli/azd/internal/agent/consent"
2627
"github.com/azure/azure-dev/cli/azd/internal/tracing/fields"
@@ -201,6 +202,22 @@ func Test_MapError(t *testing.T) {
201202
fields.ErrorKey(fields.ServiceCorrelationId.Key).String("12345"),
202203
},
203204
},
205+
{
206+
name: "WithReLoginRequiredError",
207+
err: &auth.ReLoginRequiredError{},
208+
wantErrReason: "auth.login_required",
209+
wantErrDetails: []attribute.KeyValue{
210+
fields.ErrorKey(fields.ErrCategory.Key).String("auth"),
211+
},
212+
},
213+
{
214+
name: "WithAzidentityAuthenticationFailedError",
215+
err: &azidentity.AuthenticationFailedError{},
216+
wantErrReason: "auth.identity_failed",
217+
wantErrDetails: []attribute.KeyValue{
218+
fields.ErrorKey(fields.ErrCategory.Key).String("auth"),
219+
},
220+
},
204221
{
205222
name: "WithExtServiceError",
206223
err: &azdext.ServiceError{
@@ -274,16 +291,20 @@ func Test_MapError(t *testing.T) {
274291
wantErrDetails: nil,
275292
},
276293
{
277-
name: "WithErrNoCurrentUser",
278-
err: auth.ErrNoCurrentUser,
279-
wantErrReason: "auth.not_logged_in",
280-
wantErrDetails: nil,
294+
name: "WithErrNoCurrentUser",
295+
err: auth.ErrNoCurrentUser,
296+
wantErrReason: "auth.not_logged_in",
297+
wantErrDetails: []attribute.KeyValue{
298+
fields.ErrorKey(fields.ErrCategory.Key).String("auth"),
299+
},
281300
},
282301
{
283-
name: "WithWrappedErrNoCurrentUser",
284-
err: fmt.Errorf("failed to create credential: %w: %w", errors.New("inner"), auth.ErrNoCurrentUser),
285-
wantErrReason: "auth.not_logged_in",
286-
wantErrDetails: nil,
302+
name: "WithWrappedErrNoCurrentUser",
303+
err: fmt.Errorf("failed to create credential: %w: %w", errors.New("inner"), auth.ErrNoCurrentUser),
304+
wantErrReason: "auth.not_logged_in",
305+
wantErrDetails: []attribute.KeyValue{
306+
fields.ErrorKey(fields.ErrCategory.Key).String("auth"),
307+
},
287308
},
288309
{
289310
name: "WithErrToolExecutionDenied",
@@ -974,6 +995,14 @@ func Test_ClassifySuggestionType_MatchesMapError(t *testing.T) {
974995
Parsed: &auth.AadErrorResponse{Error: "invalid_grant"},
975996
},
976997
},
998+
{
999+
name: "ReLoginRequiredError",
1000+
err: &auth.ReLoginRequiredError{},
1001+
},
1002+
{
1003+
name: "AzidentityAuthenticationFailedError",
1004+
err: &azidentity.AuthenticationFailedError{},
1005+
},
9771006
// Sentinels
9781007
{name: "context.Canceled", err: context.Canceled},
9791008
{name: "context.DeadlineExceeded", err: context.DeadlineExceeded},

0 commit comments

Comments
 (0)