[Backport 5.5.x] [logging] Only record events if a new user was created (#64005)

Follow-up on https://github.com/sourcegraph/sourcegraph/pull/63843

Based on comments from
[this](https://sourcegraph.slack.com/archives/C04RG0JD8L9/p1721668767261719?thread_ts=1721661216.365709&cid=C04RG0JD8L9)
Slack thread, it seems like the events causing the spam are ones where a
new ext acct is saved without a user being created. So if we want to fix
the spam we need to only save an event if a user was created.



## Test plan

Test updated.



## Changelog


 <br> Backport 777c7a0899 from #64004

Co-authored-by: Petri-Johan Last <petri.last@sourcegraph.com>
This commit is contained in:
Release Bot 2024-07-23 05:09:11 -07:00 committed by GitHub
parent 074af1bda8
commit 1a463ba167
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 3 additions and 9 deletions

View File

@ -261,9 +261,8 @@ func GetAndSaveUser(
// closure, to ensure we cover all exit paths correctly after the other mega
// closure above.
//
// 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 {
// We only store the event if a new user was created.
if newUserSaved {
defer func() {
action := telemetry.ActionSucceeded
if err != nil { // check final error
@ -288,7 +287,6 @@ func GetAndSaveUser(
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,

View File

@ -43,7 +43,6 @@ func TestGetAndSaveUser(t *testing.T) {
expSafeErr string
expErr error
expNewUserCreated bool
expExtAcctSaved bool
// expected side effects
expSavedExtAccts map[int32][]extsvc.AccountSpec
@ -123,7 +122,6 @@ 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",
@ -139,7 +137,6 @@ 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",
@ -155,7 +152,6 @@ 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",
@ -513,7 +509,7 @@ func TestGetAndSaveUser(t *testing.T) {
// of user) attached, and all code paths should generate
// at least 1 user event if a new user was created.
gotEvents := eventsStore.CollectStoredEvents()
if c.expNewUserCreated || c.expExtAcctSaved {
if c.expNewUserCreated {
assert.NotEmpty(t, gotEvents)
} else {
assert.Empty(t, gotEvents)