[fix] Only trigger externalAcctSignup event when a new user is created or external account is saved (#63843)

cmd/frontend/auth/user.go

@@ -260,38 +260,43 @@ func GetAndSaveUser(
 	// We handle all V2 telemetry related to GetAndSaveUser within this defer
 	// closure, to ensure we cover all exit paths correctly after the other mega
 	// closure above.
-	defer func() {
-		action := telemetry.ActionSucceeded
-		if err != nil { // check final error
-			action = telemetry.ActionFailed
-		}
+	//
+	// We only store the event if a new user was created, or if a new external
+	// account was added to an existing user.
+	if newUserSaved || extAcctSaved {
+		defer func() {
+			action := telemetry.ActionSucceeded
+			if err != nil { // check final error
+				action = telemetry.ActionFailed
+			}
 
-		// Most auth providers services have an exstvc.Variant, so try and
-		// extract that from the account spec. For ease of use in we also
-		// preserve the raw value in the private metadata.
-		serviceVariant, _ := extsvc.VariantValueOf(acct.AccountSpec.ServiceType)
-		privateMetadata := map[string]any{"serviceType": acct.AccountSpec.ServiceType}
+			// Most auth providers services have an exstvc.Variant, so try and
+			// extract that from the account spec. For ease of use in we also
+			// preserve the raw value in the private metadata.
+			serviceVariant, _ := extsvc.VariantValueOf(acct.AccountSpec.ServiceType)
+			privateMetadata := map[string]any{"serviceType": acct.AccountSpec.ServiceType}
 
-		// Include safe err if there is one for maybe-useful diagnostics
-		if len(safeErrMsg) > 0 {
-			privateMetadata["safeErrMsg"] = safeErrMsg
-		}
+			// Include safe err if there is one for maybe-useful diagnostics
+			if len(safeErrMsg) > 0 {
+				privateMetadata["safeErrMsg"] = safeErrMsg
+			}
 
-		// Record our V2 event.
-		recorder.Record(telemetryCtx, telemetryV2UserSignUpFeatureName, action, &telemetry.EventParameters{
-			Version: 2, // We've significantly refactored telemetryV2UserSignUpFeatureName occurrences
-			Metadata: telemetry.MergeMetadata(
-				telemetry.EventMetadata{
-					"serviceVariant": telemetry.Number(serviceVariant),
-					// Track the various outcomes of the massive signup closure above.
-					"newUserSaved": telemetry.Bool(newUserSaved),
-					"extAcctSaved": telemetry.Bool(extAcctSaved),
-				},
-				op.UserCreateEventProperties,
-			),
-			PrivateMetadata: privateMetadata,
-		})
-	}()
+			// Record our V2 event.
+			recorder.Record(telemetryCtx, telemetryV2UserSignUpFeatureName, action, &telemetry.EventParameters{
+				Version: 2, // We've significantly refactored telemetryV2UserSignUpFeatureName occurrences
+				Metadata: telemetry.MergeMetadata(
+					telemetry.EventMetadata{
+						"serviceVariant": telemetry.Number(serviceVariant),
+						// Track the various outcomes of the massive signup closure above.
+						"newUserSaved": telemetry.Bool(newUserSaved),
+						"extAcctSaved": telemetry.Bool(extAcctSaved),
+					},
+					op.UserCreateEventProperties,
+				),
+				PrivateMetadata: privateMetadata,
+			})
+		}()
+	}
 
 	if err != nil {
 		// Legacy event - retain because it is still exported by the legacy

cmd/frontend/auth/user_test.go

@@ -43,6 +43,7 @@ func TestGetAndSaveUser(t *testing.T) {
 		expSafeErr        string
 		expErr            error
 		expNewUserCreated bool
+		expExtAcctSaved   bool
 
 		// expected side effects
 		expSavedExtAccts                 map[int32][]extsvc.AccountSpec
@@ -122,6 +123,7 @@ func TestGetAndSaveUser(t *testing.T) {
 					1: {ext("st1", "s1", "c1", "s1/u1")},
 				},
 				expNewUserCreated: false,
+				expExtAcctSaved:   true,
 			},
 			{
 				description: "ext acct exists, username and email don't exist",
@@ -137,6 +139,7 @@ func TestGetAndSaveUser(t *testing.T) {
 					1: {ext("st1", "s1", "c1", "s1/u1")},
 				},
 				expNewUserCreated: false,
+				expExtAcctSaved:   true,
 			},
 			{
 				description: "ext acct exists, email belongs to another user",
@@ -152,6 +155,7 @@ func TestGetAndSaveUser(t *testing.T) {
 					1: {ext("st1", "s1", "c1", "s1/u1")},
 				},
 				expNewUserCreated: false,
+				expExtAcctSaved:   true,
 			},
 			{
 				description: "ext acct doesn't exist, user with username and email exists",
@@ -507,9 +511,13 @@ func TestGetAndSaveUser(t *testing.T) {
 
 						// All telemetry should have the expected user (or lack
 						// of user) attached, and all code paths should generate
-						// at least 1 user event.
+						// at least 1 user event if a new user was created.
 						gotEvents := eventsStore.CollectStoredEvents()
-						assert.NotEmpty(t, gotEvents)
+						if c.expNewUserCreated || c.expExtAcctSaved {
+							assert.NotEmpty(t, gotEvents)
+						} else {
+							assert.Empty(t, gotEvents)
+						}
 						for _, ev := range gotEvents {
 							switch {
 							// We are expecting a specific user ID