Skip to content

Commit 8cf6dd2

Browse files
karwaankit32karwaank
karwaankit32
and
karwaank
authored
Rules Engine Module: Add ruleset filter warnings in debug auction (prebid#4896)
Co-authored-by: Ankit Karwa <ankitkarwa@microsoft.com>
1 parent 43edc8b commit 8cf6dd2

10 files changed

Lines changed: 838 additions & 12 deletions

File tree

‎hooks/hookexecution/enricher_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,44 @@ func TestGetModulesJSON(t *testing.T) {
267267
}
268268
}
269269

270+
func TestGetModulesJSONIncludesHookWarnings(t *testing.T) {
271+
stageOutcomes := []StageOutcome{
272+
{
273+
Stage: "processed_auction_request",
274+
Groups: []GroupOutcome{
275+
{
276+
InvocationResults: []HookOutcome{
277+
{
278+
HookID: HookID{
279+
ModuleCode: "prebid.rulesengine",
280+
HookImplCode: "rulesengine",
281+
},
282+
Status: StatusSuccess,
283+
Warnings: []string{"Bidder [testBidder] was removed from the request by the rules engine"},
284+
},
285+
},
286+
},
287+
},
288+
},
289+
}
290+
bidRequest := &openrtb2.BidRequest{Test: 1, Ext: []byte(`{"prebid":{"trace":"basic"}}`)}
291+
account := &config.Account{DebugAllow: true}
292+
293+
modules, warns, err := GetModulesJSON(stageOutcomes, bidRequest, account)
294+
295+
require.NoError(t, err)
296+
assert.Empty(t, warns)
297+
var modulesOutcome ModulesOutcome
298+
require.NoError(t, jsonutil.UnmarshalValid(modules, &modulesOutcome))
299+
assert.Equal(t, Messages{
300+
"prebid.rulesengine": {
301+
"rulesengine": {"Bidder [testBidder] was removed from the request by the rules engine"},
302+
},
303+
}, modulesOutcome.Warnings)
304+
require.NotNil(t, modulesOutcome.Trace)
305+
assert.Len(t, modulesOutcome.Trace.Stages, 1)
306+
}
307+
270308
func getStageOutcomes(t *testing.T, file string) []StageOutcome {
271309
var stageOutcomes []StageOutcome
272310
var stageOutcomesTest []StageOutcomeTest

‎modules/prebid/rulesengine/bidder_config_ruleset.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ func buildBidderConfigRuleSet(geoscopes map[string][]string, setDefinitions map[
2525
return nil, err
2626
}
2727
crs.modelGroups[0].tree = *tree
28+
// Propagate the analytics key and model version onto the tree so they are available in the
29+
// ResultFunctionMeta at execution time (e.g. for surfacing them in exclusion warnings).
30+
crs.modelGroups[0].tree.AnalyticsKey = crs.modelGroups[0].analyticsKey
31+
crs.modelGroups[0].tree.ModelVersion = crs.modelGroups[0].version
2832

2933
return []cacheRuleSet[RequestWrapper, ProcessedAuctionHookResult]{crs}, nil
3034
}

‎modules/prebid/rulesengine/cache_entry.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ func createCacheRuleSet(cfg *config.RuleSet) (cacheRuleSet[openrtb_ext.RequestWr
106106
analyticsKey: modelGroup.AnalyticsKey,
107107
tree: *tree,
108108
}
109+
// Propagate the ruleset name, analytics key and model version onto the tree so they are
110+
// available in the ResultFunctionMeta at execution time (e.g. for surfacing them in exclusion
111+
// warnings). The ruleset name is used for display; the analytics key stays exactly as
112+
// configured (it identifies the model group for analytics).
113+
cmg.tree.RulesetName = cfg.Name
114+
cmg.tree.AnalyticsKey = modelGroup.AnalyticsKey
115+
cmg.tree.ModelVersion = modelGroup.Version
109116
crs.modelGroups = append(crs.modelGroups, cmg)
110117
}
111118

‎modules/prebid/rulesengine/hook_processed_auction.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,21 @@ import (
55

66
hs "github.com/prebid/prebid-server/v4/hooks/hookstage"
77
"github.com/prebid/prebid-server/v4/openrtb_ext"
8+
"github.com/prebid/prebid-server/v4/rules"
89
"github.com/prebid/prebid-server/v4/util/randomutil"
910
)
1011

1112
type RequestWrapper = openrtb_ext.RequestWrapper
1213
type ModelGroup = cacheModelGroup[RequestWrapper, ProcessedAuctionHookResult]
1314

15+
type includeBiddersState struct {
16+
allowedBidders map[string]struct{}
17+
contexts []rules.ResultFunctionMeta
18+
}
19+
1420
type ProcessedAuctionHookResult struct {
1521
HookResult hs.HookResult[hs.ProcessedAuctionRequestPayload]
16-
AllowedBidders map[string]struct{}
22+
IncludeBidders includeBiddersState
1723
}
1824

1925
func handleProcessedAuctionHook(
@@ -24,7 +30,9 @@ func handleProcessedAuctionHook(
2430
HookResult: hs.HookResult[hs.ProcessedAuctionRequestPayload]{
2531
ChangeSet: hs.ChangeSet[hs.ProcessedAuctionRequestPayload]{},
2632
},
27-
AllowedBidders: make(map[string]struct{}),
33+
IncludeBidders: includeBiddersState{
34+
allowedBidders: make(map[string]struct{}),
35+
},
2836
}
2937

3038
for _, ruleSet := range ruleSets {
@@ -39,11 +47,15 @@ func handleProcessedAuctionHook(
3947
result.HookResult.Errors = append(result.HookResult.Errors, err.Error())
4048
}
4149

42-
if len(result.AllowedBidders) > 0 {
43-
result.HookResult.ChangeSet.ProcessedAuctionRequest().Bidders().Add(result.AllowedBidders)
50+
if len(result.IncludeBidders.allowedBidders) > 0 {
51+
result.HookResult.ChangeSet.ProcessedAuctionRequest().Bidders().Add(result.IncludeBidders.allowedBidders)
4452
}
4553
}
4654

55+
// Once every ruleset has run the final allow-list is known, so surface a debug warning naming the
56+
// bidders that were implicitly removed by include rules (present in the request but not allowed).
57+
appendInclusionWarnings(payload.Request, &result)
58+
4759
return result.HookResult, nil
4860
}
4961

‎modules/prebid/rulesengine/module_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package rulesengine
22

33
import (
4+
"context"
45
"encoding/json"
56
"os"
67
"path/filepath"
78
"testing"
89
"time"
910

11+
hs "github.com/prebid/prebid-server/v4/hooks/hookstage"
1012
"github.com/prebid/prebid-server/v4/modules/moduledeps"
1113
"github.com/stretchr/testify/assert"
1214
)
@@ -107,6 +109,36 @@ func TestBuilderWithWorkingDir(t *testing.T) {
107109

108110
var sampleJsonConfig json.RawMessage = json.RawMessage(`{"enabled": true, "ruleSets": []}`)
109111

112+
// TestHandleProcessedAuctionHookNoConfig verifies that when the account has no rules engine
113+
// configuration, the hook short-circuits and returns an empty result with no warnings or errors,
114+
// without touching the cache or tree manager (so a zero-value Module is safe here).
115+
func TestHandleProcessedAuctionHookNoConfig(t *testing.T) {
116+
tests := []struct {
117+
name string
118+
accountConfig json.RawMessage
119+
}{
120+
{name: "nil_account_config", accountConfig: nil},
121+
{name: "empty_account_config", accountConfig: json.RawMessage{}},
122+
}
123+
124+
for _, tt := range tests {
125+
t.Run(tt.name, func(t *testing.T) {
126+
m := Module{}
127+
128+
result, err := m.HandleProcessedAuctionHook(
129+
context.Background(),
130+
hs.ModuleInvocationContext{AccountID: "account-1", AccountConfig: tt.accountConfig},
131+
hs.ProcessedAuctionRequestPayload{},
132+
)
133+
134+
assert.NoError(t, err)
135+
assert.Equal(t, hs.HookResult[hs.ProcessedAuctionRequestPayload]{}, result)
136+
assert.Empty(t, result.Warnings)
137+
assert.Empty(t, result.Errors)
138+
})
139+
}
140+
}
141+
110142
func TestConfigChanged(t *testing.T) {
111143

112144
testCases := []struct {

0 commit comments

Comments
 (0)