Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug Flag for Cookie Sync #3107

Merged
merged 19 commits into from
Nov 16, 2023
Merged

Debug Flag for Cookie Sync #3107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
53dd683
Debug flag framework
AlexBVolcy Aug 21, 2023
6cd2272
Add non fatal errors handling
AlexBVolcy Aug 23, 2023
ae6259c
Bidder Evaluation reporting
AlexBVolcy Sep 7, 2023
2d5afdc
Update how status is attached to response
AlexBVolcy Sep 10, 2023
0cf88cd
Add tests
AlexBVolcy Sep 11, 2023
2541956
Clean up, add tests
AlexBVolcy Sep 14, 2023
ff41f45
Merge branch 'master' into debug_flag_cookie_sync
AlexBVolcy Sep 14, 2023
cb33792
Minor improvements
AlexBVolcy Sep 20, 2023
96caaa9
Update tests
AlexBVolcy Sep 20, 2023
5ffbc7a
Address comments
AlexBVolcy Sep 28, 2023
e725769
Re-work debug structure in response
AlexBVolcy Oct 3, 2023
22a7777
Update name
AlexBVolcy Oct 5, 2023
2b837c6
Merge branch 'master' into debug_flag_cookie_sync
AlexBVolcy Oct 6, 2023
fdf26b4
Merge fix
AlexBVolcy Oct 6, 2023
008db67
Merge branch 'master' into debug_flag_cookie_sync
AlexBVolcy Oct 18, 2023
7c1759e
Merge branch 'master' into debug_flag_cookie_sync
AlexBVolcy Oct 23, 2023
b000c9e
Remove print stmts
AlexBVolcy Oct 23, 2023
f4a38bf
Remove print stmt
AlexBVolcy Oct 23, 2023
462295a
Add omitempty
AlexBVolcy Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 53 additions & 8 deletions endpoints/cookie_sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func NewCookieSyncEndpoint(
}

