Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 1a463ba

Browse files
[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&amp;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 777c7a0 from #64004 Co-authored-by: Petri-Johan Last <[email protected]>
1 parent 074af1b commit 1a463ba

File tree

2 files changed

+3
-9
lines changed

2 files changed

+3
-9
lines changed

cmd/frontend/auth/user.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,8 @@ func GetAndSaveUser(
261261
// closure, to ensure we cover all exit paths correctly after the other mega
262262
// closure above.
263263
//
264-
// We only store the event if a new user was created, or if a new external
265-
// account was added to an existing user.
266-
if newUserSaved || extAcctSaved {
264+
// We only store the event if a new user was created.
265+
if newUserSaved {
267266
defer func() {
268267
action := telemetry.ActionSucceeded
269268
if err != nil { // check final error
@@ -288,7 +287,6 @@ func GetAndSaveUser(
288287
telemetry.EventMetadata{
289288
"serviceVariant": telemetry.Number(serviceVariant),
290289
// Track the various outcomes of the massive signup closure above.
291-
"newUserSaved": telemetry.Bool(newUserSaved),
292290
"extAcctSaved": telemetry.Bool(extAcctSaved),
293291
},
294292
op.UserCreateEventProperties,

cmd/frontend/auth/user_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ func TestGetAndSaveUser(t *testing.T) {
4343
expSafeErr string
4444
expErr error
4545
expNewUserCreated bool
46-
expExtAcctSaved bool
4746

4847
// expected side effects
4948
expSavedExtAccts map[int32][]extsvc.AccountSpec
@@ -123,7 +122,6 @@ func TestGetAndSaveUser(t *testing.T) {
123122
1: {ext("st1", "s1", "c1", "s1/u1")},
124123
},
125124
expNewUserCreated: false,
126-
expExtAcctSaved: true,
127125
},
128126
{
129127
description: "ext acct exists, username and email don't exist",
@@ -139,7 +137,6 @@ func TestGetAndSaveUser(t *testing.T) {
139137
1: {ext("st1", "s1", "c1", "s1/u1")},
140138
},
141139
expNewUserCreated: false,
142-
expExtAcctSaved: true,
143140
},
144141
{
145142
description: "ext acct exists, email belongs to another user",
@@ -155,7 +152,6 @@ func TestGetAndSaveUser(t *testing.T) {
155152
1: {ext("st1", "s1", "c1", "s1/u1")},
156153
},
157154
expNewUserCreated: false,
158-
expExtAcctSaved: true,
159155
},
160156
{
161157
description: "ext acct doesn't exist, user with username and email exists",
@@ -513,7 +509,7 @@ func TestGetAndSaveUser(t *testing.T) {
513509
// of user) attached, and all code paths should generate
514510
// at least 1 user event if a new user was created.
515511
gotEvents := eventsStore.CollectStoredEvents()
516-
if c.expNewUserCreated || c.expExtAcctSaved {
512+
if c.expNewUserCreated {
517513
assert.NotEmpty(t, gotEvents)
518514
} else {
519515
assert.Empty(t, gotEvents)

0 commit comments

Comments
 (0)