diff --git a/hooks/hookexecution/enricher_test.go b/hooks/hookexecution/enricher_test.go index d25b30b4071..6e270616064 100644 --- a/hooks/hookexecution/enricher_test.go +++ b/hooks/hookexecution/enricher_test.go @@ -267,6 +267,44 @@ func TestGetModulesJSON(t *testing.T) { } } +func TestGetModulesJSONIncludesHookWarnings(t *testing.T) { + stageOutcomes := []StageOutcome{ + { + Stage: "processed_auction_request", + Groups: []GroupOutcome{ + { + InvocationResults: []HookOutcome{ + { + HookID: HookID{ + ModuleCode: "prebid.rulesengine", + HookImplCode: "rulesengine", + }, + Status: StatusSuccess, + Warnings: []string{"Bidder [testBidder] was removed from the request by the rules engine"}, + }, + }, + }, + }, + }, + } + bidRequest := &openrtb2.BidRequest{Test: 1, Ext: []byte(`{"prebid":{"trace":"basic"}}`)} + account := &config.Account{DebugAllow: true} + + modules, warns, err := GetModulesJSON(stageOutcomes, bidRequest, account) + + require.NoError(t, err) + assert.Empty(t, warns) + var modulesOutcome ModulesOutcome + require.NoError(t, jsonutil.UnmarshalValid(modules, &modulesOutcome)) + assert.Equal(t, Messages{ + "prebid.rulesengine": { + "rulesengine": {"Bidder [testBidder] was removed from the request by the rules engine"}, + }, + }, modulesOutcome.Warnings) + require.NotNil(t, modulesOutcome.Trace) + assert.Len(t, modulesOutcome.Trace.Stages, 1) +} + func getStageOutcomes(t *testing.T, file string) []StageOutcome { var stageOutcomes []StageOutcome var stageOutcomesTest []StageOutcomeTest diff --git a/modules/prebid/rulesengine/bidder_config_ruleset.go b/modules/prebid/rulesengine/bidder_config_ruleset.go index 59d2c0079ba..4a2c4d8c58c 100644 --- a/modules/prebid/rulesengine/bidder_config_ruleset.go +++ b/modules/prebid/rulesengine/bidder_config_ruleset.go @@ -25,6 +25,10 @@ func buildBidderConfigRuleSet(geoscopes map[string][]string, setDefinitions map[ return nil, err } crs.modelGroups[0].tree = *tree + // Propagate the analytics key and model version onto the tree so they are available in the + // ResultFunctionMeta at execution time (e.g. for surfacing them in exclusion warnings). + crs.modelGroups[0].tree.AnalyticsKey = crs.modelGroups[0].analyticsKey + crs.modelGroups[0].tree.ModelVersion = crs.modelGroups[0].version return []cacheRuleSet[RequestWrapper, ProcessedAuctionHookResult]{crs}, nil } diff --git a/modules/prebid/rulesengine/cache_entry.go b/modules/prebid/rulesengine/cache_entry.go index 5f912d18272..4acac5dceb2 100644 --- a/modules/prebid/rulesengine/cache_entry.go +++ b/modules/prebid/rulesengine/cache_entry.go @@ -106,6 +106,13 @@ func createCacheRuleSet(cfg *config.RuleSet) (cacheRuleSet[openrtb_ext.RequestWr analyticsKey: modelGroup.AnalyticsKey, tree: *tree, } + // Propagate the ruleset name, analytics key and model version onto the tree so they are + // available in the ResultFunctionMeta at execution time (e.g. for surfacing them in exclusion + // warnings). The ruleset name is used for display; the analytics key stays exactly as + // configured (it identifies the model group for analytics). + cmg.tree.RulesetName = cfg.Name + cmg.tree.AnalyticsKey = modelGroup.AnalyticsKey + cmg.tree.ModelVersion = modelGroup.Version crs.modelGroups = append(crs.modelGroups, cmg) } diff --git a/modules/prebid/rulesengine/hook_processed_auction.go b/modules/prebid/rulesengine/hook_processed_auction.go index dad68913538..a54e08f1c2f 100644 --- a/modules/prebid/rulesengine/hook_processed_auction.go +++ b/modules/prebid/rulesengine/hook_processed_auction.go @@ -5,6 +5,7 @@ import ( hs "github.com/prebid/prebid-server/v4/hooks/hookstage" "github.com/prebid/prebid-server/v4/openrtb_ext" + "github.com/prebid/prebid-server/v4/rules" "github.com/prebid/prebid-server/v4/util/randomutil" ) @@ -12,8 +13,9 @@ type RequestWrapper = openrtb_ext.RequestWrapper type ModelGroup = cacheModelGroup[RequestWrapper, ProcessedAuctionHookResult] type ProcessedAuctionHookResult struct { - HookResult hs.HookResult[hs.ProcessedAuctionRequestPayload] - AllowedBidders map[string]struct{} + HookResult hs.HookResult[hs.ProcessedAuctionRequestPayload] + AllowedBidders map[string]struct{} + IncludeContexts []rules.ResultFunctionMeta } func handleProcessedAuctionHook( @@ -44,6 +46,10 @@ func handleProcessedAuctionHook( } } + // Once every ruleset has run the final allow-list is known, so surface a debug warning naming the + // bidders that were implicitly removed by include rules (present in the request but not allowed). + appendInclusionWarnings(payload.Request, &result) + return result.HookResult, nil } diff --git a/modules/prebid/rulesengine/module_test.go b/modules/prebid/rulesengine/module_test.go index 9bffeb88168..8b3dc225be4 100644 --- a/modules/prebid/rulesengine/module_test.go +++ b/modules/prebid/rulesengine/module_test.go @@ -1,12 +1,14 @@ package rulesengine import ( + "context" "encoding/json" "os" "path/filepath" "testing" "time" + hs "github.com/prebid/prebid-server/v4/hooks/hookstage" "github.com/prebid/prebid-server/v4/modules/moduledeps" "github.com/stretchr/testify/assert" ) @@ -107,6 +109,36 @@ func TestBuilderWithWorkingDir(t *testing.T) { var sampleJsonConfig json.RawMessage = json.RawMessage(`{"enabled": true, "ruleSets": []}`) +// TestHandleProcessedAuctionHookNoConfig verifies that when the account has no rules engine +// configuration, the hook short-circuits and returns an empty result with no warnings or errors, +// without touching the cache or tree manager (so a zero-value Module is safe here). +func TestHandleProcessedAuctionHookNoConfig(t *testing.T) { + tests := []struct { + name string + accountConfig json.RawMessage + }{ + {name: "nil_account_config", accountConfig: nil}, + {name: "empty_account_config", accountConfig: json.RawMessage{}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := Module{} + + result, err := m.HandleProcessedAuctionHook( + context.Background(), + hs.ModuleInvocationContext{AccountID: "account-1", AccountConfig: tt.accountConfig}, + hs.ProcessedAuctionRequestPayload{}, + ) + + assert.NoError(t, err) + assert.Equal(t, hs.HookResult[hs.ProcessedAuctionRequestPayload]{}, result) + assert.Empty(t, result.Warnings) + assert.Empty(t, result.Errors) + }) + } +} + func TestConfigChanged(t *testing.T) { testCases := []struct { diff --git a/modules/prebid/rulesengine/result_functions.go b/modules/prebid/rulesengine/result_functions.go index eef632ba5bb..e6c68a13186 100644 --- a/modules/prebid/rulesengine/result_functions.go +++ b/modules/prebid/rulesengine/result_functions.go @@ -4,6 +4,8 @@ import ( "encoding/json" "errors" "fmt" + "sort" + "strings" "github.com/prebid/prebid-server/v4/modules/prebid/rulesengine/config" "github.com/prebid/prebid-server/v4/openrtb_ext" @@ -67,6 +69,17 @@ func (eb *ExcludeBidders) Call(req *openrtb_ext.RequestWrapper, result *Processe } result.HookResult.ChangeSet.ProcessedAuctionRequest().Bidders().Delete(excludedBidders) + + // Only warn about bidders that are actually present in the request, so the debug output + // reflects what was really removed rather than the rule's full configured exclusion list. + removedBidders := filterPresentBidders(req, eb.Args.Bidders) + if len(removedBidders) > 0 { + warning := buildExclusionWarning(removedBidders, meta) + result.HookResult.Warnings = append( + result.HookResult.Warnings, + warning, + ) + } return nil } @@ -74,6 +87,100 @@ func (eb *ExcludeBidders) Name() string { return ExcludeBiddersName } +// filterPresentBidders returns the subset of the given bidders that appear in at least one +// imp's ext.prebid.bidder map, preserving the input order. It lets exclusion warnings mention +// only the bidders that were really removed from the request instead of the configured list. +func filterPresentBidders(req *openrtb_ext.RequestWrapper, bidders []string) []string { + if req == nil { + return nil + } + + present := make(map[string]struct{}) + for _, impWrapper := range req.GetImp() { + impExt, err := impWrapper.GetImpExt() + if err != nil { + continue + } + impPrebid := impExt.GetPrebid() + if impPrebid == nil { + continue + } + for bidderName := range impPrebid.Bidder { + present[bidderName] = struct{}{} + } + } + + filtered := make([]string, 0, len(bidders)) + for _, bidderName := range bidders { + if _, ok := present[bidderName]; ok { + filtered = append(filtered, bidderName) + } + } + return filtered +} + +// buildExclusionWarning builds a single human-readable warning line describing why a set of +// bidders was removed from the request by the rules engine. The ruleset label (the configured +// ruleset name, falling back to the model group's analytics key when no name is set) and the +// evaluated conditions (SchemaFunctionResults) are only included when present. +func buildExclusionWarning(bidders []string, meta rules.ResultFunctionMeta) string { + var sb strings.Builder + + if len(bidders) == 1 { + sb.WriteString(fmt.Sprintf("Bidder [%s] was removed from the request by the rules engine", bidders[0])) + } else { + sb.WriteString(fmt.Sprintf("Bidders [%s] were removed from the request by the rules engine", strings.Join(bidders, ", "))) + } + + // Prefer the human-readable ruleset name; fall back to the analytics key when no name is set. + rulesetLabel := meta.RulesetName + if len(rulesetLabel) == 0 { + rulesetLabel = meta.AnalyticsKey + } + if len(rulesetLabel) > 0 { + sb.WriteString(fmt.Sprintf(" ruleset %q", rulesetLabel)) + } + + if reason := buildSchemaReason(meta.SchemaFunctionResults); len(reason) > 0 { + sb.WriteString(": ") + sb.WriteString(reason) + } + + return sb.String() +} + +// buildSchemaReason renders the evaluated schema conditions into a human-readable reason. It is +// generic across every schema function, so any rule added to the engine gets sensible reasoning +// without per-function special casing. It is shared by the exclude and include warning builders. +func buildSchemaReason(steps []rules.SchemaFunctionStep) string { + if len(steps) == 0 { + return "" + } + + reasons := make([]string, 0, len(steps)) + for _, step := range steps { + reasons = append(reasons, describeSchemaStep(step)) + } + return strings.Join(reasons, ", ") +} + +// describeSchemaStep turns a single evaluated schema condition into plain text of the form +// " rule evaluated to ". It is generic across every schema function, so any rule +// added to the engine reads sensibly without per-function special casing. Boolean results +// (true/false) are rendered unquoted; an empty result (e.g. a value function like deviceCountry +// when the request has no geo) is rendered as "(no value)"; any other result (e.g. a country code +// or channel) is quoted. +func describeSchemaStep(step rules.SchemaFunctionStep) string { + switch step.FuncResult { + case "true", "false": + return fmt.Sprintf("%s rule evaluated to %s", step.FuncName, step.FuncResult) + case "": + return fmt.Sprintf("%s rule evaluated to (no value)", step.FuncName) + default: + return fmt.Sprintf("%s rule evaluated to %q", step.FuncName, step.FuncResult) + } +} + // NewIncludeBidders is a factory function that creates a new IncludeBidders result function. // It takes a JSON raw message as input, unmarshals it into a slice of ResultFuncParams, // and returns an IncludeBidders instance. @@ -100,9 +207,114 @@ func (ib *IncludeBidders) Call(req *openrtb_ext.RequestWrapper, result *Processe for _, bidderName := range ib.Args.Bidders { result.AllowedBidders[bidderName] = struct{}{} // Ensure the bidder is included in the allowed bidders } + // Record the ruleset context so that, once every ruleset has run and the final allow-list is + // known, we can surface a debug warning naming the bidders that were implicitly removed because + // they were not on any include list. + result.IncludeContexts = append(result.IncludeContexts, meta) return nil } func (ib *IncludeBidders) Name() string { return IncludeBiddersName } + +// biddersRemovedByInclude returns the bidders present in the request that are not in the final +// allow-list accumulated by the include rules. These are the bidders that will be implicitly +// dropped when the allow-list is applied. The result is sorted for deterministic output. +func biddersRemovedByInclude(req *openrtb_ext.RequestWrapper, allowed map[string]struct{}) []string { + if req == nil { + return nil + } + + removed := make(map[string]struct{}) + for _, impWrapper := range req.GetImp() { + impExt, err := impWrapper.GetImpExt() + if err != nil { + continue + } + impPrebid := impExt.GetPrebid() + if impPrebid == nil { + continue + } + for bidderName := range impPrebid.Bidder { + if _, ok := allowed[bidderName]; !ok { + removed[bidderName] = struct{}{} + } + } + } + + result := make([]string, 0, len(removed)) + for bidderName := range removed { + result = append(result, bidderName) + } + sort.Strings(result) + return result +} + +// appendInclusionWarnings surfaces a debug warning naming the bidders that an include rule +// implicitly removed from the request (bidders present in the request but absent from every +// include list). It is a no-op when no include rule fired or when nothing was removed. +func appendInclusionWarnings(req *openrtb_ext.RequestWrapper, result *ProcessedAuctionHookResult) { + if result == nil || len(result.IncludeContexts) == 0 { + return + } + + removed := biddersRemovedByInclude(req, result.AllowedBidders) + if len(removed) == 0 { + return + } + + result.HookResult.Warnings = append( + result.HookResult.Warnings, + buildInclusionWarning(removed, result.IncludeContexts), + ) +} + +// buildInclusionWarning builds a single human-readable warning line describing why a set of bidders +// was removed from the request because they were not on any include list. Each include rule that +// constrained the request is described by its ruleset label and evaluated conditions when present. +func buildInclusionWarning(bidders []string, contexts []rules.ResultFunctionMeta) string { + var sb strings.Builder + + if len(bidders) == 1 { + sb.WriteString(fmt.Sprintf("Bidder [%s] was removed from the request by the rules engine because it was not in the include list", bidders[0])) + } else { + sb.WriteString(fmt.Sprintf("Bidders [%s] were removed from the request by the rules engine because they were not in the include list", strings.Join(bidders, ", "))) + } + + descriptions := make([]string, 0, len(contexts)) + for _, meta := range contexts { + if desc := describeIncludeContext(meta); len(desc) > 0 { + descriptions = append(descriptions, desc) + } + } + if len(descriptions) > 0 { + sb.WriteString(" applied by ") + sb.WriteString(strings.Join(descriptions, ", ")) + } + + return sb.String() +} + +// describeIncludeContext renders a single include rule's ruleset label (the configured ruleset name, +// falling back to the model group's analytics key) and evaluated conditions into text of the form +// `ruleset "X" (deviceCountry rule evaluated to "JPN")`. Either portion is omitted when not present. +func describeIncludeContext(meta rules.ResultFunctionMeta) string { + rulesetLabel := meta.RulesetName + if len(rulesetLabel) == 0 { + rulesetLabel = meta.AnalyticsKey + } + + reason := buildSchemaReason(meta.SchemaFunctionResults) + + switch { + case len(rulesetLabel) > 0 && len(reason) > 0: + return fmt.Sprintf("ruleset %q (%s)", rulesetLabel, reason) + case len(rulesetLabel) > 0: + return fmt.Sprintf("ruleset %q", rulesetLabel) + case len(reason) > 0: + return reason + default: + return "" + } +} diff --git a/modules/prebid/rulesengine/result_functions_test.go b/modules/prebid/rulesengine/result_functions_test.go index d452f1d83da..ef8ea65a6b1 100644 --- a/modules/prebid/rulesengine/result_functions_test.go +++ b/modules/prebid/rulesengine/result_functions_test.go @@ -140,6 +140,283 @@ func TestExcludeBiddersName(t *testing.T) { assert.Equal(t, ExcludeBiddersName, actualName, "ExcludeBidders name should match expected value") } +// TestExcludeBiddersCallEmitsWarnings verifies that ExcludeBidders.Call appends a debug warning +// describing why bidders were removed, and that only bidders actually present in the request are +// mentioned (not the rule's full configured exclusion list). +func TestExcludeBiddersCallEmitsWarnings(t *testing.T) { + tests := []struct { + name string + argBidders []string + meta rules.ResultFunctionMeta + req *openrtb_ext.RequestWrapper + expectedWarnings []string + }{ + { + name: "value_function_quoted_country", + argBidders: []string{"bidder1"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountry", FuncResult: "JPN"}, + }, + }, + req: mockRequestWrapperWithBidders(t, []string{"bidder1", "bidder2", "bidder3"}), + expectedWarnings: []string{ + `Bidder [bidder1] was removed from the request by the rules engine ruleset "bidderConfig": deviceCountry rule evaluated to "JPN"`, + }, + }, + { + name: "ruleset_name_preferred_over_analytics_key", + argBidders: []string{"rise"}, + meta: rules.ResultFunctionMeta{ + RulesetName: "microsoft-account-rise", + AnalyticsKey: "someKey", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountryIn", FuncResult: "false"}, + }, + }, + req: mockRequestWrapperWithBidders(t, []string{"rise", "bidder2"}), + expectedWarnings: []string{ + `Bidder [rise] was removed from the request by the rules engine ruleset "microsoft-account-rise": deviceCountryIn rule evaluated to false`, + }, + }, + { + name: "warn_only_for_bidders_present_in_request", + argBidders: []string{"bidder1", "bidder2", "bidder3"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountryIn", FuncResult: "true"}, + }, + }, + req: mockRequestWrapperWithBidders(t, []string{"bidder2"}), + expectedWarnings: []string{ + `Bidder [bidder2] was removed from the request by the rules engine ruleset "bidderConfig": deviceCountryIn rule evaluated to true`, + }, + }, + { + name: "no_warning_when_no_configured_bidder_present", + argBidders: []string{"bidder1", "bidder2"}, + meta: rules.ResultFunctionMeta{}, + req: mockRequestWrapperWithBidders(t, []string{"bidder3"}), + expectedWarnings: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + eb := &ExcludeBidders{Args: config.ResultFuncParams{Bidders: tt.argBidders}} + result := &ProcessedAuctionHookResult{ + HookResult: hs.HookResult[hs.ProcessedAuctionRequestPayload]{ + ChangeSet: hs.ChangeSet[hs.ProcessedAuctionRequestPayload]{}, + }, + AllowedBidders: make(map[string]struct{}), + } + + err := eb.Call(tt.req, result, tt.meta) + + assert.NoError(t, err) + assert.Equal(t, tt.expectedWarnings, result.HookResult.Warnings) + }) + } +} + +// TestExcludeBiddersCallMultipleExclusions proves that two separate ExcludeBidders.Call +// invocations for different reasons produce two distinct warnings - reasons are never merged. +func TestExcludeBiddersCallMultipleExclusions(t *testing.T) { + req := mockRequestWrapperWithBidders(t, []string{"bidderA", "bidderB", "bidderC"}) + result := &ProcessedAuctionHookResult{ + HookResult: hs.HookResult[hs.ProcessedAuctionRequestPayload]{ + ChangeSet: hs.ChangeSet[hs.ProcessedAuctionRequestPayload]{}, + }, + AllowedBidders: make(map[string]struct{}), + } + + ebA := &ExcludeBidders{Args: config.ResultFuncParams{Bidders: []string{"bidderA"}}} + metaA := rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountry", FuncResult: "JPN"}, + }, + } + assert.NoError(t, ebA.Call(req, result, metaA)) + + ebB := &ExcludeBidders{Args: config.ResultFuncParams{Bidders: []string{"bidderB"}}} + metaB := rules.ResultFunctionMeta{ + RulesetName: "customRuleset", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "channel", FuncResult: "web"}, + }, + } + assert.NoError(t, ebB.Call(req, result, metaB)) + + assert.Len(t, result.HookResult.ChangeSet.Mutations(), 2) + assert.Equal(t, []string{ + `Bidder [bidderA] was removed from the request by the rules engine ruleset "bidderConfig": deviceCountry rule evaluated to "JPN"`, + `Bidder [bidderB] was removed from the request by the rules engine ruleset "customRuleset": channel rule evaluated to "web"`, + }, result.HookResult.Warnings) +} + +func TestExcludeBiddersCallMultipleBiddersEmitsOneWarning(t *testing.T) { + req := mockRequestWrapperWithBidders(t, []string{"openx", "rubicon", "pubmatic", "appnexus"}) + result := &ProcessedAuctionHookResult{ + HookResult: hs.HookResult[hs.ProcessedAuctionRequestPayload]{ + ChangeSet: hs.ChangeSet[hs.ProcessedAuctionRequestPayload]{}, + }, + AllowedBidders: make(map[string]struct{}), + } + exclude := &ExcludeBidders{Args: config.ResultFuncParams{Bidders: []string{"openx", "rubicon", "pubmatic"}}} + meta := rules.ResultFunctionMeta{ + RulesetName: "geo-ruleset", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountryIn", FuncResult: "true"}, + }, + } + + err := exclude.Call(req, result, meta) + + assert.NoError(t, err) + assert.Equal(t, []string{ + `Bidders [openx, rubicon, pubmatic] were removed from the request by the rules engine ruleset "geo-ruleset": deviceCountryIn rule evaluated to true`, + }, result.HookResult.Warnings) +} + +// TestBuildExclusionWarning asserts the warning-string builder directly across value/boolean/empty +// conditions, ruleset-name-vs-analytics-key preference, and single/plural bidder phrasing. +func TestBuildExclusionWarning(t *testing.T) { + tests := []struct { + name string + bidders []string + meta rules.ResultFunctionMeta + expected string + }{ + { + name: "plural_full_context", + bidders: []string{"bidder1", "bidder3"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountry", FuncResult: "JPN"}, + }, + }, + expected: `Bidders [bidder1, bidder3] were removed from the request by the rules engine ruleset "bidderConfig": deviceCountry rule evaluated to "JPN"`, + }, + { + name: "ruleset_name_preferred_over_analytics_key", + bidders: []string{"openx"}, + meta: rules.ResultFunctionMeta{ + RulesetName: "cross-account-openx", + AnalyticsKey: "someAnalyticsKey", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountryIn", FuncResult: "true"}, + }, + }, + expected: `Bidder [openx] was removed from the request by the rules engine ruleset "cross-account-openx": deviceCountryIn rule evaluated to true`, + }, + { + name: "analytics_key_used_when_no_ruleset_name", + bidders: []string{"openx"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountry", FuncResult: "USA"}, + }, + }, + expected: `Bidder [openx] was removed from the request by the rules engine ruleset "bidderConfig": deviceCountry rule evaluated to "USA"`, + }, + { + name: "no_schema_context", + bidders: []string{"bidder1"}, + meta: rules.ResultFunctionMeta{}, + expected: `Bidder [bidder1] was removed from the request by the rules engine`, + }, + { + name: "ruleset_only", + bidders: []string{"bidder1"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + }, + expected: `Bidder [bidder1] was removed from the request by the rules engine ruleset "bidderConfig"`, + }, + { + name: "boolean_condition_false", + bidders: []string{"rise"}, + meta: rules.ResultFunctionMeta{ + RulesetName: "microsoft-account-rise", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountryIn", FuncResult: "false"}, + }, + }, + expected: `Bidder [rise] was removed from the request by the rules engine ruleset "microsoft-account-rise": deviceCountryIn rule evaluated to false`, + }, + { + name: "empty_value_result_renders_no_value", + bidders: []string{"openx"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "bidderConfig", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountry", FuncResult: ""}, + }, + }, + expected: `Bidder [openx] was removed from the request by the rules engine ruleset "bidderConfig": deviceCountry rule evaluated to (no value)`, + }, + { + name: "mixed_value_boolean_and_empty_conditions", + bidders: []string{"openx"}, + meta: rules.ResultFunctionMeta{ + AnalyticsKey: "mixed-empty-ruleset", + SchemaFunctionResults: []rules.SchemaFunctionStep{ + {FuncName: "deviceCountry", FuncResult: ""}, + {FuncName: "deviceCountryIn", FuncResult: "false"}, + {FuncName: "channel", FuncResult: "web"}, + }, + }, + expected: `Bidder [openx] was removed from the request by the rules engine ruleset "mixed-empty-ruleset": deviceCountry rule evaluated to (no value), deviceCountryIn rule evaluated to false, channel rule evaluated to "web"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, buildExclusionWarning(tt.bidders, tt.meta)) + }) + } +} + +// TestDescribeSchemaStep exercises single-step rendering across every result kind: value functions +// (quoted), boolean membership/availability functions (unquoted true/false), and the empty-value +// edge case (rendered as "(no value)"). +func TestDescribeSchemaStep(t *testing.T) { + tests := []struct { + name string + step rules.SchemaFunctionStep + expected string + }{ + {name: "value_deviceCountry", step: rules.SchemaFunctionStep{FuncName: "deviceCountry", FuncResult: "JPN"}, expected: `deviceCountry rule evaluated to "JPN"`}, + {name: "value_dataCenter", step: rules.SchemaFunctionStep{FuncName: "dataCenter", FuncResult: "us-west"}, expected: `dataCenter rule evaluated to "us-west"`}, + {name: "value_channel", step: rules.SchemaFunctionStep{FuncName: "channel", FuncResult: "web"}, expected: `channel rule evaluated to "web"`}, + {name: "empty_value_deviceCountry", step: rules.SchemaFunctionStep{FuncName: "deviceCountry", FuncResult: ""}, expected: `deviceCountry rule evaluated to (no value)`}, + {name: "empty_value_channel", step: rules.SchemaFunctionStep{FuncName: "channel", FuncResult: ""}, expected: `channel rule evaluated to (no value)`}, + {name: "boolean_deviceCountryIn_true", step: rules.SchemaFunctionStep{FuncName: "deviceCountryIn", FuncResult: "true"}, expected: `deviceCountryIn rule evaluated to true`}, + {name: "boolean_deviceCountryIn_false", step: rules.SchemaFunctionStep{FuncName: "deviceCountryIn", FuncResult: "false"}, expected: `deviceCountryIn rule evaluated to false`}, + {name: "boolean_dataCenterIn", step: rules.SchemaFunctionStep{FuncName: "dataCenterIn", FuncResult: "true"}, expected: `dataCenterIn rule evaluated to true`}, + {name: "boolean_eidAvailable", step: rules.SchemaFunctionStep{FuncName: "eidAvailable", FuncResult: "false"}, expected: `eidAvailable rule evaluated to false`}, + {name: "boolean_eidIn", step: rules.SchemaFunctionStep{FuncName: "eidIn", FuncResult: "true"}, expected: `eidIn rule evaluated to true`}, + {name: "boolean_userFpdAvailable", step: rules.SchemaFunctionStep{FuncName: "userFpdAvailable", FuncResult: "false"}, expected: `userFpdAvailable rule evaluated to false`}, + {name: "boolean_fpdAvailable", step: rules.SchemaFunctionStep{FuncName: "fpdAvailable", FuncResult: "true"}, expected: `fpdAvailable rule evaluated to true`}, + {name: "boolean_gppSidAvailable", step: rules.SchemaFunctionStep{FuncName: "gppSidAvailable", FuncResult: "true"}, expected: `gppSidAvailable rule evaluated to true`}, + {name: "boolean_gppSidIn", step: rules.SchemaFunctionStep{FuncName: "gppSidIn", FuncResult: "false"}, expected: `gppSidIn rule evaluated to false`}, + {name: "boolean_percent", step: rules.SchemaFunctionStep{FuncName: "percent", FuncResult: "true"}, expected: `percent rule evaluated to true`}, + {name: "boolean_tcfInScope", step: rules.SchemaFunctionStep{FuncName: "tcfInScope", FuncResult: "false"}, expected: `tcfInScope rule evaluated to false`}, + {name: "unexpected_value_quoted", step: rules.SchemaFunctionStep{FuncName: "deviceCountry", FuncResult: "US-CA"}, expected: `deviceCountry rule evaluated to "US-CA"`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, describeSchemaStep(tt.step)) + }) + } +} + func TestIncludeBiddersCall(t *testing.T) { tests := []struct { name string @@ -186,6 +463,216 @@ func TestIncludeBiddersCall(t *testing.T) { assert.Emptyf(t, result.HookResult.ChangeSet, "change set is empty") assert.Len(t, result.HookResult.ChangeSet.Mutations(), 0) assert.Len(t, result.AllowedBidders, len(tt.argBidders)) + // Every include invocation records its ruleset context so the removal warning can be + // built after all rulesets have run. + assert.Len(t, result.IncludeContexts, 1) + }) + } +} + +// TestIncludeBiddersCallRecordsContext verifies that each IncludeBidders.Call appends its +// ResultFunctionMeta to IncludeContexts (accumulating across multiple invocations) so the final +// removal warning can attribute the removal to the correct ruleset(s). +func TestIncludeBiddersCallRecordsContext(t *testing.T) { + req := mockRequestWrapperWithBidders(t, []string{"bidder1", "bidder2"}) + result := &ProcessedAuctionHookResult{ + HookResult: hs.HookResult[hs.ProcessedAuctionRequestPayload]{ChangeSet: hs.ChangeSet[hs.ProcessedAuctionRequestPayload]{}}, + AllowedBidders: make(map[string]struct{}), + } + + metaA := rules.ResultFunctionMeta{RulesetName: "rulesetA"} + metaB := rules.ResultFunctionMeta{AnalyticsKey: "bidderConfig"} + + ibA := &IncludeBidders{Args: config.ResultFuncParams{Bidders: []string{"bidder1"}}} + assert.NoError(t, ibA.Call(req, result, metaA)) + + ibB := &IncludeBidders{Args: config.ResultFuncParams{Bidders: []string{"bidder2"}}} + assert.NoError(t, ibB.Call(req, result, metaB)) + + assert.Equal(t, []rules.ResultFunctionMeta{metaA, metaB}, result.IncludeContexts) + assert.Equal(t, map[string]struct{}{"bidder1": {}, "bidder2": {}}, result.AllowedBidders) +} + +// TestBiddersRemovedByInclude asserts that only bidders present in the request but absent from the +// accumulated allow-list are reported, and that the output is sorted for deterministic warnings. +func TestBiddersRemovedByInclude(t *testing.T) { + tests := []struct { + name string + req *openrtb_ext.RequestWrapper + allowed map[string]struct{} + expected []string + }{ + { + name: "removes_present_bidders_not_allowed_sorted", + req: mockRequestWrapperWithBidders(t, []string{"openx", "rise", "appnexus"}), + allowed: map[string]struct{}{"appnexus": {}}, + expected: []string{"openx", "rise"}, + }, + { + name: "nothing_removed_when_all_allowed", + req: mockRequestWrapperWithBidders(t, []string{"appnexus", "rise"}), + allowed: map[string]struct{}{"appnexus": {}, "rise": {}}, + expected: []string{}, + }, + { + name: "allowed_bidder_absent_from_request_is_ignored", + req: mockRequestWrapperWithBidders(t, []string{"appnexus"}), + allowed: map[string]struct{}{"rise": {}}, + expected: []string{"appnexus"}, + }, + { + name: "nil_request", + req: nil, + allowed: map[string]struct{}{"appnexus": {}}, + expected: nil, + }, + { + name: "empty_allow_list_removes_all_present", + req: mockRequestWrapperWithBidders(t, []string{"rise", "openx"}), + allowed: map[string]struct{}{}, + expected: []string{"openx", "rise"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, biddersRemovedByInclude(tt.req, tt.allowed)) + }) + } +} + +// TestAppendInclusionWarnings verifies the end-to-end warning emission: a warning is appended only +// when at least one include rule fired and something was actually removed. +func TestAppendInclusionWarnings(t *testing.T) { + tests := []struct { + name string + req *openrtb_ext.RequestWrapper + allowed map[string]struct{} + includeContexts []rules.ResultFunctionMeta + expectedWarnings []string + }{ + { + name: "warns_for_bidders_removed_by_single_include_rule", + req: mockRequestWrapperWithBidders(t, []string{"appnexus", "rise", "openx"}), + allowed: map[string]struct{}{"appnexus": {}}, + includeContexts: []rules.ResultFunctionMeta{ + { + RulesetName: "microsoft-account-rise", + SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "deviceCountry", FuncResult: "JPN"}}, + }, + }, + expectedWarnings: []string{ + `Bidders [openx, rise] were removed from the request by the rules engine because they were not in the include list applied by ruleset "microsoft-account-rise" (deviceCountry rule evaluated to "JPN")`, + }, + }, + { + name: "single_bidder_phrasing", + req: mockRequestWrapperWithBidders(t, []string{"appnexus", "openx"}), + allowed: map[string]struct{}{"appnexus": {}}, + includeContexts: []rules.ResultFunctionMeta{ + {AnalyticsKey: "bidderConfig"}, + }, + expectedWarnings: []string{ + `Bidder [openx] was removed from the request by the rules engine because it was not in the include list applied by ruleset "bidderConfig"`, + }, + }, + { + name: "multiple_include_contexts_joined", + req: mockRequestWrapperWithBidders(t, []string{"appnexus", "rise", "openx"}), + allowed: map[string]struct{}{"appnexus": {}, "rise": {}}, + includeContexts: []rules.ResultFunctionMeta{ + {RulesetName: "rulesetA", SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "deviceCountry", FuncResult: "JPN"}}}, + {RulesetName: "rulesetB", SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "channel", FuncResult: "web"}}}, + }, + expectedWarnings: []string{ + `Bidder [openx] was removed from the request by the rules engine because it was not in the include list applied by ruleset "rulesetA" (deviceCountry rule evaluated to "JPN"), ruleset "rulesetB" (channel rule evaluated to "web")`, + }, + }, + { + name: "no_warning_when_no_include_context", + req: mockRequestWrapperWithBidders(t, []string{"appnexus", "openx"}), + allowed: map[string]struct{}{"appnexus": {}}, + includeContexts: nil, + expectedWarnings: nil, + }, + { + name: "no_warning_when_nothing_removed", + req: mockRequestWrapperWithBidders(t, []string{"appnexus"}), + allowed: map[string]struct{}{"appnexus": {}}, + includeContexts: []rules.ResultFunctionMeta{ + {RulesetName: "rulesetA"}, + }, + expectedWarnings: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := &ProcessedAuctionHookResult{ + HookResult: hs.HookResult[hs.ProcessedAuctionRequestPayload]{ChangeSet: hs.ChangeSet[hs.ProcessedAuctionRequestPayload]{}}, + AllowedBidders: tt.allowed, + IncludeContexts: tt.includeContexts, + } + + appendInclusionWarnings(tt.req, result) + + assert.Equal(t, tt.expectedWarnings, result.HookResult.Warnings) + }) + } +} + +// TestBuildInclusionWarning asserts the warning-string builder directly across single/plural bidder +// phrasing, ruleset-name-vs-analytics-key preference, missing reasons, and multiple contexts. +func TestBuildInclusionWarning(t *testing.T) { + tests := []struct { + name string + bidders []string + contexts []rules.ResultFunctionMeta + expected string + }{ + { + name: "single_bidder_full_context", + bidders: []string{"openx"}, + contexts: []rules.ResultFunctionMeta{ + {RulesetName: "rulesetA", SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "deviceCountry", FuncResult: "JPN"}}}, + }, + expected: `Bidder [openx] was removed from the request by the rules engine because it was not in the include list applied by ruleset "rulesetA" (deviceCountry rule evaluated to "JPN")`, + }, + { + name: "plural_bidders_analytics_key_fallback", + bidders: []string{"openx", "rise"}, + contexts: []rules.ResultFunctionMeta{ + {AnalyticsKey: "bidderConfig", SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "deviceCountryIn", FuncResult: "true"}}}, + }, + expected: `Bidders [openx, rise] were removed from the request by the rules engine because they were not in the include list applied by ruleset "bidderConfig" (deviceCountryIn rule evaluated to true)`, + }, + { + name: "ruleset_name_preferred_over_analytics_key", + bidders: []string{"openx"}, + contexts: []rules.ResultFunctionMeta{ + {RulesetName: "cross-account-openx", AnalyticsKey: "someKey"}, + }, + expected: `Bidder [openx] was removed from the request by the rules engine because it was not in the include list applied by ruleset "cross-account-openx"`, + }, + { + name: "no_context_details", + bidders: []string{"openx"}, + contexts: []rules.ResultFunctionMeta{{}}, + expected: `Bidder [openx] was removed from the request by the rules engine because it was not in the include list`, + }, + { + name: "multiple_contexts_joined", + bidders: []string{"openx"}, + contexts: []rules.ResultFunctionMeta{ + {RulesetName: "rulesetA", SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "deviceCountry", FuncResult: "JPN"}}}, + {RulesetName: "rulesetB", SchemaFunctionResults: []rules.SchemaFunctionStep{{FuncName: "channel", FuncResult: "web"}}}, + }, + expected: `Bidder [openx] was removed from the request by the rules engine because it was not in the include list applied by ruleset "rulesetA" (deviceCountry rule evaluated to "JPN"), ruleset "rulesetB" (channel rule evaluated to "web")`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, buildInclusionWarning(tt.bidders, tt.contexts)) }) } } diff --git a/rules/result_functions.go b/rules/result_functions.go index 97d12ce0790..51d87102b07 100644 --- a/rules/result_functions.go +++ b/rules/result_functions.go @@ -7,6 +7,7 @@ type ResultFunction[T1 any, T2 any] interface { type ResultFunctionMeta struct { SchemaFunctionResults []SchemaFunctionStep + RulesetName string AnalyticsKey string RuleFired string ModelVersion string diff --git a/rules/tree.go b/rules/tree.go index 60f53299149..706b1c192f8 100644 --- a/rules/tree.go +++ b/rules/tree.go @@ -38,6 +38,7 @@ func (n *Node[T1, T2]) matchChild(value string) (string, *Node[T1, T2]) { type Tree[T1 any, T2 any] struct { Root *Node[T1, T2] DefaultFunctions []ResultFunction[T1, T2] + RulesetName string AnalyticsKey string ModelVersion string } @@ -54,6 +55,7 @@ func (t *Tree[T1, T2]) Run(payload *T1, result *T2) error { currNode := t.Root resFuncMeta := ResultFunctionMeta{ + RulesetName: t.RulesetName, AnalyticsKey: t.AnalyticsKey, ModelVersion: t.ModelVersion, }