return &cookieSyncEndpoint{
chooser: usersync.NewChooser(syncersByBidder),
chooser: usersync.NewChooser(syncersByBidder, bidderHashSet),
config: config,
privacyConfig: usersyncPrivacyConfig{
gdprConfig: config.GDPR,
Expand Down Expand Up @@ -98,17 +98,18 @@ func (c *cookieSyncEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ ht
usersync.SyncHostCookie(r, cookie, &c.config.HostCookie)

result := c.chooser.Choose(request, cookie)

switch result.Status {
case usersync.StatusBlockedByUserOptOut:
c.metrics.RecordCookieSync(metrics.CookieSyncOptOut)
c.handleError(w, errCookieSyncOptOut, http.StatusUnauthorized)
case usersync.StatusBlockedByGDPR:
case usersync.StatusBlockedByPrivacy:
c.metrics.RecordCookieSync(metrics.CookieSyncGDPRHostCookieBlocked)
c.handleResponse(w, request.SyncTypeFilter, cookie, privacyMacros, nil)
c.handleResponse(w, request.SyncTypeFilter, cookie, privacyMacros, nil, result.BiddersEvaluated, request.Debug)
case usersync.StatusOK:
c.metrics.RecordCookieSync(metrics.CookieSyncOK)
c.writeSyncerMetrics(result.BiddersEvaluated)
c.handleResponse(w, request.SyncTypeFilter, cookie, privacyMacros, result.SyncersChosen)
c.handleResponse(w, request.SyncTypeFilter, cookie, privacyMacros, result.SyncersChosen, result.BiddersEvaluated, request.Debug)
}
}

Expand Down Expand Up @@ -172,6 +173,7 @@ func (c *cookieSyncEndpoint) parseRequest(r *http.Request) (usersync.Request, ma
Enabled: (request.CooperativeSync != nil && *request.CooperativeSync) || (request.CooperativeSync == nil && c.config.UserSync.Cooperative.EnabledByDefault),
PriorityGroups: c.config.UserSync.PriorityGroups,
},
Debug: request.Debug,
Limit: request.Limit,
Privacy: usersyncPrivacy{
gdprPermissions: gdprPerms,
Expand Down Expand Up @@ -387,9 +389,7 @@ func (c *cookieSyncEndpoint) writeSyncerMetrics(biddersEvaluated []usersync.Bidd
switch bidder.Status {
case usersync.StatusOK:
c.metrics.RecordSyncerRequest(bidder.SyncerKey, metrics.SyncerCookieSyncOK)
case usersync.StatusBlockedByGDPR:
c.metrics.RecordSyncerRequest(bidder.SyncerKey, metrics.SyncerCookieSyncPrivacyBlocked)
case usersync.StatusBlockedByCCPA:
case usersync.StatusBlockedByPrivacy:
Comment on lines -390 to +392
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious what's driving the change from regulation-specific statuses to a more generic privacy status. Was there a discussion about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scott and I did have a conversation a while ago and this name change was brought up I think in conjunction with removing CCPA from the evaluate() function. @SyntaxNode I think could touch on this more.

c.metrics.RecordSyncerRequest(bidder.SyncerKey, metrics.SyncerCookieSyncPrivacyBlocked)
case usersync.StatusAlreadySynced:
c.metrics.RecordSyncerRequest(bidder.SyncerKey, metrics.SyncerCookieSyncAlreadySynced)
Expand All @@ -399,7 +399,7 @@ func (c *cookieSyncEndpoint) writeSyncerMetrics(biddersEvaluated []usersync.Bidd
}
}

func (c *cookieSyncEndpoint) handleResponse(w http.ResponseWriter, tf usersync.SyncTypeFilter, co *usersync.Cookie, m macros.UserSyncPrivacy, s []usersync.SyncerChoice) {
func (c *cookieSyncEndpoint) handleResponse(w http.ResponseWriter, tf usersync.SyncTypeFilter, co *usersync.Cookie, m macros.UserSyncPrivacy, s []usersync.SyncerChoice, biddersEvaluated []usersync.BidderEvaluation, debug bool) {
status := "no_cookie"
if co.HasAnyLiveSyncs() {
status = "ok"
Expand Down Expand Up @@ -429,6 +429,24 @@ func (c *cookieSyncEndpoint) handleResponse(w http.ResponseWriter, tf usersync.S
})
}

if debug {
biddersSeen := make(map[string]struct{})
var debugInfo []cookieSyncResponseDebug
for _, bidderEval := range biddersEvaluated {
var debugResponse cookieSyncResponseDebug
debugResponse.Bidder = bidderEval.Bidder
if bidderEval.Status == usersync.StatusDuplicate && biddersSeen[bidderEval.Bidder] == struct{}{} {
debugResponse.Error = getDebugMessage(bidderEval.Status) + " synced as " + bidderEval.SyncerKey
debugInfo = append(debugInfo, debugResponse)
} else if bidderEval.Status != usersync.StatusOK {
debugResponse.Error = getDebugMessage(bidderEval.Status)
debugInfo = append(debugInfo, debugResponse)
}
biddersSeen[bidderEval.Bidder] = struct{}{}
}
response.Debug = debugInfo
}

c.pbsAnalytics.LogCookieSyncObject(&analytics.CookieSyncObject{
Status: http.StatusOK,
BidderStatus: mapBidderStatusToAnalytics(response.BidderStatus),
Expand Down Expand Up @@ -456,6 +474,26 @@ func mapBidderStatusToAnalytics(from []cookieSyncResponseBidder) []*analytics.Co
return to
}

func getDebugMessage(status usersync.Status) string {
switch status {
case usersync.StatusAlreadySynced:
return "Already in sync"
case usersync.StatusBlockedByPrivacy:
return "Rejected by privacy"
case usersync.StatusBlockedByUserOptOut:
return "Status blocked by user opt out"
case usersync.StatusDuplicate:
return "Duplicate bidder"
Comment on lines +485 to +486
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this equivalent to PBS-Java's "synced as FAMILY" status or is there a difference here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is the equivalent, and it'll have appended to it: synced as name_of_bidder_syncer

case usersync.StatusUnknownBidder:
return "Unsupported bidder"
case usersync.StatusUnconfiguredBidder:
return "No sync config"
case usersync.StatusTypeNotSupported:
return "Type not supported"
}
return ""
}

type cookieSyncRequest struct {
Bidders []string `json:"bidders"`
GDPR *int `json:"gdpr"`
Expand All @@ -467,6 +505,7 @@ type cookieSyncRequest struct {
CooperativeSync *bool `json:"coopSync"`
FilterSettings *cookieSyncRequestFilterSettings `json:"filterSettings"`
Account string `json:"account"`
Debug bool `json:"debug"`
}

type cookieSyncRequestFilterSettings struct {
Expand All @@ -482,6 +521,7 @@ type cookieSyncRequestFilter struct {
type cookieSyncResponse struct {
Status string `json:"status"`
BidderStatus []cookieSyncResponseBidder `json:"bidder_status"`
Debug []cookieSyncResponseDebug `json:"debug,omitempty"`
}

type cookieSyncResponseBidder struct {
Expand All @@ -496,6 +536,11 @@ type cookieSyncResponseSync struct {
SupportCORS bool `json:"supportCORS,omitempty"`
}

type cookieSyncResponseDebug struct {
Bidder string `json:"bidder"`
Error string `json:"error,omitempty"`
}

type usersyncPrivacyConfig struct {
gdprConfig config.GDPR
gdprPermissionsBuilder gdpr.PermissionsBuilder
Expand Down
69 changes: 64 additions & 5 deletions endpoints/cookie_sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func TestNewCookieSyncEndpoint(t *testing.T) {
analytics = MockAnalyticsRunner{}
fetcher = FakeAccountsFetcher{}
bidders = map[string]openrtb_ext.BidderName{"bidderA": openrtb_ext.BidderName("bidderA"), "bidderB": openrtb_ext.BidderName("bidderB")}
biddersKnown = map[string]struct{}{"bidderA": {}, "bidderB": {}}
)

endpoint := NewCookieSyncEndpoint(
Expand All @@ -65,7 +66,7 @@ func TestNewCookieSyncEndpoint(t *testing.T) {
result := endpoint.(*cookieSyncEndpoint)

expected := &cookieSyncEndpoint{
chooser: usersync.NewChooser(syncersByBidder),
chooser: usersync.NewChooser(syncersByBidder, biddersKnown),
config: &config.Configuration{
UserSync: configUserSync,
HostCookie: configHostCookie,
Expand Down Expand Up @@ -231,7 +232,7 @@ func TestCookieSyncHandle(t *testing.T) {
givenCookie: cookieWithSyncs,
givenBody: strings.NewReader(`{}`),
givenChooserResult: usersync.Result{
Status: usersync.StatusBlockedByGDPR,
Status: usersync.StatusBlockedByPrivacy,
BiddersEvaluated: []usersync.BidderEvaluation{{Bidder: "a", SyncerKey: "aSyncer", Status: usersync.StatusOK}},
SyncersChosen: []usersync.SyncerChoice{{Bidder: "a", Syncer: &syncer}},
},
Expand All @@ -249,6 +250,38 @@ func TestCookieSyncHandle(t *testing.T) {
a.On("LogCookieSyncObject", &expected).Once()
},
},
{
description: "Debug Check",
givenCookie: cookieWithSyncs,
givenBody: strings.NewReader(`{"debug": true}`),
givenChooserResult: usersync.Result{
Status: usersync.StatusOK,
BiddersEvaluated: []usersync.BidderEvaluation{{Bidder: "a", SyncerKey: "aSyncer", Status: usersync.StatusAlreadySynced}},
SyncersChosen: []usersync.SyncerChoice{{Bidder: "a", Syncer: &syncer}},
},
expectedStatusCode: 200,
expectedBody: `{"status":"ok","bidder_status":[` +
`{"bidder":"a","no_cookie":true,"usersync":{"url":"aURL","type":"redirect","supportCORS":true}}` +
`],"debug":[{"bidder":"a","error":"Already in sync"}]}` + "\n",
setMetricsExpectations: func(m *metrics.MetricsEngineMock) {
m.On("RecordCookieSync", metrics.CookieSyncOK).Once()
m.On("RecordSyncerRequest", "aSyncer", metrics.SyncerCookieSyncAlreadySynced).Once()
},
setAnalyticsExpectations: func(a *MockAnalyticsRunner) {
expected := analytics.CookieSyncObject{
Status: 200,
Errors: nil,
BidderStatus: []*analytics.CookieSyncBidder{
{
BidderCode: "a",
NoCookie: true,
UsersyncInfo: &analytics.UsersyncInfo{URL: "aURL", Type: "redirect", SupportCORS: true},
},
},
}
a.On("LogCookieSyncObject", &expected).Once()
},
},
}

for _, test := range testCases {
Expand Down Expand Up @@ -1508,14 +1541,14 @@ func TestCookieSyncWriteBidderMetrics(t *testing.T) {
},
{
description: "One - Blocked By GDPR",
given: []usersync.BidderEvaluation{{Bidder: "a", SyncerKey: "aSyncer", Status: usersync.StatusBlockedByGDPR}},
given: []usersync.BidderEvaluation{{Bidder: "a", SyncerKey: "aSyncer", Status: usersync.StatusBlockedByPrivacy}},
setExpectations: func(m *metrics.MetricsEngineMock) {
m.On("RecordSyncerRequest", "aSyncer", metrics.SyncerCookieSyncPrivacyBlocked).Once()
},
},
{
description: "One - Blocked By CCPA",
given: []usersync.BidderEvaluation{{Bidder: "a", SyncerKey: "aSyncer", Status: usersync.StatusBlockedByCCPA}},
given: []usersync.BidderEvaluation{{Bidder: "a", SyncerKey: "aSyncer", Status: usersync.StatusBlockedByPrivacy}},
setExpectations: func(m *metrics.MetricsEngineMock) {
m.On("RecordSyncerRequest", "aSyncer", metrics.SyncerCookieSyncPrivacyBlocked).Once()
},
Expand Down Expand Up @@ -1580,10 +1613,21 @@ func TestCookieSyncHandleResponse(t *testing.T) {
syncerWithError := MockSyncer{}
syncerWithError.On("GetSync", syncTypeExpected, privacyMacros).Return(syncWithError, errors.New("anyError")).Maybe()

bidderEvalForDebug := []usersync.BidderEvaluation{
{Bidder: "Bidder1", Status: usersync.StatusAlreadySynced},
{Bidder: "Bidder2", Status: usersync.StatusUnknownBidder},
{Bidder: "Bidder3", Status: usersync.StatusUnconfiguredBidder},
{Bidder: "Bidder4", Status: usersync.StatusBlockedByPrivacy},
{Bidder: "Bidder5", Status: usersync.StatusTypeNotSupported},
{Bidder: "Bidder6", Status: usersync.StatusBlockedByUserOptOut},
{Bidder: "BidderA", Status: usersync.StatusDuplicate, SyncerKey: "syncerB"},
}
AlexBVolcy marked this conversation as resolved.
Show resolved Hide resolved

testCases := []struct {
description string
givenCookieHasSyncs bool
givenSyncersChosen []usersync.SyncerChoice
givenDebug bool
expectedJSON string
expectedAnalytics analytics.CookieSyncObject
}{
Expand Down Expand Up @@ -1661,6 +1705,14 @@ func TestCookieSyncHandleResponse(t *testing.T) {
expectedJSON: `{"status":"no_cookie","bidder_status":[]}` + "\n",
expectedAnalytics: analytics.CookieSyncObject{Status: 200, BidderStatus: []*analytics.CookieSyncBidder{}},
},
{
description: "Debug is true, should see all rejected bidder eval statuses in response",
givenCookieHasSyncs: true,
givenDebug: true,
givenSyncersChosen: []usersync.SyncerChoice{},
expectedJSON: `{"status":"ok","bidder_status":[],"debug":[{"bidder":"Bidder1","error":"Already in sync"},{"bidder":"Bidder2","error":"Unsupported bidder"},{"bidder":"Bidder3","error":"No sync config"},{"bidder":"Bidder4","error":"Rejected by privacy"},{"bidder":"Bidder5","error":"Type not supported"},{"bidder":"Bidder6","error":"Status blocked by user opt out"},{"bidder":"BidderA","error":"Duplicate bidder synced as syncerB"}]}` + "\n",
expectedAnalytics: analytics.CookieSyncObject{Status: 200, BidderStatus: []*analytics.CookieSyncBidder{}},
},
}

for _, test := range testCases {
Expand All @@ -1676,7 +1728,14 @@ func TestCookieSyncHandleResponse(t *testing.T) {

writer := httptest.NewRecorder()
endpoint := cookieSyncEndpoint{pbsAnalytics: &mockAnalytics}
endpoint.handleResponse(writer, syncTypeFilter, cookie, privacyMacros, test.givenSyncersChosen)

var bidderEval []usersync.BidderEvaluation
if test.givenDebug {
bidderEval = bidderEvalForDebug
} else {
bidderEval = []usersync.BidderEvaluation{}
}
endpoint.handleResponse(writer, syncTypeFilter, cookie, privacyMacros, test.givenSyncersChosen, bidderEval, test.givenDebug)

if assert.Equal(t, writer.Code, http.StatusOK, test.description+":http_status") {
assert.Equal(t, writer.Header().Get("Content-Type"), "application/json; charset=utf-8", test.description+":http_header")
Expand Down
35 changes: 20 additions & 15 deletions usersync/chooser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ type Chooser interface {
}

// NewChooser returns a new instance of the standard chooser implementation.
func NewChooser(bidderSyncerLookup map[string]Syncer) Chooser {
func NewChooser(bidderSyncerLookup map[string]Syncer, biddersKnown map[string]struct{}) Chooser {
bidders := make([]string, 0, len(bidderSyncerLookup))

for k := range bidderSyncerLookup {
bidders = append(bidders, k)
}
Expand All @@ -25,6 +26,7 @@ func NewChooser(bidderSyncerLookup map[string]Syncer) Chooser {
biddersAvailable: bidders,
bidderChooser: standardBidderChooser{shuffler: randomShuffler{}},
normalizeValidBidderName: openrtb_ext.NormalizeBidderName,
biddersKnown: biddersKnown,
}
}

Expand All @@ -35,6 +37,7 @@ type Request struct {
Limit int
Privacy Privacy
SyncTypeFilter SyncTypeFilter
Debug bool
}

// Cooperative specifies the settings for cooperative syncing for a given request, where bidders
Expand Down Expand Up @@ -74,13 +77,6 @@ const (
// StatusBlockedByUserOptOut specifies a user's cookie explicitly signals an opt-out.
StatusBlockedByUserOptOut

// StatusBlockedByGDPR specifies a user's GDPR TCF consent explicitly forbids host cookies
// or specific bidder syncing.
StatusBlockedByGDPR

// StatusBlockedByCCPA specifies a user's CCPA consent explicitly forbids bidder syncing.
StatusBlockedByCCPA

// StatusAlreadySynced specifies a user's cookie has an existing non-expired sync for a specific bidder.
StatusAlreadySynced

Expand All @@ -95,6 +91,9 @@ const (

// StatusBlockedByPrivacy specifies a bidder sync url is not allowed by privacy activities
StatusBlockedByPrivacy

// StatusUnconfiguredBidder refers to a bidder who hasn't been configured to have a syncer key, but is known by Prebid Server
StatusUnconfiguredBidder
AlexBVolcy marked this conversation as resolved.
Show resolved Hide resolved
AlexBVolcy marked this conversation as resolved.
Show resolved Hide resolved
)

// Privacy determines which privacy policies will be enforced for a user sync request.
Expand All @@ -111,6 +110,7 @@ type standardChooser struct {
biddersAvailable []string
bidderChooser bidderChooser
normalizeValidBidderName func(name string) (openrtb_ext.BidderName, bool)
biddersKnown map[string]struct{}
}

// Choose randomly selects user syncers which are permitted by the user's privacy settings and
Expand All @@ -121,23 +121,28 @@ func (c standardChooser) Choose(request Request, cookie *Cookie) Result {
}

if !request.Privacy.GDPRAllowsHostCookie() {
return Result{Status: StatusBlockedByGDPR}
return Result{Status: StatusBlockedByPrivacy}
}

syncersSeen := make(map[string]struct{})
biddersSeen := make(map[string]struct{})
limitDisabled := request.Limit <= 0

biddersEvaluated := make([]BidderEvaluation, 0)
syncersChosen := make([]SyncerChoice, 0)

bidders := c.bidderChooser.choose(request.Bidders, c.biddersAvailable, request.Cooperative)
for i := 0; i < len(bidders) && (limitDisabled || len(syncersChosen) < request.Limit); i++ {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is necessary, but I see that PBS-Java provides debug information indicating when a bidder is ignored because the limit has been reached. Similarly they are indicating if a bidder is known but disabled. We should discuss whether we want to add this logic, perhaps in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, yeah let's talk about this at the next team time.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. PBS-Go behavior is different from PBS-Java. We stop processing when we reach the limit so there's no need to report on this.

if _, ok := biddersSeen[bidders[i]]; ok {
continue
}
syncer, evaluation := c.evaluate(bidders[i], syncersSeen, request.SyncTypeFilter, request.Privacy, cookie)

biddersEvaluated = append(biddersEvaluated, evaluation)
if evaluation.Status == StatusOK {
syncersChosen = append(syncersChosen, SyncerChoice{Bidder: bidders[i], Syncer: syncer})
}
biddersSeen[bidders[i]] = struct{}{}
}

return Result{Status: StatusOK, BiddersEvaluated: biddersEvaluated, SyncersChosen: syncersChosen}
Expand All @@ -151,7 +156,11 @@ func (c standardChooser) evaluate(bidder string, syncersSeen map[string]struct{}

AlexBVolcy marked this conversation as resolved.
Show resolved Hide resolved
syncer, exists := c.bidderSyncerLookup[bidderNormalized.String()]
if !exists {
return nil, BidderEvaluation{Status: StatusUnknownBidder, Bidder: bidder}
if _, ok := c.biddersKnown[bidder]; !ok {
return nil, BidderEvaluation{Status: StatusUnknownBidder, Bidder: bidder}
} else {
return nil, BidderEvaluation{Status: StatusUnconfiguredBidder, Bidder: bidder}
}
}

_, seen := syncersSeen[syncer.Key()]
Expand All @@ -174,11 +183,7 @@ func (c standardChooser) evaluate(bidder string, syncersSeen map[string]struct{}
}

if !privacy.GDPRAllowsBidderSync(bidderNormalized.String()) {
return nil, BidderEvaluation{Status: StatusBlockedByGDPR, Bidder: bidder, SyncerKey: syncer.Key()}
}

if !privacy.CCPAAllowsBidderSync(bidderNormalized.String()) {
return nil, BidderEvaluation{Status: StatusBlockedByCCPA, Bidder: bidder, SyncerKey: syncer.Key()}
return nil, BidderEvaluation{Status: StatusBlockedByPrivacy, Bidder: bidder, SyncerKey: syncer.Key()}
}

return syncer, BidderEvaluation{Status: StatusOK, Bidder: bidder, SyncerKey: syncer.Key()}
Expand Down
Loading
Loading