diff --git a/.github/workflows/adapter-code-coverage.yml b/.github/workflows/adapter-code-coverage.yml index 5b5dc9331e5..b74ecebb446 100644 --- a/.github/workflows/adapter-code-coverage.yml +++ b/.github/workflows/adapter-code-coverage.yml @@ -43,6 +43,8 @@ jobs: id: run_coverage if: ${{ steps.get_directories.outputs.result }} != "" run: | + git config --global url."https://${USERNAME}:${TOKEN}@git.pubmatic.com".insteadOf "https://git.pubmatic.com" + directories=$(echo '${{ steps.get_directories.outputs.result }}' | jq -r '.[]') go mod download @@ -65,13 +67,18 @@ jobs: # remove pull request branch files cd .. rm -f -r ./* + env: + GO111MODULE: "on" + GOPRIVATE: "git.pubmatic.com/PubMatic/*" + TOKEN: ${{ secrets.PM_OPENWRAP_CICD_PASSWORD }} + USERNAME: ${{ secrets.PM_OPENWRAP_CICD_USERNAME }} - name: Checkout coverage-preview branch uses: actions/checkout@v3 with: fetch-depth: 0 ref: coverage-preview - repository: prebid/prebid-server + repository: PubMatic-OpenWrap/prebid-server - name: Commit coverage files to coverage-preview branch if: ${{ steps.run_coverage.outputs.coverage_dir }} != "" diff --git a/.github/workflows/cross-repo-issue.yml b/.github/workflows/cross-repo-issue.yml index 2bea44a301c..fd479cd2ad9 100644 --- a/.github/workflows/cross-repo-issue.yml +++ b/.github/workflows/cross-repo-issue.yml @@ -4,7 +4,7 @@ on: pull_request_target: types: [closed] branches: - - "master" + - "master-disable" jobs: cross-repo: diff --git a/.github/workflows/validate-merge.yml b/.github/workflows/validate-merge.yml index 07f1bacaa45..0fe3e5523d3 100644 --- a/.github/workflows/validate-merge.yml +++ b/.github/workflows/validate-merge.yml @@ -2,7 +2,10 @@ name: Validate Merge on: pull_request: - branches: [master] + branches: + - master + - main + - ci jobs: validate-merge: @@ -19,6 +22,10 @@ jobs: - name: Validate run: | + git config --global url."https://${USERNAME}:${TOKEN}@git.pubmatic.com".insteadOf "https://git.pubmatic.com" ./validate.sh --nofmt --cov --race 10 env: GO111MODULE: "on" + GOPRIVATE: "git.pubmatic.com/PubMatic/*" + TOKEN: ${{ secrets.PM_OPENWRAP_CICD_PASSWORD }} + USERNAME: ${{ secrets.PM_OPENWRAP_CICD_USERNAME }} diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 9047e1f468f..b28ac8ee5f8 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -28,6 +28,10 @@ jobs: - name: Validate run: | + git config --global url."https://${USERNAME}:${TOKEN}@git.pubmatic.com".insteadOf "https://git.pubmatic.com" ./validate.sh --nofmt --cov --race 10 env: GO111MODULE: "on" + GOPRIVATE: "git.pubmatic.com/PubMatic/*" + TOKEN: ${{ secrets.PM_OPENWRAP_CICD_PASSWORD }} + USERNAME: ${{ secrets.PM_OPENWRAP_CICD_USERNAME }} diff --git a/adapters/pubmatic/pubmatic.go b/adapters/pubmatic/pubmatic.go index f6788c05f80..96656959bd0 100644 --- a/adapters/pubmatic/pubmatic.go +++ b/adapters/pubmatic/pubmatic.go @@ -21,6 +21,7 @@ import ( const MAX_IMPRESSIONS_PUBMATIC = 30 const ( + ae = "ae" PUBMATIC = "[PUBMATIC]" buyId = "buyid" buyIdTargetingKey = "hb_buyid_" @@ -60,6 +61,7 @@ type pubmaticBidExtVideo struct { type ExtImpBidderPubmatic struct { adapters.ExtImpBidder Data json.RawMessage `json:"data,omitempty"` + AE int `json:"ae,omitempty"` SKAdnetwork json.RawMessage `json:"skadn,omitempty"` } @@ -79,6 +81,10 @@ type extRequestAdServer struct { openrtb_ext.ExtRequest } +type respExt struct { + FledgeAuctionConfigs map[string]json.RawMessage `json:"fledge_auction_configs,omitempty"` +} + const ( dctrKeyName = "key_val" pmZoneIDKeyName = "pmZoneId" @@ -386,6 +392,10 @@ func parseImpressionObject(imp *openrtb2.Imp, extractWrapperExtFromImp, extractP extMap[bidViewability] = pubmaticExt.BidViewabilityScore } + if bidderExt.AE != 0 { + extMap[ae] = bidderExt.AE + } + imp.Ext = nil if len(extMap) > 0 { ext, err := json.Marshal(extMap) @@ -582,6 +592,20 @@ func (a *PubmaticAdapter) MakeBids(internalRequest *openrtb2.BidRequest, externa if bidResp.Cur != "" { bidResponse.Currency = bidResp.Cur } + + if bidResp.Ext != nil { + var bidRespExt respExt + if err := json.Unmarshal(bidResp.Ext, &bidRespExt); err == nil && bidRespExt.FledgeAuctionConfigs != nil { + bidResponse.FledgeAuctionConfigs = make([]*openrtb_ext.FledgeAuctionConfig, 0, len(bidRespExt.FledgeAuctionConfigs)) + for impId, config := range bidRespExt.FledgeAuctionConfigs { + fledgeAuctionConfig := &openrtb_ext.FledgeAuctionConfig{ + ImpId: impId, + Config: config, + } + bidResponse.FledgeAuctionConfigs = append(bidResponse.FledgeAuctionConfigs, fledgeAuctionConfig) + } + } + } return bidResponse, errs } diff --git a/adapters/pubmatic/pubmatictest/exemplary/fledge.json b/adapters/pubmatic/pubmatictest/exemplary/fledge.json new file mode 100644 index 00000000000..793f0984624 --- /dev/null +++ b/adapters/pubmatic/pubmatictest/exemplary/fledge.json @@ -0,0 +1,166 @@ +{ + "mockBidRequest": { + "id": "test-request-id", + "imp": [ + { + "id": "test-imp-id", + "banner": { + "format": [ + { + "w": 728, + "h": 90 + } + ] + }, + "ext": { + "ae": 1, + "bidder": { + "publisherId": "999", + "adSlot": "AdTag_Div1@300x250" + } + } + } + ] + }, + "httpCalls": [ + { + "expectedRequest": { + "uri": "https://hbopenbid.pubmatic.com/translator?source=prebid-server", + "body": { + "id": "test-request-id", + "imp": [ + { + "id": "test-imp-id", + "banner": { + "format": [ + { + "w": 728, + "h": 90 + } + ], + "h": 250, + "w": 300 + }, + "tagid": "AdTag_Div1", + "ext": { + "ae": 1 + } + } + ], + "ext": {"prebid":{}} + } + }, + "mockResponse": { + "status": 200, + "body": { + "id": "test-request-id", + "seatbid": [ + { + "seat": "pubmatic", + "bid": [ + { + "id": "9d8e77c2-ee0f-4781-af59-5359493b11af", + "impid": "test-imp-id", + "price": 1.1, + "adm": "some-test-ad", + "crid": "crid_10", + "h": 90, + "w": 728, + "ext": {} + } + ] + } + ], + "cur": "USD", + "ext": { + "fledge_auction_configs": { + "test-imp-id": { + "seller": "PUBMATIC_SELLER_CONSTANT_STRING", + "sellerTimeout": 123, + "decisionLogicUrl": "PUBMATIC_URL_CONSTANT_STRING", + "interestGroupBuyers": [ + "somedomain1.com", + "somedomain2.com", + "somedomain3.com", + "somedomain4.com" + ], + "perBuyerSignals": { + "somedomain1.com": { + "multiplier": 1, + "win_reporting_id": "1234" + }, + "somedomain2.com": { + "multiplier": 2, + "win_reporting_id": "2345" + }, + "somedomain3.com": { + "multiplier": 3, + "win_reporting_id": "3456" + }, + "somedomain4.com": { + "multiplier": 4, + "win_reporting_id": "4567" + } + } + } + } + } + } + } + } + ], + "expectedBidResponses": [ + { + "currency": "USD", + "bids": [ + { + "bid": { + "id": "9d8e77c2-ee0f-4781-af59-5359493b11af", + "impid": "test-imp-id", + "price": 1.1, + "adm": "some-test-ad", + "crid": "crid_10", + "w": 728, + "h": 90, + "ext": {} + }, + "type": "banner" + } + ], + "fledgeauctionconfigs": [ + { + "impid": "test-imp-id", + "config": { + "seller": "PUBMATIC_SELLER_CONSTANT_STRING", + "sellerTimeout": 123, + "decisionLogicUrl": "PUBMATIC_URL_CONSTANT_STRING", + "interestGroupBuyers": [ + "somedomain1.com", + "somedomain2.com", + "somedomain3.com", + "somedomain4.com" + ], + "perBuyerSignals": { + "somedomain1.com": { + "multiplier": 1, + "win_reporting_id": "1234" + }, + "somedomain2.com": { + "multiplier": 2, + "win_reporting_id": "2345" + }, + "somedomain3.com": { + "multiplier": 3, + "win_reporting_id": "3456" + }, + "somedomain4.com": { + "multiplier": 4, + "win_reporting_id": "4567" + } + } + } + } + ] + } + ] +} \ No newline at end of file diff --git a/adapters/vastbidder/bidder_macro.go b/adapters/vastbidder/bidder_macro.go index 2fbba3c28a4..7c95e45bbb4 100644 --- a/adapters/vastbidder/bidder_macro.go +++ b/adapters/vastbidder/bidder_macro.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/http" - "net/url" "strconv" "strings" "time" @@ -177,16 +176,35 @@ func (tag *BidderMacro) GetHeaders() http.Header { return http.Header{} } -// GetValueFromKV returns the value from KV map wrt key -func (tag *BidderMacro) GetValueFromKV(key string) string { - if tag.KV == nil { - return "" - } - key = strings.TrimPrefix(key, kvPrefix) - if value, found := tag.KV[key]; found { - return fmt.Sprintf("%v", value) +// GetValue returns the value for given key +// isKeyFound will check the key is present or not +func (tag *BidderMacro) GetValue(key string) (string, bool) { + macroKeys := strings.Split(key, ".") + isKeyFound := false + + // This will check if key has prefix kv/kvm + // if prefix present it will always return isKeyFound as true as it will help to replace the key with empty string in VAST TAG + if (macroKeys[0] == MacroKV || macroKeys[0] == MacroKVM) && len(macroKeys) > 1 { + isKeyFound = true + if tag.KV == nil { + return "", isKeyFound + } + switch macroKeys[0] { + case MacroKV: + val := getValueFromMap(macroKeys[1:], tag.KV) + if dataMap, ok := val.(map[string]interface{}); ok { + return mapToQuery(dataMap), isKeyFound + } + return fmt.Sprintf("%v", val), isKeyFound + case MacroKVM: + val := getValueFromMap(macroKeys[1:], tag.KV) + if isMap(val) { + return getJSONString(val), isKeyFound + } + return fmt.Sprintf("%v", val), isKeyFound + } } - return "" + return "", isKeyFound } /********************* Request *********************/ @@ -1209,13 +1227,7 @@ func (tag *BidderMacro) MacroKV(key string) string { if tag.KV == nil { return "" } - - values := url.Values{} - for key, val := range tag.KV { - values.Add(key, fmt.Sprintf("%v", val)) - } - return values.Encode() - + return mapToQuery(tag.KV) } // MacroKVM replace the kvm macro @@ -1223,11 +1235,7 @@ func (tag *BidderMacro) MacroKVM(key string) string { if tag.KV == nil { return "" } - jsonBytes, err := json.Marshal(tag.KV) - if err != nil { - return "" - } - return string(jsonBytes) + return getJSONString(tag.KV) } /********************* Request Headers *********************/ diff --git a/adapters/vastbidder/bidder_macro_test.go b/adapters/vastbidder/bidder_macro_test.go index 92daedff663..a5db950800f 100644 --- a/adapters/vastbidder/bidder_macro_test.go +++ b/adapters/vastbidder/bidder_macro_test.go @@ -1264,7 +1264,7 @@ func TestBidderMacro_MacroTest(t *testing.T) { } } -func TestBidderGetValueFromKV(t *testing.T) { +func TestBidderGetValue(t *testing.T) { type fields struct { KV map[string]interface{} } @@ -1272,41 +1272,157 @@ func TestBidderGetValueFromKV(t *testing.T) { key string } tests := []struct { - name string - fields fields - args args - want string - found bool + name string + fields fields + args args + want string + isKeyFound bool // if key has the prefix kv/kvm then it should return thr isKeyFound true }{ { - name: "Valid_Key", + name: "valid_Key", fields: fields{KV: map[string]interface{}{ "name": "test", "age": 22, }}, - args: args{key: "kv.name"}, - want: "test", + args: args{key: "kv.name"}, + want: "test", + isKeyFound: true, }, { - name: "Invalid_Key", + name: "invalid_Key", fields: fields{KV: map[string]interface{}{ "name": "test", "age": 22, }}, - args: args{key: "kv.anykey"}, - want: "", + args: args{key: "kv.anykey"}, + want: "", + isKeyFound: true, }, { - name: "Empty_KV_Map", - fields: fields{KV: nil}, - args: args{key: "kv.anykey"}, - want: "", + name: "empty_kv_map", + fields: fields{KV: nil}, + args: args{key: "kv.anykey"}, + want: "", + isKeyFound: true, }, { - name: "KV_map_with_no_key_val_pair", - fields: fields{KV: map[string]interface{}{}}, - args: args{key: "kv.anykey"}, - want: "", + name: "kv_map_with_no_key_val_pair", + fields: fields{KV: map[string]interface{}{}}, + args: args{key: "kv.anykey"}, + want: "", + isKeyFound: true, + }, + { + name: "key_with_value_as_url", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + "url": "http://example.com?k1=v1&k2=v2", + }, + }}, + args: args{key: "kvm.country.url"}, + want: "http://example.com?k1=v1&k2=v2", + isKeyFound: true, + }, + { + name: "kvm_prefix_key_with_value_as_nested_map", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + "url": "http//example.com?k1=v1&k2=v2", + "metadata": map[string]interface{}{ + "k1": "v1", + "k2": "v2", + }, + }, + }}, + args: args{key: "kvm.country"}, + want: "{\"metadata\":{\"k1\":\"v1\",\"k2\":\"v2\"},\"pincode\":411041,\"state\":\"MH\",\"url\":\"http//example.com?k1=v1&k2=v2\"}", + isKeyFound: true, + }, + { + name: "kv_prefix_key_with_value_as_nested_map", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + "url": "http://example.com?k1=v1&k2=v2", + "metadata": map[string]interface{}{ + "k1": "v1", + "k2": "v2", + }, + }, + }}, + args: args{key: "kv.country"}, + want: "metadata=k1%3Dv1%26k2%3Dv2&pincode=411041&state=MH&url=http%3A%2F%2Fexample.com%3Fk1%3Dv1%26k2%3Dv2", + isKeyFound: true, + }, + { + name: "key_without_kv_kvm_prefix", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + "url": "http//example.com?k1=v1&k2=v2", + "metadata": map[string]interface{}{ + "k1": "v1", + "k2": "v2", + }, + }, + }}, + args: args{key: "someprefix.kv"}, + want: "", + isKeyFound: false, // hence this key is not starting with kv/kvm prefix we return isKeyFound as false + }, + { + name: "multi-level_key", + fields: fields{KV: map[string]interface{}{ + "k1": map[string]interface{}{ + "k2": map[string]interface{}{ + "k3": map[string]interface{}{ + "k4": map[string]interface{}{ + "name": "test", + }, + }, + }, + }, + }}, + args: args{key: "kv.k1.k2.k3.k4.name"}, + want: "test", + isKeyFound: true, + }, + { + name: "key_not_matched", + fields: fields{KV: map[string]interface{}{ + "k1": map[string]interface{}{ + "k2": map[string]interface{}{ + "k3": map[string]interface{}{ + "k4": map[string]interface{}{ + "name": "test", + }, + }, + }, + }, + }}, + args: args{key: "kv.k1.k2.k3.name"}, + want: "", + isKeyFound: true, + }, + { + name: "key_wihtout_any_prefix", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "age": 22, + }}, + args: args{key: "kv"}, + want: "", + isKeyFound: false, }, } for _, tt := range tests { @@ -1314,8 +1430,9 @@ func TestBidderGetValueFromKV(t *testing.T) { tag := &BidderMacro{ KV: tt.fields.KV, } - value := tag.GetValueFromKV(tt.args.key) + value, isKeyFound := tag.GetValue(tt.args.key) assert.Equal(t, tt.want, value, tt.name) + assert.Equal(t, tt.isKeyFound, isKeyFound) }) } } @@ -1334,7 +1451,7 @@ func TestBidderMacroKV(t *testing.T) { want string }{ { - name: "Valid_test", + name: "valid_test", fields: fields{KV: map[string]interface{}{ "name": "test", "age": "22", @@ -1343,27 +1460,63 @@ func TestBidderMacroKV(t *testing.T) { want: "age=22&name=test", }, { - name: "Valid_test_with_url", + name: "valid_test_with_url", fields: fields{KV: map[string]interface{}{ - "name": "test", - "age": "22", - "url": "http://example.com?k1=v1&k2=v2", + "age": "22", + "url": "http://example.com?k1=v1&k2=v2", + }}, + args: args{key: "kv"}, + want: "age=22&url=http%3A%2F%2Fexample.com%3Fk1%3Dv1%26k2%3Dv2", + }, + { + name: "valid_test_with_encoded_url", + fields: fields{KV: map[string]interface{}{ + "age": "22", + "url": "http%3A%2F%2Fexample.com%3Fk1%3Dv1%26k2%3Dv2", }}, args: args{key: "kv"}, - want: "age=22&name=test&url=http%3A%2F%2Fexample.com%3Fk1%3Dv1%26k2%3Dv2", + want: "age=22&url=http%253A%252F%252Fexample.com%253Fk1%253Dv1%2526k2%253Dv2", }, { - name: "Empty_KV_map", + name: "empty_KV_map", fields: fields{KV: nil}, args: args{key: "kv"}, want: "", }, { - name: "KV_map_with_no_key_val_pair", + name: "kv_map_with_no_key_val_pair", fields: fields{KV: map[string]interface{}{}}, args: args{key: "kv"}, want: "", }, + { + name: "key_with_value_as_map", + fields: fields{KV: map[string]interface{}{ + "age": 22, + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + }, + }}, + args: args{key: "kv"}, + want: "age=22&country=pincode%3D411041%26state%3DMH", + }, + { + name: "key_with_value_as_nested_map", + fields: fields{KV: map[string]interface{}{ + "age": 22, + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + "metadata": map[string]interface{}{ + "k1": 223, + "k2": "v2", + }, + }, + }}, + args: args{key: "kv"}, + want: "age=22&country=metadata%3Dk1%253D223%2526k2%253Dv2%26pincode%3D411041%26state%3DMH", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1371,6 +1524,7 @@ func TestBidderMacroKV(t *testing.T) { KV: tt.fields.KV, } got := tag.MacroKV(tt.args.key) + assert.Equal(t, tt.want, got, tt.name) }) } @@ -1390,7 +1544,7 @@ func TestBidderMacroKVM(t *testing.T) { want string }{ { - name: "Valid_test", + name: "valid_test", fields: fields{KV: map[string]interface{}{ "name": "test", "age": "22", @@ -1399,13 +1553,13 @@ func TestBidderMacroKVM(t *testing.T) { want: "{\"age\":\"22\",\"name\":\"test\"}", }, { - name: "Empty_KV_map", + name: "empty_kv_map", fields: fields{KV: nil}, args: args{key: "kvm"}, want: "", }, { - name: "Value_as_int_data_type", + name: "value_as_int_data_type", fields: fields{KV: map[string]interface{}{ "name": "test", "age": 22, @@ -1414,13 +1568,13 @@ func TestBidderMacroKVM(t *testing.T) { want: "{\"age\":22,\"name\":\"test\"}", }, { - name: "KV_map_with_no_key_val_pair", + name: "kv_map_with_no_key_val_pair", fields: fields{KV: map[string]interface{}{}}, args: args{key: "kvm"}, want: "{}", }, { - name: "Marshal_error", + name: "marshal_error", fields: fields{KV: map[string]interface{}{ "name": "test", "age": make(chan int), @@ -1428,6 +1582,32 @@ func TestBidderMacroKVM(t *testing.T) { args: args{key: "kvm"}, want: "", }, + { + name: "test_with_url", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "url": "http://example.com?k1=v1&k2=v2", + }}, + args: args{key: "kvm"}, + want: "{\"name\":\"test\",\"url\":\"http://example.com?k1=v1&k2=v2\"}", + }, + { + name: "key_with_value_as_nested_map", + fields: fields{KV: map[string]interface{}{ + "name": "test", + "age": 22, + "country": map[string]interface{}{ + "state": "MH", + "pincode": 411041, + "metadata": map[string]interface{}{ + "k1": "v1", + "k2": "v2", + }, + }, + }}, + args: args{key: "kvm"}, + want: "{\"age\":22,\"country\":{\"metadata\":{\"k1\":\"v1\",\"k2\":\"v2\"},\"pincode\":411041,\"state\":\"MH\"},\"name\":\"test\"}", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/adapters/vastbidder/constant.go b/adapters/vastbidder/constant.go index 6a0d707fab5..1ba81122f78 100644 --- a/adapters/vastbidder/constant.go +++ b/adapters/vastbidder/constant.go @@ -169,9 +169,8 @@ const ( ) const ( - prebid = "prebid" - keyval = "keyval" - kvPrefix = "kv." + prebid = "prebid" + keyval = "keyval" ) var ParamKeys = []string{"param1", "param2", "param3", "param4", "param5"} diff --git a/adapters/vastbidder/ibidder_macro.go b/adapters/vastbidder/ibidder_macro.go index 6d7e555408c..a503c667e52 100644 --- a/adapters/vastbidder/ibidder_macro.go +++ b/adapters/vastbidder/ibidder_macro.go @@ -18,7 +18,7 @@ type IBidderMacro interface { SetAdapterConfig(*config.Adapter) GetURI() string GetHeaders() http.Header - GetValueFromKV(string) string + GetValue(string) (string, bool) //getAllHeaders returns default and custom heades getAllHeaders() http.Header diff --git a/adapters/vastbidder/macro_processor.go b/adapters/vastbidder/macro_processor.go index 354d922e1db..c6f2f3d871e 100644 --- a/adapters/vastbidder/macro_processor.go +++ b/adapters/vastbidder/macro_processor.go @@ -88,10 +88,8 @@ func (mp *MacroProcessor) processKey(key string) (string, bool) { tmpKey = tmpKey[0 : len(tmpKey)-macroEscapeSuffixLen] nEscaping++ continue - } else if strings.HasPrefix(tmpKey, kvPrefix) { - value = mp.bidderMacro.GetValueFromKV(tmpKey) - found = true - break + } else { + value, found = mp.bidderMacro.GetValue(tmpKey) } break } diff --git a/adapters/vastbidder/util.go b/adapters/vastbidder/util.go index 8ad02535ec6..c1ddb53fb4a 100644 --- a/adapters/vastbidder/util.go +++ b/adapters/vastbidder/util.go @@ -5,7 +5,10 @@ import ( "encoding/json" "fmt" "math/rand" + "net/url" + "reflect" "strconv" + "strings" "github.com/prebid/prebid-server/adapters" "github.com/prebid/prebid-server/openrtb_ext" @@ -68,3 +71,59 @@ func NormalizeJSON(obj map[string]interface{}) map[string]string { var GetRandomID = func() string { return strconv.FormatInt(rand.Int63(), intBase) } + +func getJSONString(kvmap any) string { + + var buf bytes.Buffer + encoder := json.NewEncoder(&buf) + + // Disable HTML escaping for special characters + encoder.SetEscapeHTML(false) + + if err := encoder.Encode(kvmap); err != nil { + return "" + } + return strings.TrimRight(buf.String(), "\n") + +} + +func isMap(data any) bool { + return reflect.TypeOf(data).Kind() == reflect.Map +} + +// extractDataFromMap help to get value from nested map +func getValueFromMap(lookUpOrder []string, m map[string]any) any { + if len(lookUpOrder) == 0 { + return "" + } + + for _, key := range lookUpOrder { + value, keyExists := m[key] + if !keyExists { + return "" + } + if nestedMap, isMap := value.(map[string]any); isMap { + m = nestedMap + } else { + return value + } + } + return m +} + +// mapToQuery convert the map data into & seperated string +func mapToQuery(m map[string]any) string { + values := url.Values{} + for key, value := range m { + switch reflect.TypeOf(value).Kind() { + case reflect.Map: + mvalue, ok := value.(map[string]any) + if ok { + values.Add(key, mapToQuery(mvalue)) + } + default: + values.Add(key, fmt.Sprintf("%v", value)) + } + } + return values.Encode() +} diff --git a/adapters/vastbidder/util_test.go b/adapters/vastbidder/util_test.go new file mode 100644 index 00000000000..e8c8681c19f --- /dev/null +++ b/adapters/vastbidder/util_test.go @@ -0,0 +1,195 @@ +package vastbidder + +import ( + "testing" + + "github.com/magiconair/properties/assert" +) + +func Test_getJSONString(t *testing.T) { + type args struct { + kvmap any + } + tests := []struct { + name string + args args + want string + }{ + { + name: "empty_map", + args: args{kvmap: map[string]any{}}, + want: "{}", + }, + { + name: "map_without_nesting", + args: args{kvmap: map[string]any{ + "k1": "v1", + "k2": "v2", + }}, + want: "{\"k1\":\"v1\",\"k2\":\"v2\"}", + }, + { + name: "map_with_nesting", + args: args{kvmap: map[string]any{ + "k1": "v1", + "metadata": map[string]any{ + "k2": "v2", + "k3": "v3", + }, + }}, + want: "{\"k1\":\"v1\",\"metadata\":{\"k2\":\"v2\",\"k3\":\"v3\"}}", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getJSONString(tt.args.kvmap) + assert.Equal(t, got, tt.want, tt.name) + }) + } +} + +func Test_getValueFromMap(t *testing.T) { + type args struct { + lookUpOrder []string + m map[string]any + } + tests := []struct { + name string + args args + want any + }{ + { + name: "map_without_nesting", + args: args{lookUpOrder: []string{"k1"}, + m: map[string]any{ + "k1": "v1", + "k2": "v2", + }, + }, + want: "v1", + }, + { + name: "map_with_nesting", + args: args{lookUpOrder: []string{"country", "state"}, + m: map[string]any{ + "name": "test", + "country": map[string]any{ + "state": "MH", + "pin": 12345, + }, + }, + }, + want: "MH", + }, + { + name: "key_not_exists", + args: args{lookUpOrder: []string{"country", "name"}, + m: map[string]any{ + "name": "test", + "country": map[string]any{ + "state": "MH", + "pin": 12345, + }, + }, + }, + want: "", + }, + { + name: "empty_m", + args: args{lookUpOrder: []string{"country", "name"}, + m: map[string]any{}, + }, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getValueFromMap(tt.args.lookUpOrder, tt.args.m) + assert.Equal(t, got, tt.want) + }) + } +} + +func Test_mapToQuery(t *testing.T) { + type args struct { + m map[string]any + } + tests := []struct { + name string + args args + want string + }{ + { + name: "map_without_nesting", + args: args{ + m: map[string]any{ + "k1": "v1", + "k2": "v2", + }, + }, + want: "k1=v1&k2=v2", + }, + { + name: "map_with_nesting", + args: args{ + m: map[string]any{ + "name": "test", + "country": map[string]any{ + "state": "MH", + "pin": 12345, + }, + }, + }, + want: "country=pin%3D12345%26state%3DMH&name=test", + }, + { + name: "empty_map", + args: args{ + m: map[string]any{}, + }, + want: "", + }, + { + name: "nil_map", + args: args{ + m: nil, + }, + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := mapToQuery(tt.args.m); got != tt.want { + t.Errorf("mapToQuery() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_isMap(t *testing.T) { + type args struct { + data any + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "map_data_type", + args: args{data: map[string]any{}}, + want: true, + }, + { + name: "string_data_type", + args: args{data: "data type is string"}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isMap(tt.args.data) + assert.Equal(t, got, tt.want) + }) + } +} diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 5055dd40029..aaa8c64a011 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -1524,12 +1524,12 @@ func (deps *endpointDeps) validateImpExt(imp *openrtb_ext.ImpWrapper, aliases ma errL := []error{} for bidder, ext := range prebid.Bidder { - coreBidder := bidder + coreBidder, _ := openrtb_ext.NormalizeBidderName(bidder) if tmp, isAlias := aliases[bidder]; isAlias { - coreBidder = tmp + coreBidder = openrtb_ext.BidderName(tmp) } - if coreBidderNormalized, isValid := deps.bidderMap[coreBidder]; isValid { + if coreBidderNormalized, isValid := deps.bidderMap[coreBidder.String()]; isValid { if err := deps.paramsValidator.Validate(coreBidderNormalized, ext); err != nil { msg := fmt.Sprintf("request.imp[%d].ext.prebid.bidder.%s failed validation.\n%v", impIndex, bidder, err) @@ -1593,18 +1593,21 @@ func (deps *endpointDeps) parseBidExt(req *openrtb_ext.RequestWrapper) error { } func (deps *endpointDeps) validateAliases(aliases map[string]string) error { - for alias, coreBidder := range aliases { - if _, isCoreBidderDisabled := deps.disabledBidders[coreBidder]; isCoreBidderDisabled { - return fmt.Errorf("request.ext.prebid.aliases.%s refers to disabled bidder: %s", alias, coreBidder) + for alias, bidderName := range aliases { + normalisedBidderName, _ := openrtb_ext.NormalizeBidderName(bidderName) + coreBidderName := normalisedBidderName.String() + if _, isCoreBidderDisabled := deps.disabledBidders[coreBidderName]; isCoreBidderDisabled { + return fmt.Errorf("request.ext.prebid.aliases.%s refers to disabled bidder: %s", alias, bidderName) } - if _, isCoreBidder := deps.bidderMap[coreBidder]; !isCoreBidder { - return fmt.Errorf("request.ext.prebid.aliases.%s refers to unknown bidder: %s", alias, coreBidder) + if _, isCoreBidder := deps.bidderMap[coreBidderName]; !isCoreBidder { + return fmt.Errorf("request.ext.prebid.aliases.%s refers to unknown bidder: %s", alias, bidderName) } - if alias == coreBidder { + if alias == coreBidderName { return fmt.Errorf("request.ext.prebid.aliases.%s defines a no-op alias. Choose a different alias, or remove this entry.", alias) } + aliases[alias] = coreBidderName } return nil } diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index 350b3057185..6206ac02ad4 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -2833,19 +2833,21 @@ func TestValidateImpExt(t *testing.T) { for _, group := range testGroups { for _, test := range group.testCases { - imp := &openrtb2.Imp{Ext: test.impExt} - impWrapper := &openrtb_ext.ImpWrapper{Imp: imp} + t.Run(test.description, func(t *testing.T) { + imp := &openrtb2.Imp{Ext: test.impExt} + impWrapper := &openrtb_ext.ImpWrapper{Imp: imp} - errs := deps.validateImpExt(impWrapper, nil, 0, false, nil) + errs := deps.validateImpExt(impWrapper, nil, 0, false, nil) - assert.NoError(t, impWrapper.RebuildImpressionExt(), test.description+":rebuild_imp") + assert.NoError(t, impWrapper.RebuildImpressionExt(), test.description+":rebuild_imp") - if len(test.expectedImpExt) > 0 { - assert.JSONEq(t, test.expectedImpExt, string(imp.Ext), "imp.ext JSON does not match expected. Test: %s. %s\n", group.description, test.description) - } else { - assert.Empty(t, imp.Ext, "imp.ext expected to be empty but was: %s. Test: %s. %s\n", string(imp.Ext), group.description, test.description) - } - assert.ElementsMatch(t, test.expectedErrs, errs, "errs slice does not match expected. Test: %s. %s\n", group.description, test.description) + if len(test.expectedImpExt) > 0 { + assert.JSONEq(t, test.expectedImpExt, string(imp.Ext), "imp.ext JSON does not match expected. Test: %s. %s\n", group.description, test.description) + } else { + assert.Empty(t, imp.Ext, "imp.ext expected to be empty but was: %s. Test: %s. %s\n", string(imp.Ext), group.description, test.description) + } + assert.Equal(t, test.expectedErrs, errs, "errs slice does not match expected. Test: %s. %s\n", group.description, test.description) + }) } } } @@ -5771,3 +5773,59 @@ func TestSetSeatNonBidRaw(t *testing.T) { }) } } + +func TestValidateAliases(t *testing.T) { + deps := &endpointDeps{ + disabledBidders: map[string]string{"rubicon": "rubicon"}, + bidderMap: map[string]openrtb_ext.BidderName{"appnexus": openrtb_ext.BidderName("appnexus")}, + } + + testCases := []struct { + description string + aliases map[string]string + expectedAliases map[string]string + expectedError error + }{ + { + description: "valid case", + aliases: map[string]string{"test": "appnexus"}, + expectedAliases: map[string]string{"test": "appnexus"}, + expectedError: nil, + }, + { + description: "valid case - case insensitive", + aliases: map[string]string{"test": "Appnexus"}, + expectedAliases: map[string]string{"test": "appnexus"}, + expectedError: nil, + }, + { + description: "disabled bidder", + aliases: map[string]string{"test": "rubicon"}, + expectedAliases: nil, + expectedError: errors.New("request.ext.prebid.aliases.test refers to disabled bidder: rubicon"), + }, + { + description: "coreBidderName not found", + aliases: map[string]string{"test": "anyBidder"}, + expectedAliases: nil, + expectedError: errors.New("request.ext.prebid.aliases.test refers to unknown bidder: anyBidder"), + }, + { + description: "alias name is coreBidder name", + aliases: map[string]string{"appnexus": "appnexus"}, + expectedAliases: nil, + expectedError: errors.New("request.ext.prebid.aliases.appnexus defines a no-op alias. Choose a different alias, or remove this entry."), + }, + } + + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + err := deps.validateAliases(testCase.aliases) + if err != nil { + assert.Equal(t, testCase.expectedError, err) + } else { + assert.ObjectsAreEqualValues(testCase.expectedAliases, map[string]string{"test": "appnexus"}) + } + }) + } +} diff --git a/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext-case-insensitive.json b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext-case-insensitive.json new file mode 100644 index 00000000000..20997076af2 --- /dev/null +++ b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext-case-insensitive.json @@ -0,0 +1,140 @@ +{ + "description": "This demonstrates all extension in case insensitive.", + "config": { + "mockBidders": [ + { + "bidderName": "appnexus", + "currency": "USD", + "price": 1.00 + }, + { + "bidderName": "rubicon", + "currency": "USD", + "price": 1.00 + } + ] + }, + "mockBidRequest": { + "id": "some-request-id", + "site": { + "page": "prebid.org" + }, + "user": { + "ext": { + "consent": "gdpr-consent-string", + "prebid": { + "buyeruids": { + "appnexus": "override-appnexus-id-in-cookie" + } + } + } + }, + "regs": { + "ext": { + "gdpr": 1, + "us_privacy": "1NYN" + } + }, + "imp": [ + { + "id": "some-impression-id", + "banner": { + "format": [ + { + "w": 300, + "h": 250 + }, + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "appnexus": { + "placementId": 12883451 + }, + "districtm": { + "placementId": 105 + }, + "rubicon": { + "accountId": 1001, + "siteId": 113932, + "zoneId": 535510 + } + } + } + ], + "tmax": 500, + "ext": { + "prebid": { + "aliases": { + "districtm": "Appnexus" + }, + "bidadjustmentfactors": { + "appnexus": 1.01, + "districtm": 0.98, + "rubicon": 0.99 + }, + "cache": { + "bids": {} + }, + "channel": { + "name": "video", + "version": "1.0" + }, + "targeting": { + "includewinners": false, + "pricegranularity": { + "precision": 2, + "ranges": [ + { + "max": 20, + "increment": 0.10 + } + ] + } + } + } + } + }, + "expectedBidResponse": { + "id": "some-request-id", + "seatbid": [ + { + "bid": [ + { + "id": "appnexus-bid", + "impid": "some-impression-id", + "price": 1.01 + } + ], + "seat": "appnexus" + }, + { + "bid": [ + { + "id": "appnexus-bid", + "impid": "some-impression-id", + "price": 0.98 + } + ], + "seat": "districtm" + }, + { + "bid": [ + { + "id": "rubicon-bid", + "impid": "some-impression-id", + "price": 0.99 + } + ], + "seat": "rubicon" + } + ], + "bidid": "test bid id", + "cur": "USD", + "nbr": 0 + }, + "expectedReturnCode": 200 +} \ No newline at end of file diff --git a/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext.json b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext.json index 8f64fd2a5fd..02dc6160d49 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/all-ext.json @@ -2,8 +2,16 @@ "description": "This demonstrates all of the OpenRTB extensions supported by Prebid Server. Very few requests will need all of these at once.", "config": { "mockBidders": [ - {"bidderName": "appnexus", "currency": "USD", "price": 1.00}, - {"bidderName": "rubicon", "currency": "USD", "price": 1.00} + { + "bidderName": "appnexus", + "currency": "USD", + "price": 1.00 + }, + { + "bidderName": "rubicon", + "currency": "USD", + "price": 1.00 + } ] }, "mockBidRequest": { @@ -91,42 +99,42 @@ } }, "expectedBidResponse": { - "id":"some-request-id", - "seatbid": [ - { - "bid": [ - { - "id": "appnexus-bid", - "impid": "some-impression-id", - "price": 1.01 - } - ], - "seat": "appnexus" - }, - { - "bid": [ - { - "id": "appnexus-bid", - "impid": "some-impression-id", - "price": 0.98 - } - ], - "seat": "districtm" - }, - { - "bid": [ - { - "id": "rubicon-bid", - "impid": "some-impression-id", - "price": 0.99 - } - ], - "seat": "rubicon" - } - ], - "bidid":"test bid id", - "cur":"USD", - "nbr":0 + "id": "some-request-id", + "seatbid": [ + { + "bid": [ + { + "id": "appnexus-bid", + "impid": "some-impression-id", + "price": 1.01 + } + ], + "seat": "appnexus" + }, + { + "bid": [ + { + "id": "appnexus-bid", + "impid": "some-impression-id", + "price": 0.98 + } + ], + "seat": "districtm" + }, + { + "bid": [ + { + "id": "rubicon-bid", + "impid": "some-impression-id", + "price": 0.99 + } + ], + "seat": "rubicon" + } + ], + "bidid": "test bid id", + "cur": "USD", + "nbr": 0 }, "expectedReturnCode": 200 -} +} \ No newline at end of file diff --git a/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple-case-insensitive.json b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple-case-insensitive.json new file mode 100644 index 00000000000..9b66a59d0a5 --- /dev/null +++ b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple-case-insensitive.json @@ -0,0 +1,61 @@ +{ + "description": "Simple request - bidder case insensitive", + "config": { + "mockBidders": [ + { + "bidderName": "appnexus", + "currency": "USD", + "price": 0.00 + } + ] + }, + "mockBidRequest": { + "id": "some-request-id", + "site": { + "page": "prebid.org" + }, + "imp": [ + { + "id": "some-impression-id", + "banner": { + "format": [ + { + "w": 300, + "h": 250 + }, + { + "w": 300, + "h": 600 + } + ] + }, + "ext": { + "Appnexus": { + "placementId": 12883451 + } + } + } + ], + "tmax": 500, + "ext": {} + }, + "expectedBidResponse": { + "id": "some-request-id", + "seatbid": [ + { + "bid": [ + { + "id": "appnexus-bid", + "impid": "some-impression-id", + "price": 0 + } + ], + "seat": "Appnexus" + } + ], + "bidid": "test bid id", + "cur": "USD", + "nbr": 0 + }, + "expectedReturnCode": 200 +} \ No newline at end of file diff --git a/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple.json b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple.json index ba9079d4675..5a7dbd20747 100644 --- a/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple.json +++ b/endpoints/openrtb2/sample-requests/valid-whole/exemplary/simple.json @@ -2,7 +2,11 @@ "description": "Simple request", "config": { "mockBidders": [ - {"bidderName": "appnexus", "currency": "USD", "price": 0.00} + { + "bidderName": "appnexus", + "currency": "USD", + "price": 0.00 + } ] }, "mockBidRequest": { @@ -36,22 +40,22 @@ "ext": {} }, "expectedBidResponse": { - "id":"some-request-id", - "seatbid": [ - { - "bid": [ - { - "id": "appnexus-bid", - "impid": "some-impression-id", - "price": 0 - } - ], - "seat": "appnexus" - } - ], - "bidid":"test bid id", - "cur":"USD", - "nbr":0 + "id": "some-request-id", + "seatbid": [ + { + "bid": [ + { + "id": "appnexus-bid", + "impid": "some-impression-id", + "price": 0 + } + ], + "seat": "appnexus" + } + ], + "bidid": "test bid id", + "cur": "USD", + "nbr": 0 }, "expectedReturnCode": 200 -} +} \ No newline at end of file diff --git a/exchange/bidder.go b/exchange/bidder.go index 263d27af878..2ac15c10fe5 100644 --- a/exchange/bidder.go +++ b/exchange/bidder.go @@ -261,6 +261,7 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, bidderRequest Bidde errs = append(errs, moreErrs...) if bidResponse != nil { + recordVASTTagType(bidder.me, bidResponse, bidder.BidderName) reject := hookExecutor.ExecuteRawBidderResponseStage(bidResponse, string(bidder.BidderName)) if reject != nil { errs = append(errs, reject) diff --git a/exchange/exchange_ow.go b/exchange/exchange_ow.go index 395a828b790..db7bb06aa78 100644 --- a/exchange/exchange_ow.go +++ b/exchange/exchange_ow.go @@ -11,6 +11,7 @@ import ( "github.com/golang/glog" "github.com/prebid/openrtb/v19/openrtb2" "github.com/prebid/openrtb/v19/openrtb3" + "github.com/prebid/prebid-server/adapters" "github.com/prebid/prebid-server/exchange/entities" "github.com/prebid/prebid-server/metrics" pubmaticstats "github.com/prebid/prebid-server/metrics/pubmatic_stats" @@ -27,6 +28,21 @@ const ( vastVersionUndefined = "undefined" ) +const ( + VASTTypeWrapperEndTag = "" + VASTTypeInLineEndTag = "" +) + +// VASTTagType describes the allowed values for VASTTagType +type VASTTagType string + +const ( + WrapperVASTTagType VASTTagType = "Wrapper" + InLineVASTTagType VASTTagType = "InLine" + URLVASTTagType VASTTagType = "URL" + UnknownVASTTagType VASTTagType = "Unknown" +) + var ( vastVersionRegex = regexp.MustCompile(``) ) @@ -174,6 +190,27 @@ func recordVastVersion(metricsEngine metrics.MetricsEngine, adapterBids map[open } } +func recordVASTTagType(metricsEngine metrics.MetricsEngine, adapterBids *adapters.BidderResponse, bidder openrtb_ext.BidderName) { + for _, adapterBid := range adapterBids.Bids { + if adapterBid.BidType == openrtb_ext.BidTypeVideo { + vastTagType := UnknownVASTTagType + if index := strings.LastIndex(adapterBid.Bid.AdM, VASTTypeWrapperEndTag); index != -1 { + vastTagType = WrapperVASTTagType + } else if index := strings.LastIndex(adapterBid.Bid.AdM, VASTTypeInLineEndTag); index != -1 { + vastTagType = InLineVASTTagType + } else if IsUrl(adapterBid.Bid.AdM) { + vastTagType = URLVASTTagType + } + metricsEngine.RecordVASTTagType(string(bidder), string(vastTagType)) + } + } +} + +func IsUrl(adm string) bool { + url, err := url.Parse(adm) + return err == nil && url.Scheme != "" && url.Host != "" +} + // recordPartnerTimeout captures the partnertimeout if any at publisher profile level func recordPartnerTimeout(ctx context.Context, pubID, aliasBidder string) { if metricEnabled, ok := ctx.Value(bidCountMetricEnabled).(bool); metricEnabled && ok { diff --git a/exchange/exchange_ow_test.go b/exchange/exchange_ow_test.go index 8240cd74957..e660ced3377 100644 --- a/exchange/exchange_ow_test.go +++ b/exchange/exchange_ow_test.go @@ -1615,3 +1615,216 @@ func Test_updateSeatNonBidsFloors(t *testing.T) { }) } } + +func TestRecordVASTTagType(t *testing.T) { + var vastXMLAdM = "" + var inlineXMLAdM = "Acudeo CompatibleVAST 2.0 Instream Test 1VAST 2.0 Instream Test 1" + var URLAdM = "http://pubmatic.com" + type args struct { + metricsEngine metrics.MetricsEngine + adapterBids *adapters.BidderResponse + getMetricsEngine func() *metrics.MetricsEngineMock + } + tests := []struct { + name string + args args + }{ + { + name: "no_bids", + args: args{ + adapterBids: &adapters.BidderResponse{}, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + return metricEngine + }, + }, + }, + { + name: "empty_bids_in_seatbids", + args: args{ + adapterBids: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{}, + }, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + return metricEngine + }, + }, + }, + { + name: "empty_adm", + args: args{ + adapterBids: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{ + { + Bid: &openrtb2.Bid{ + AdM: "", + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + }, + }, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "Unknown").Return() + return metricEngine + }, + }, + }, + { + name: "adm_has_wrapped_xml", + args: args{ + adapterBids: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{ + { + Bid: &openrtb2.Bid{ + AdM: vastXMLAdM, + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + }, + }, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "Wrapper").Return() + return metricEngine + }, + }, + }, + { + name: "adm_has_inline_xml", + args: args{ + adapterBids: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{ + { + Bid: &openrtb2.Bid{ + AdM: inlineXMLAdM, + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + }, + }, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "InLine").Return() + return metricEngine + }, + }, + }, + { + name: "adm_has_url", + args: args{ + adapterBids: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{ + { + Bid: &openrtb2.Bid{ + AdM: URLAdM, + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + }, + }, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "URL").Return() + return metricEngine + }, + }, + }, + { + name: "adm_has_wrapper_inline_url_adm", + args: args{ + adapterBids: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{ + { + Bid: &openrtb2.Bid{ + AdM: vastXMLAdM, + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + { + Bid: &openrtb2.Bid{ + AdM: inlineXMLAdM, + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + { + Bid: &openrtb2.Bid{ + AdM: URLAdM, + }, + Seat: "pubmatic", + BidType: openrtb_ext.BidTypeVideo, + }, + }, + }, + getMetricsEngine: func() *metrics.MetricsEngineMock { + metricEngine := &metrics.MetricsEngineMock{} + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "Wrapper").Return() + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "InLine").Return() + metricEngine.Mock.On("RecordVASTTagType", "pubmatic", "URL").Return() + return metricEngine + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockMetricEngine := tt.args.getMetricsEngine() + recordVASTTagType(mockMetricEngine, tt.args.adapterBids, "pubmatic") + mockMetricEngine.AssertExpectations(t) + }) + } +} + +func TestIsUrl(t *testing.T) { + type args struct { + adm string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "empty_url", + args: args{ + adm: "", + }, + want: false, + }, + { + name: "valid_url", + args: args{ + adm: "http://www.test.com", + }, + want: true, + }, + { + name: "invalid_url_without_protocol", + args: args{ + adm: "//www.test.com/vast.xml", + }, + want: false, + }, + { + name: "invalid_url_without_host", + args: args{ + adm: "http://", + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsUrl(tt.args.adm); got != tt.want { + t.Errorf("IsUrl() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index fa9c2770f55..4dee6bbbfc5 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -772,7 +772,7 @@ func TestAdapterCurrency(t *testing.T) { categoriesFetcher: nilCategoryFetcher{}, bidIDGenerator: &mockBidIDGenerator{false, false}, adapterMap: map[openrtb_ext.BidderName]AdaptedBidder{ - openrtb_ext.BidderName("foo"): AdaptBidder(mockBidder, nil, &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderName("foo"), nil, ""), + openrtb_ext.BidderName("appnexus"): AdaptBidder(mockBidder, nil, &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderName("appnexus"), nil, ""), }, } e.requestSplitter = requestSplitter{ @@ -786,7 +786,7 @@ func TestAdapterCurrency(t *testing.T) { Imp: []openrtb2.Imp{{ ID: "some-impression-id", Banner: &openrtb2.Banner{Format: []openrtb2.Format{{W: 300, H: 250}, {W: 300, H: 600}}}, - Ext: json.RawMessage(`{"prebid":{"bidder":{"foo":{"placementId":1}}}}`), + Ext: json.RawMessage(`{"prebid":{"bidder":{"appnexus":{"placementId":1}}}}`), }}, Site: &openrtb2.Site{ Page: "prebid.org", @@ -809,7 +809,7 @@ func TestAdapterCurrency(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "some-request-id", response.ID, "Response ID") assert.Empty(t, response.SeatBid, "Response Bids") - assert.Contains(t, string(response.Ext), `"errors":{"foo":[{"code":5,"message":"The adapter failed to generate any bid requests, but also failed to generate an error explaining why"}]}`, "Response Ext") + assert.Contains(t, string(response.Ext), `"errors":{"appnexus":[{"code":5,"message":"The adapter failed to generate any bid requests, but also failed to generate an error explaining why"}]}`, "Response Ext") // Test Currency Converter Properly Passed To Adapter if assert.NotNil(t, mockBidder.lastExtraRequestInfo, "Currency Conversion Argument") { diff --git a/exchange/utils.go b/exchange/utils.go index 7cb71e69e58..1df406528ce 100644 --- a/exchange/utils.go +++ b/exchange/utils.go @@ -769,10 +769,11 @@ func setUserExtWithCopy(request *openrtb2.BidRequest, userExtJSON json.RawMessag // resolveBidder returns the known BidderName associated with bidder, if bidder is an alias. If it's not an alias, the bidder is returned. func resolveBidder(bidder string, aliases map[string]string) openrtb_ext.BidderName { + normalisedBidderName, _ := openrtb_ext.NormalizeBidderName(bidder) if coreBidder, ok := aliases[bidder]; ok { return openrtb_ext.BidderName(coreBidder) } - return openrtb_ext.BidderName(bidder) + return normalisedBidderName } // parseAliases parses the aliases from the BidRequest diff --git a/go.mod b/go.mod index 15653c53348..7a8abb2f726 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/PubMatic-OpenWrap/prebid-server go 1.20 -replace git.pubmatic.com/vastunwrap => git.pubmatic.com/PubMatic/vastunwrap v0.0.0-20231012062530-95f4848c9fb7 +replace git.pubmatic.com/vastunwrap => git.pubmatic.com/PubMatic/vastunwrap v0.0.0-20231102070946-3c5a3bc1dff5 require ( github.com/DATA-DOG/go-sqlmock v1.5.0 diff --git a/go.sum b/go.sum index e375fadc049..8086862d444 100644 --- a/go.sum +++ b/go.sum @@ -292,8 +292,8 @@ cloud.google.com/go/workflows v1.9.0/go.mod h1:ZGkj1aFIOd9c8Gerkjjq7OW7I5+l6cSvT dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9 h1:VpgP7xuJadIUuKccphEpTJnWhS2jkQyMt6Y7pJCD7fY= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= -git.pubmatic.com/PubMatic/vastunwrap v0.0.0-20231012062530-95f4848c9fb7 h1:YD51mgFU5YJX6ufDBO19QIx/vX38uk7bxNhxYfqEytA= -git.pubmatic.com/PubMatic/vastunwrap v0.0.0-20231012062530-95f4848c9fb7/go.mod h1:dgTumQ6/KYeLbpWq3HVOaqkZos6Q0QGwZmQmEIhQ3To= +git.pubmatic.com/PubMatic/vastunwrap v0.0.0-20231102070946-3c5a3bc1dff5 h1:nK2YP3aS8+5dwc5cMJ8IxI0Sh7yfd858LKmcvwfkOUo= +git.pubmatic.com/PubMatic/vastunwrap v0.0.0-20231102070946-3c5a3bc1dff5/go.mod h1:dgTumQ6/KYeLbpWq3HVOaqkZos6Q0QGwZmQmEIhQ3To= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802 h1:1BDTz0u9nC3//pOCMdNH+CiXJVYJh5UQNCOBG7jbELc= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= diff --git a/hooks/plan.go b/hooks/plan.go index d83db2f77c1..042c60708d5 100644 --- a/hooks/plan.go +++ b/hooks/plan.go @@ -3,7 +3,6 @@ package hooks import ( "time" - "github.com/golang/glog" "github.com/prebid/prebid-server/config" "github.com/prebid/prebid-server/hooks/hookstage" ) @@ -209,9 +208,10 @@ func getGroup[T any](getHookFn hookFn[T], cfg config.HookExecutionGroup) Group[T for _, hookCfg := range cfg.HookSequence { if h, ok := getHookFn(hookCfg.ModuleCode); ok { group.Hooks = append(group.Hooks, HookWrapper[T]{Module: hookCfg.ModuleCode, Code: hookCfg.HookImplCode, Hook: h}) - } else { - glog.Warningf("Not found hook while building hook execution plan: %s %s", hookCfg.ModuleCode, hookCfg.HookImplCode) } + // else { + // glog.Warningf("Not found hook while building hook execution plan: %s %s", hookCfg.ModuleCode, hookCfg.HookImplCode) + // } } return group diff --git a/metrics/config/metrics.go b/metrics/config/metrics.go index ce3799e4b99..13fd1b53bfd 100644 --- a/metrics/config/metrics.go +++ b/metrics/config/metrics.go @@ -150,20 +150,6 @@ func (me *MultiMetricsEngine) RecordAdapterRequest(labels metrics.AdapterLabels) } } -// RecordRejectedBidsForBidder across all engines -func (me *MultiMetricsEngine) RecordRejectedBidsForBidder(bidder openrtb_ext.BidderName) { - for _, thisME := range *me { - thisME.RecordRejectedBidsForBidder(bidder) - } -} - -// RecordDynamicFetchFailure across all engines -func (me *MultiMetricsEngine) RecordDynamicFetchFailure(pubId, code string) { - for _, thisME := range *me { - thisME.RecordDynamicFetchFailure(pubId, code) - } -} - // Keeps track of created and reused connections to adapter bidders and the time from the // connection request, to the connection creation, or reuse from the pool across all engines func (me *MultiMetricsEngine) RecordAdapterConnections(bidderName openrtb_ext.BidderName, connWasReused bool, connWaitTime time.Duration) { @@ -317,10 +303,6 @@ func (me *MultiMetricsEngine) RecordPodImpGenTime(labels metrics.PodLabels, star } } -// RecordRejectedBidsForBidder as a noop -func (me *NilMetricsEngine) RecordRejectedBidsForBidder(bidder openrtb_ext.BidderName) { -} - // RecordPodCombGenTime as a noop func (me *MultiMetricsEngine) RecordPodCombGenTime(labels metrics.PodLabels, elapsedTime time.Duration) { for _, thisME := range *me { @@ -472,25 +454,6 @@ func (me *MultiMetricsEngine) RecordModuleTimeout(labels metrics.ModuleLabels) { thisME.RecordModuleTimeout(labels) } } -func (me *MultiMetricsEngine) RecordRejectedBids(pubid, bidder, code string) { - for _, thisME := range *me { - thisME.RecordRejectedBids(pubid, bidder, code) - } -} - -func (me *MultiMetricsEngine) RecordBids(pubid, profileid, biddder, deal string) { - for _, thisME := range *me { - thisME.RecordBids(pubid, profileid, biddder, deal) - } -} -func (me *MultiMetricsEngine) RecordHttpCounter() { -} - -func (me *MultiMetricsEngine) RecordVastVersion(biddder, vastVersion string) { - for _, thisME := range *me { - thisME.RecordVastVersion(biddder, vastVersion) - } -} // NilMetricsEngine implements the MetricsEngine interface where no metrics are actually captured. This is // used if no metric backend is configured and also for tests. @@ -699,22 +662,3 @@ func (me *NilMetricsEngine) RecordModuleExecutionError(labels metrics.ModuleLabe func (me *NilMetricsEngine) RecordModuleTimeout(labels metrics.ModuleLabels) { } - -// RecordDynamicFetchFailure as a noop -func (me *NilMetricsEngine) RecordDynamicFetchFailure(pubId, code string) { -} - -// RecordRejectedBids as a noop -func (me *NilMetricsEngine) RecordRejectedBids(pubid, bidder, code string) { -} - -// RecordBids as a noop -func (me *NilMetricsEngine) RecordBids(pubid, profileid, biddder, deal string) { -} - -// RecordVastVersion as a noop -func (me *NilMetricsEngine) RecordVastVersion(biddder, vastVersion string) { -} - -func (m *NilMetricsEngine) RecordHttpCounter() { -} diff --git a/metrics/config/metrics_ow.go b/metrics/config/metrics_ow.go index f076df3e61c..5590984a5ec 100644 --- a/metrics/config/metrics_ow.go +++ b/metrics/config/metrics_ow.go @@ -1,6 +1,7 @@ package config import ( + "github.com/prebid/prebid-server/openrtb_ext" "github.com/prometheus/client_golang/prometheus" gometrics "github.com/rcrowley/go-metrics" ) @@ -20,3 +21,70 @@ func NewMetricsRegistry() MetricsRegistry { InfluxRegistry: gometrics.NewPrefixedRegistry("prebidserver."), } } + +func (me *MultiMetricsEngine) RecordVASTTagType(biddder, vastTag string) { + for _, thisME := range *me { + thisME.RecordVASTTagType(biddder, vastTag) + } +} + +func (me *MultiMetricsEngine) RecordRejectedBids(pubid, bidder, code string) { + for _, thisME := range *me { + thisME.RecordRejectedBids(pubid, bidder, code) + } +} + +func (me *MultiMetricsEngine) RecordBids(pubid, profileid, biddder, deal string) { + for _, thisME := range *me { + thisME.RecordBids(pubid, profileid, biddder, deal) + } +} +func (me *MultiMetricsEngine) RecordHttpCounter() { +} + +func (me *MultiMetricsEngine) RecordVastVersion(biddder, vastVersion string) { + for _, thisME := range *me { + thisME.RecordVastVersion(biddder, vastVersion) + } +} + +// RecordRejectedBidsForBidder across all engines +func (me *MultiMetricsEngine) RecordRejectedBidsForBidder(bidder openrtb_ext.BidderName) { + for _, thisME := range *me { + thisME.RecordRejectedBidsForBidder(bidder) + } +} + +// RecordDynamicFetchFailure across all engines +func (me *MultiMetricsEngine) RecordDynamicFetchFailure(pubId, code string) { + for _, thisME := range *me { + thisME.RecordDynamicFetchFailure(pubId, code) + } +} + +// RecordVASTTagType as a noop +func (me *NilMetricsEngine) RecordVASTTagType(biddder, vastTag string) { +} + +// RecordDynamicFetchFailure as a noop +func (me *NilMetricsEngine) RecordDynamicFetchFailure(pubId, code string) { +} + +// RecordRejectedBids as a noop +func (me *NilMetricsEngine) RecordRejectedBids(pubid, bidder, code string) { +} + +// RecordBids as a noop +func (me *NilMetricsEngine) RecordBids(pubid, profileid, biddder, deal string) { +} + +// RecordVastVersion as a noop +func (me *NilMetricsEngine) RecordVastVersion(biddder, vastVersion string) { +} + +func (m *NilMetricsEngine) RecordHttpCounter() { +} + +// RecordRejectedBidsForBidder as a noop +func (me *NilMetricsEngine) RecordRejectedBidsForBidder(bidder openrtb_ext.BidderName) { +} diff --git a/metrics/go_metrics_ow.go b/metrics/go_metrics_ow.go index 2ef51f2f132..e03a402ee7a 100644 --- a/metrics/go_metrics_ow.go +++ b/metrics/go_metrics_ow.go @@ -40,3 +40,7 @@ func (me *Metrics) RecordVastVersion(biddder, vastVersion string) { func (me *Metrics) RecordHttpCounter() { } + +// RecordVASTTagType as a noop +func (me *Metrics) RecordVASTTagType(biddder, vastTag string) { +} diff --git a/metrics/metrics.go b/metrics/metrics.go index 09423971c23..8a77a2f1278 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -429,6 +429,7 @@ func SyncerSetUidStatuses() []SyncerSetUidStatus { // two groups should be consistent within themselves, but comparing numbers between groups // is generally not useful. type MetricsEngine interface { + OWMetricsEngine RecordConnectionAccept(success bool) RecordTMaxTimeout() RecordConnectionClose(success bool) @@ -521,12 +522,4 @@ type MetricsEngine interface { //RecordRejectedBids records the rejected bids labeled by pubid, bidder and reason code RecordRejectedBids(pubid, bidder, code string) - - //RecordBids records the bidder deal bids labeled by pubid, profile, bidder and deal - RecordBids(pubid, profileid, bidder, deal string) - - //RecordVastVersion record the count of vast version labelled by bidder and vast version - RecordVastVersion(coreBidder, vastVersion string) - - RecordHttpCounter() } diff --git a/metrics/metrics_mock_ow.go b/metrics/metrics_mock_ow.go index 40e70c0d79e..a3b06ddd9c8 100644 --- a/metrics/metrics_mock_ow.go +++ b/metrics/metrics_mock_ow.go @@ -40,3 +40,8 @@ func (me *MetricsEngineMock) RecordBids(pubid, profileid, biddder, deal string) func (me *MetricsEngineMock) RecordVastVersion(coreBidder, vastVersion string) { me.Called(coreBidder, vastVersion) } + +// RecordVASTTagType mock +func (me *MetricsEngineMock) RecordVASTTagType(bidder, vastTagType string) { + me.Called(bidder, vastTagType) +} diff --git a/metrics/metrics_ow.go b/metrics/metrics_ow.go new file mode 100644 index 00000000000..958394dd03d --- /dev/null +++ b/metrics/metrics_ow.go @@ -0,0 +1,11 @@ +package metrics + +type OWMetricsEngine interface { + RecordHttpCounter() + //RecordBids records the bidder deal bids labeled by pubid, profile, bidder and deal + RecordBids(pubid, profileid, bidder, deal string) + //RecordVastVersion record the count of vast version labelled by bidder and vast version + RecordVastVersion(coreBidder, vastVersion string) + //RecordVASTTagType record the count of vast tag type labeled by bidder and vast tag + RecordVASTTagType(bidder, vastTagType string) +} diff --git a/metrics/prometheus/prometheus.go b/metrics/prometheus/prometheus.go index 3e7d366b2c4..30615024197 100644 --- a/metrics/prometheus/prometheus.go +++ b/metrics/prometheus/prometheus.go @@ -14,6 +14,7 @@ import ( // Metrics defines the Prometheus metrics backing the MetricsEngine implementation. type Metrics struct { + OWMetrics Registerer prometheus.Registerer Gatherer *prometheus.Registry @@ -57,8 +58,6 @@ type Metrics struct { adsCertSignTimer prometheus.Histogram bidderServerResponseTimer prometheus.Histogram - requestsDuplicateBidIDCounter prometheus.Counter // total request having duplicate bid.id for given bidder - // Adapter Metrics adapterBids *prometheus.CounterVec adapterErrors *prometheus.CounterVec @@ -76,25 +75,12 @@ type Metrics struct { adapterBidResponseSecureMarkupError *prometheus.CounterVec adapterBidResponseSecureMarkupWarn *prometheus.CounterVec - adapterDuplicateBidIDCounter *prometheus.CounterVec - adapterVideoBidDuration *prometheus.HistogramVec - tlsHandhakeTimer *prometheus.HistogramVec + tlsHandhakeTimer *prometheus.HistogramVec // Syncer Metrics syncerRequests *prometheus.CounterVec syncerSets *prometheus.CounterVec - // Rejected Bids - rejectedBids *prometheus.CounterVec - bids *prometheus.CounterVec - vastVersion *prometheus.CounterVec - //rejectedBids *prometheus.CounterVec - accountRejectedBid *prometheus.CounterVec - accountFloorsRequest *prometheus.CounterVec - - //Dynamic Fetch Failure - dynamicFetchFailure *prometheus.CounterVec - // Account Metrics accountRequests *prometheus.CounterVec accountDebugRequests *prometheus.CounterVec @@ -130,21 +116,7 @@ type Metrics struct { moduleTimeouts map[string]*prometheus.CounterVec // Ad Pod Metrics - // podImpGenTimer indicates time taken by impression generator - // algorithm to generate impressions for given ad pod request - podImpGenTimer *prometheus.HistogramVec - - // podImpGenTimer indicates time taken by combination generator - // algorithm to generate combination based on bid response and ad pod request - podCombGenTimer *prometheus.HistogramVec - - // podCompExclTimer indicates time taken by compititve exclusion - // algorithm to generate final pod response based on bid response and ad pod request - podCompExclTimer *prometheus.HistogramVec - metricsDisabled config.DisabledMetrics - - httpCounter prometheus.Counter } const ( @@ -224,6 +196,7 @@ func NewMetrics(cfg config.PrometheusMetrics, reg *prometheus.Registry, disabled if reg == nil { reg = prometheus.NewRegistry() } + metrics.init(cfg, reg) metrics.metricsDisabled = disabledMetrics metrics.connectionsClosed = newCounterWithoutLabels(cfg, reg, @@ -369,11 +342,6 @@ func NewMetrics(cfg config.PrometheusMetrics, reg *prometheus.Registry, disabled // "Seconds to perform TLS Handshake", // standardTimeBuckets) - metrics.bids = newCounter(cfg, reg, - "bids", - "Count of no of bids by publisher id, profile, bidder and deal", - []string{pubIDLabel, profileLabel, bidderLabel, dealLabel}) - metrics.privacyCCPA = newCounter(cfg, reg, "privacy_ccpa", "Count of total requests to Prebid Server where CCPA was provided by source and opt-out .", @@ -554,31 +522,6 @@ func NewMetrics(cfg config.PrometheusMetrics, reg *prometheus.Registry, disabled "Count of total requests to Prebid Server that have stored responses labled by account", []string{accountLabel}) - metrics.accountRejectedBid = newCounter(cfg, reg, - "floors_account_rejected_bid_requests", - "Count of total requests to Prebid Server that have rejected bids due to floors enfocement labled by account", - []string{accountLabel}) - - metrics.accountFloorsRequest = newCounter(cfg, reg, - "floors_account_requests", - "Count of total requests to Prebid Server that have non-zero imp.bidfloor labled by account", - []string{accountLabel}) - - metrics.rejectedBids = newCounter(cfg, reg, - "rejected_bids", - "Count of rejected bids by publisher id, bidder and rejection reason code", - []string{pubIDLabel, bidderLabel, codeLabel}) - - metrics.vastVersion = newCounter(cfg, reg, - "vast_version", - "Count of vast version by bidder and vast version", - []string{adapterLabel, versionLabel}) - - metrics.dynamicFetchFailure = newCounter(cfg, reg, - "floors_account_fetch_err", - "Count of failures in case of dynamic fetch labeled by account", - []string{codeLabel, accountLabel}) - metrics.adsCertSignTimer = newHistogram(cfg, reg, "ads_cert_sign_time", "Seconds to generate an AdsCert header", @@ -645,48 +588,7 @@ func NewMetrics(cfg config.PrometheusMetrics, reg *prometheus.Registry, disabled metrics.Registerer = prometheus.WrapRegistererWithPrefix(metricsPrefix, reg) metrics.Registerer.MustRegister(promCollector.NewGoCollector()) - - metrics.adapterDuplicateBidIDCounter = newCounter(cfg, reg, - "duplicate_bid_ids", - "Number of collisions observed for given adaptor", - []string{adapterLabel}) - - metrics.requestsDuplicateBidIDCounter = newCounterWithoutLabels(cfg, reg, - "requests_having_duplicate_bid_ids", - "Count of number of request where bid collision is detected.") - - // adpod specific metrics - metrics.podImpGenTimer = newHistogramVec(cfg, reg, - "impr_gen", - "Time taken by Ad Pod Impression Generator in seconds", []string{podAlgorithm, podNoOfImpressions}, - // 200 µS, 250 µS, 275 µS, 300 µS - //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) - // 100 µS, 200 µS, 300 µS, 400 µS, 500 µS, 600 µS, - []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) - - metrics.podCombGenTimer = newHistogramVec(cfg, reg, - "comb_gen", - "Time taken by Ad Pod Combination Generator in seconds", []string{podAlgorithm, podTotalCombinations}, - // 200 µS, 250 µS, 275 µS, 300 µS - //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) - []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) - - metrics.podCompExclTimer = newHistogramVec(cfg, reg, - "comp_excl", - "Time taken by Ad Pod Compititve Exclusion in seconds", []string{podAlgorithm, podNoOfResponseBids}, - // 200 µS, 250 µS, 275 µS, 300 µS - //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) - []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) - - metrics.adapterVideoBidDuration = newHistogramVec(cfg, reg, - "adapter_vidbid_dur", - "Video Ad durations returned by the bidder", []string{adapterLabel}, - []float64{4, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, 120}) - preloadLabelValues(&metrics, syncerKeys, moduleStageNames) - - metrics.httpCounter = newHttpCounter(cfg, reg) - return &metrics } @@ -905,22 +807,6 @@ func (m *Metrics) RecordStoredResponse(pubId string) { } } -func (m *Metrics) RecordRejectedBidsForAccount(pubId string) { - if pubId != metrics.PublisherUnknown { - m.accountRejectedBid.With(prometheus.Labels{ - accountLabel: pubId, - }).Inc() - } -} - -func (m *Metrics) RecordFloorsRequestForAccount(pubId string) { - if pubId != metrics.PublisherUnknown { - m.accountFloorsRequest.With(prometheus.Labels{ - accountLabel: pubId, - }).Inc() - } -} - func (m *Metrics) RecordImps(labels metrics.ImpLabels) { m.impressions.With(prometheus.Labels{ isBannerLabel: strconv.FormatBool(labels.BannerImps), @@ -1019,15 +905,6 @@ func (m *Metrics) RecordRejectedBidsForBidder(Adapter openrtb_ext.BidderName) { } } -func (m *Metrics) RecordDynamicFetchFailure(pubId, code string) { - if pubId != metrics.PublisherUnknown { - m.dynamicFetchFailure.With(prometheus.Labels{ - accountLabel: pubId, - codeLabel: code, - }).Inc() - } -} - // Keeps track of created and reused connections to adapter bidders and the time from the // connection request, to the connection creation, or reuse from the pool across all engines func (m *Metrics) RecordAdapterConnections(adapterName openrtb_ext.BidderName, connWasReused bool, connWaitTime time.Duration) { diff --git a/metrics/prometheus/prometheus_ow.go b/metrics/prometheus/prometheus_ow.go index ad80c8f5b6f..be9977626b6 100644 --- a/metrics/prometheus/prometheus_ow.go +++ b/metrics/prometheus/prometheus_ow.go @@ -10,13 +10,44 @@ import ( ) const ( - pubIDLabel = "pubid" - bidderLabel = "bidder" - codeLabel = "code" - profileLabel = "profileid" - dealLabel = "deal" + pubIDLabel = "pubid" + bidderLabel = "bidder" + codeLabel = "code" + profileLabel = "profileid" + dealLabel = "deal" + vastTagTypeLabel = "type" ) +type OWMetrics struct { + vastTagType *prometheus.CounterVec + // Rejected Bids + rejectedBids *prometheus.CounterVec + bids *prometheus.CounterVec + vastVersion *prometheus.CounterVec + //rejectedBids *prometheus.CounterVec + accountRejectedBid *prometheus.CounterVec + accountFloorsRequest *prometheus.CounterVec + + //Dynamic Fetch Failure + dynamicFetchFailure *prometheus.CounterVec + adapterDuplicateBidIDCounter *prometheus.CounterVec + requestsDuplicateBidIDCounter prometheus.Counter // total request having duplicate bid.id for given bidder + adapterVideoBidDuration *prometheus.HistogramVec + + // podImpGenTimer indicates time taken by impression generator + // algorithm to generate impressions for given ad pod request + podImpGenTimer *prometheus.HistogramVec + + // podImpGenTimer indicates time taken by combination generator + // algorithm to generate combination based on bid response and ad pod request + podCombGenTimer *prometheus.HistogramVec + + // podCompExclTimer indicates time taken by compititve exclusion + // algorithm to generate final pod response based on bid response and ad pod request + podCompExclTimer *prometheus.HistogramVec + httpCounter prometheus.Counter +} + func newHttpCounter(cfg config.PrometheusMetrics, registry *prometheus.Registry) prometheus.Counter { httpCounter := prometheus.NewCounter(prometheus.CounterOpts{ Name: "http_requests_total", @@ -30,7 +61,7 @@ func newHttpCounter(cfg config.PrometheusMetrics, registry *prometheus.Registry) // gives the bid response with multiple bids containing same bid.ID // ensure collisions value is greater than 1. This function will not give any error // if collisions = 1 is passed -func (m *Metrics) RecordAdapterDuplicateBidID(adaptor string, collisions int) { +func (m *OWMetrics) RecordAdapterDuplicateBidID(adaptor string, collisions int) { m.adapterDuplicateBidIDCounter.With(prometheus.Labels{ adapterLabel: adaptor, }).Add(float64(collisions)) @@ -38,7 +69,7 @@ func (m *Metrics) RecordAdapterDuplicateBidID(adaptor string, collisions int) { // RecordRequestHavingDuplicateBidID keeps count of request when duplicate bid.id is // detected in partner's response -func (m *Metrics) RecordRequestHavingDuplicateBidID() { +func (m *OWMetrics) RecordRequestHavingDuplicateBidID() { m.requestsDuplicateBidIDCounter.Inc() } @@ -66,32 +97,32 @@ func recordAlgoTime(timer *prometheus.HistogramVec, labels metrics.PodLabels, el // RecordPodImpGenTime records number of impressions generated and time taken // by underneath algorithm to generate them -func (m *Metrics) RecordPodImpGenTime(labels metrics.PodLabels, start time.Time) { +func (m *OWMetrics) RecordPodImpGenTime(labels metrics.PodLabels, start time.Time) { elapsedTime := time.Since(start) recordAlgoTime(m.podImpGenTimer, labels, elapsedTime) } // RecordPodCombGenTime records number of combinations generated and time taken // by underneath algorithm to generate them -func (m *Metrics) RecordPodCombGenTime(labels metrics.PodLabels, elapsedTime time.Duration) { +func (m *OWMetrics) RecordPodCombGenTime(labels metrics.PodLabels, elapsedTime time.Duration) { recordAlgoTime(m.podCombGenTimer, labels, elapsedTime) } // RecordPodCompititveExclusionTime records number of combinations comsumed for forming // final ad pod response and time taken by underneath algorithm to generate them -func (m *Metrics) RecordPodCompititveExclusionTime(labels metrics.PodLabels, elapsedTime time.Duration) { +func (m *OWMetrics) RecordPodCompititveExclusionTime(labels metrics.PodLabels, elapsedTime time.Duration) { recordAlgoTime(m.podCompExclTimer, labels, elapsedTime) } // RecordAdapterVideoBidDuration records actual ad duration (>0) returned by the bidder -func (m *Metrics) RecordAdapterVideoBidDuration(labels metrics.AdapterLabels, videoBidDuration int) { +func (m *OWMetrics) RecordAdapterVideoBidDuration(labels metrics.AdapterLabels, videoBidDuration int) { if videoBidDuration > 0 { m.adapterVideoBidDuration.With(prometheus.Labels{adapterLabel: string(labels.Adapter)}).Observe(float64(videoBidDuration)) } } // RecordRejectedBids records rejected bids labeled by pubid, bidder and reason code -func (m *Metrics) RecordRejectedBids(pubid, biddder, code string) { +func (m *OWMetrics) RecordRejectedBids(pubid, biddder, code string) { m.rejectedBids.With(prometheus.Labels{ pubIDLabel: pubid, bidderLabel: biddder, @@ -100,7 +131,7 @@ func (m *Metrics) RecordRejectedBids(pubid, biddder, code string) { } // RecordBids records bids labeled by pubid, profileid, bidder and deal -func (m *Metrics) RecordBids(pubid, profileid, biddder, deal string) { +func (m *OWMetrics) RecordBids(pubid, profileid, biddder, deal string) { m.bids.With(prometheus.Labels{ pubIDLabel: pubid, profileLabel: profileid, @@ -110,13 +141,120 @@ func (m *Metrics) RecordBids(pubid, profileid, biddder, deal string) { } // RecordVastVersion record the count of vast version labelled by bidder and vast version -func (m *Metrics) RecordVastVersion(coreBiddder, vastVersion string) { +func (m *OWMetrics) RecordVastVersion(coreBiddder, vastVersion string) { m.vastVersion.With(prometheus.Labels{ adapterLabel: coreBiddder, versionLabel: vastVersion, }).Inc() } +// RecordVASTTagType record the count of vast tags labeled by bidder and vast tag +func (m *OWMetrics) RecordVASTTagType(bidder, vastTagType string) { + m.vastTagType.With(prometheus.Labels{ + bidderLabel: bidder, + vastTagTypeLabel: vastTagType, + }).Inc() +} +func (m *Metrics) RecordRejectedBidsForAccount(pubId string) { + if pubId != metrics.PublisherUnknown { + m.accountRejectedBid.With(prometheus.Labels{ + accountLabel: pubId, + }).Inc() + } +} + +func (m *Metrics) RecordFloorsRequestForAccount(pubId string) { + if pubId != metrics.PublisherUnknown { + m.accountFloorsRequest.With(prometheus.Labels{ + accountLabel: pubId, + }).Inc() + } +} +func (m *Metrics) RecordDynamicFetchFailure(pubId, code string) { + if pubId != metrics.PublisherUnknown { + m.dynamicFetchFailure.With(prometheus.Labels{ + accountLabel: pubId, + codeLabel: code, + }).Inc() + } +} + func (m *Metrics) RecordHttpCounter() { m.httpCounter.Inc() } + +func (m *OWMetrics) init(cfg config.PrometheusMetrics, reg *prometheus.Registry) { + m.httpCounter = newHttpCounter(cfg, reg) + m.rejectedBids = newCounter(cfg, reg, + "rejected_bids", + "Count of rejected bids by publisher id, bidder and rejection reason code", + []string{pubIDLabel, bidderLabel, codeLabel}) + + m.vastVersion = newCounter(cfg, reg, + "vast_version", + "Count of vast version by bidder and vast version", + []string{adapterLabel, versionLabel}) + + m.vastTagType = newCounter(cfg, reg, + "vast_tag_type", + "Count of vast tag by bidder and vast tag type (Wrapper, InLine, URL, Unknown)", + []string{bidderLabel, vastTagTypeLabel}) + + m.dynamicFetchFailure = newCounter(cfg, reg, + "floors_account_fetch_err", + "Count of failures in case of dynamic fetch labeled by account", + []string{codeLabel, accountLabel}) + + m.adapterDuplicateBidIDCounter = newCounter(cfg, reg, + "duplicate_bid_ids", + "Number of collisions observed for given adaptor", + []string{adapterLabel}) + + m.requestsDuplicateBidIDCounter = newCounterWithoutLabels(cfg, reg, + "requests_having_duplicate_bid_ids", + "Count of number of request where bid collision is detected.") + + m.adapterVideoBidDuration = newHistogramVec(cfg, reg, + "adapter_vidbid_dur", + "Video Ad durations returned by the bidder", []string{adapterLabel}, + []float64{4, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, 120}) + + m.bids = newCounter(cfg, reg, + "bids", + "Count of no of bids by publisher id, profile, bidder and deal", + []string{pubIDLabel, profileLabel, bidderLabel, dealLabel}) + + m.accountRejectedBid = newCounter(cfg, reg, + "floors_account_rejected_bid_requests", + "Count of total requests to Prebid Server that have rejected bids due to floors enfocement labled by account", + []string{accountLabel}) + + m.accountFloorsRequest = newCounter(cfg, reg, + "floors_account_requests", + "Count of total requests to Prebid Server that have non-zero imp.bidfloor labled by account", + []string{accountLabel}) + + // adpod specific metrics + m.podImpGenTimer = newHistogramVec(cfg, reg, + "impr_gen", + "Time taken by Ad Pod Impression Generator in seconds", []string{podAlgorithm, podNoOfImpressions}, + // 200 µS, 250 µS, 275 µS, 300 µS + //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) + // 100 µS, 200 µS, 300 µS, 400 µS, 500 µS, 600 µS, + []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) + + m.podCombGenTimer = newHistogramVec(cfg, reg, + "comb_gen", + "Time taken by Ad Pod Combination Generator in seconds", []string{podAlgorithm, podTotalCombinations}, + // 200 µS, 250 µS, 275 µS, 300 µS + //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) + []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) + + m.podCompExclTimer = newHistogramVec(cfg, reg, + "comp_excl", + "Time taken by Ad Pod Compititve Exclusion in seconds", []string{podAlgorithm, podNoOfResponseBids}, + // 200 µS, 250 µS, 275 µS, 300 µS + //[]float64{0.000200000, 0.000250000, 0.000275000, 0.000300000}) + []float64{0.000100000, 0.000200000, 0.000300000, 0.000400000, 0.000500000, 0.000600000}) + +} diff --git a/metrics/prometheus/prometheus_ow_test.go b/metrics/prometheus/prometheus_ow_test.go index 838a9c7c301..35a8c9d05b8 100644 --- a/metrics/prometheus/prometheus_ow_test.go +++ b/metrics/prometheus/prometheus_ow_test.go @@ -14,12 +14,12 @@ func TestRecordRejectedBids(t *testing.T) { expCount int } testCases := []struct { - description string - in testIn - out testOut + name string + in testIn + out testOut }{ { - description: "record rejected bids", + name: "record rejected bids", in: testIn{ pubid: "1010", bidder: "bidder", @@ -55,12 +55,12 @@ func TestRecordBids(t *testing.T) { expCount int } testCases := []struct { - description string - in testIn - out testOut + name string + in testIn + out testOut }{ { - description: "record bids", + name: "record bids", in: testIn{ pubid: "1010", bidder: "bidder", @@ -98,12 +98,12 @@ func TestRecordVastVersion(t *testing.T) { expCount int } testCases := []struct { - description string - in testIn - out testOut + name string + in testIn + out testOut }{ { - description: "record vast version", + name: "record vast version", in: testIn{ coreBidder: "bidder", vastVersion: "2.0", @@ -127,3 +127,44 @@ func TestRecordVastVersion(t *testing.T) { }) } } + +func TestRecordVASTTagType(t *testing.T) { + type args struct { + bidder, vastTagType string + } + type want struct { + expCount int + } + tests := []struct { + name string + args args + want want + }{ + { + name: "record_vast_tag", + args: args{ + bidder: "bidder", + vastTagType: "Wrapper", + }, + want: want{ + expCount: 1, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + pm := createMetricsForTesting() + pm.RecordVASTTagType(tt.args.bidder, tt.args.vastTagType) + assertCounterVecValue(t, + "", + "record vastTag", + pm.vastTagType, + float64(tt.want.expCount), + prometheus.Labels{ + bidderLabel: tt.args.bidder, + vastTagTypeLabel: tt.args.vastTagType, + }) + }) + } +} diff --git a/modules/pubmatic/openwrap/adunitconfig/common_test.go b/modules/pubmatic/openwrap/adunitconfig/common_test.go index e6b0f0648ce..db2c242dc02 100644 --- a/modules/pubmatic/openwrap/adunitconfig/common_test.go +++ b/modules/pubmatic/openwrap/adunitconfig/common_test.go @@ -79,7 +79,14 @@ func TestSelectSlot(t *testing.T) { name: "Matching_Slot_config_when_regex_is_present_and_slotconfig_is_absent", args: args{ rCtx: models.RequestCtx{ - AdUnitConfig: getAdunitConfigWithRx(), + AdUnitConfig: func() *adunitconfig.AdUnitConfig { + auc := getAdunitConfigWithRx() + + // Temporary fix to make UT execution consistent. + // TODO: make getRegexMatch()'s loop consistent. + delete(auc.Config, "/15671365/test_adunit1") + return auc + }(), }, h: 300, w: 200, diff --git a/modules/pubmatic/openwrap/adunitconfig/device.go b/modules/pubmatic/openwrap/adunitconfig/device.go index 57b3517a5cb..b52644afd1f 100644 --- a/modules/pubmatic/openwrap/adunitconfig/device.go +++ b/modules/pubmatic/openwrap/adunitconfig/device.go @@ -7,8 +7,10 @@ import ( "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models/adunitconfig" ) -func ReplaceDeviceTypeFromAdUnitConfig(rCtx models.RequestCtx, device *openrtb2.Device) { - if device != nil || device.DeviceType != 0 { +func ReplaceDeviceTypeFromAdUnitConfig(rCtx models.RequestCtx, device **openrtb2.Device) { + if *device == nil { + *device = &openrtb2.Device{} + } else if (*device).DeviceType != 0 { return } @@ -28,9 +30,5 @@ func ReplaceDeviceTypeFromAdUnitConfig(rCtx models.RequestCtx, device *openrtb2. return } - if device == nil { - device = &openrtb2.Device{} - } - - device.DeviceType = adcom1.DeviceType(adUnitCfg.Device.DeviceType) + (*device).DeviceType = adcom1.DeviceType(adUnitCfg.Device.DeviceType) } diff --git a/modules/pubmatic/openwrap/allprocessedbidresponsehook.go b/modules/pubmatic/openwrap/allprocessedbidresponsehook.go new file mode 100644 index 00000000000..6ebaed0ae85 --- /dev/null +++ b/modules/pubmatic/openwrap/allprocessedbidresponsehook.go @@ -0,0 +1,43 @@ +package openwrap + +import ( + "context" + + "github.com/prebid/prebid-server/exchange/entities" + "github.com/prebid/prebid-server/hooks/hookstage" + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/utils" + "github.com/prebid/prebid-server/openrtb_ext" +) + +// handleAllProcessedBidResponsesHook will create unique id for each bid in bid Response. This hook is introduced +// because bidresponse should be updated in mutations and we need modified bidID at the start of auction response hook. +func (m OpenWrap) handleAllProcessedBidResponsesHook( + ctx context.Context, + moduleCtx hookstage.ModuleInvocationContext, + payload hookstage.AllProcessedBidResponsesPayload, +) (hookstage.HookResult[hookstage.AllProcessedBidResponsesPayload], error) { + result := hookstage.HookResult[hookstage.AllProcessedBidResponsesPayload]{ + ChangeSet: hookstage.ChangeSet[hookstage.AllProcessedBidResponsesPayload]{}, + } + + // absence of rctx at this hook means the first hook failed!. Do nothing + if len(moduleCtx.ModuleContext) == 0 { + result.DebugMessages = append(result.DebugMessages, "error: module-ctx not found in handleAllProcessedBidResponsesHook()") + return result, nil + } + + result.ChangeSet.AddMutation(func(apbrp hookstage.AllProcessedBidResponsesPayload) (hookstage.AllProcessedBidResponsesPayload, error) { + updateBidIds(apbrp.Responses) + return apbrp, nil + }, hookstage.MutationUpdate, "update-bid-id") + + return result, nil +} + +func updateBidIds(bidderResponses map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid) { + for _, seatBid := range bidderResponses { + for i := range seatBid.Bids { + seatBid.Bids[i].Bid.ID = utils.SetUniqueBidID(seatBid.Bids[i].Bid.ID, seatBid.Bids[i].GeneratedBidID) + } + } +} diff --git a/modules/pubmatic/openwrap/allprocessedbidresponsehook_test.go b/modules/pubmatic/openwrap/allprocessedbidresponsehook_test.go new file mode 100644 index 00000000000..102e722f4af --- /dev/null +++ b/modules/pubmatic/openwrap/allprocessedbidresponsehook_test.go @@ -0,0 +1,69 @@ +package openwrap + +import ( + "testing" + + "github.com/prebid/openrtb/v19/openrtb2" + "github.com/prebid/prebid-server/exchange/entities" + "github.com/prebid/prebid-server/openrtb_ext" + "github.com/stretchr/testify/assert" +) + +func TestUpdateBidIds(t *testing.T) { + type args struct { + bidderResponses map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid + } + tests := []struct { + name string + args args + want map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid + }{ + { + name: "All bidIds are updated", + args: args{ + bidderResponses: map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid{ + "pubmatic": { + Bids: []*entities.PbsOrtbBid{ + { + Bid: &openrtb2.Bid{ + ID: "bid-1", + }, + GeneratedBidID: "gen-1", + }, + { + Bid: &openrtb2.Bid{ + ID: "bid-2", + }, + GeneratedBidID: "gen-2", + }, + }, + }, + }, + }, + want: map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid{ + "pubmatic": { + Bids: []*entities.PbsOrtbBid{ + { + Bid: &openrtb2.Bid{ + ID: "bid-1::gen-1", + }, + GeneratedBidID: "gen-1", + }, + { + Bid: &openrtb2.Bid{ + ID: "bid-2::gen-2", + }, + GeneratedBidID: "gen-2", + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + updateBidIds(tt.args.bidderResponses) + assert.Equal(t, tt.want, tt.args.bidderResponses, "Bid Id should be equal") + }) + } +} diff --git a/modules/pubmatic/openwrap/auctionresponsehook.go b/modules/pubmatic/openwrap/auctionresponsehook.go index 7810b05b87a..63481756086 100644 --- a/modules/pubmatic/openwrap/auctionresponsehook.go +++ b/modules/pubmatic/openwrap/auctionresponsehook.go @@ -12,6 +12,7 @@ import ( "github.com/prebid/prebid-server/modules/pubmatic/openwrap/adunitconfig" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/tracker" + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/utils" "github.com/prebid/prebid-server/openrtb_ext" ) @@ -255,8 +256,9 @@ func (m OpenWrap) handleAuctionResponseHook( } ap.BidResponse, err = m.applyDefaultBids(rctx, ap.BidResponse) - ap.BidResponse.Ext = rctx.ResponseExt + + resetBidIdtoOriginal(ap.BidResponse) return ap, err }, hookstage.MutationUpdate, "response-body-with-sshb-format") @@ -349,3 +351,11 @@ func getPlatformName(platform string) string { } return platform } + +func resetBidIdtoOriginal(bidResponse *openrtb2.BidResponse) { + for i, seatBid := range bidResponse.SeatBid { + for j, bid := range seatBid.Bid { + bidResponse.SeatBid[i].Bid[j].ID = utils.GetOriginalBidId(bid.ID) + } + } +} diff --git a/modules/pubmatic/openwrap/auctionresponsehook_test.go b/modules/pubmatic/openwrap/auctionresponsehook_test.go index f32be0519e2..a5ec0cf9bae 100644 --- a/modules/pubmatic/openwrap/auctionresponsehook_test.go +++ b/modules/pubmatic/openwrap/auctionresponsehook_test.go @@ -124,3 +124,72 @@ func TestSeatNonBidsInHandleAuctionResponseHook(t *testing.T) { }) } } + +func TestResetBidIdtoOriginal(t *testing.T) { + type args struct { + bidResponse *openrtb2.BidResponse + } + tests := []struct { + name string + args args + want *openrtb2.BidResponse + }{ + { + name: "Reset Bid Id to original", + args: args{ + bidResponse: &openrtb2.BidResponse{ + SeatBid: []openrtb2.SeatBid{ + { + Bid: []openrtb2.Bid{ + { + ID: "original::generated", + }, + { + ID: "original-1::generated-1", + }, + }, + Seat: "pubmatic", + }, + { + Bid: []openrtb2.Bid{ + { + ID: "original-2::generated-2", + }, + }, + Seat: "index", + }, + }, + }, + }, + want: &openrtb2.BidResponse{ + SeatBid: []openrtb2.SeatBid{ + { + Bid: []openrtb2.Bid{ + { + ID: "original", + }, + { + ID: "original-1", + }, + }, + Seat: "pubmatic", + }, + { + Bid: []openrtb2.Bid{ + { + ID: "original-2", + }, + }, + Seat: "index", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resetBidIdtoOriginal(tt.args.bidResponse) + assert.Equal(t, tt.want, tt.args.bidResponse, "Bid Id should reset to original") + }) + } +} diff --git a/modules/pubmatic/openwrap/beforevalidationhook.go b/modules/pubmatic/openwrap/beforevalidationhook.go index 365a4019e6f..f272b47fc0e 100644 --- a/modules/pubmatic/openwrap/beforevalidationhook.go +++ b/modules/pubmatic/openwrap/beforevalidationhook.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" + "github.com/buger/jsonparser" "github.com/prebid/openrtb/v19/openrtb2" "github.com/prebid/prebid-server/hooks/hookstage" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/adapters" @@ -142,12 +143,15 @@ func (m OpenWrap) handleBeforeValidationHook( IncludeWinners: boolutil.BoolPtr(true), } + isAdPodRequest := false disabledSlots := 0 serviceSideBidderPresent := false aliasgvlids := make(map[string]uint16) for i := 0; i < len(payload.BidRequest.Imp); i++ { + slotType := "banner" var adpodExt *models.AdPod + var isAdPodImpression bool imp := payload.BidRequest.Imp[i] impExt := &models.ImpExtension{} @@ -171,6 +175,8 @@ func (m OpenWrap) handleBeforeValidationHook( } if imp.Video != nil { + slotType = "video" + //add stats for video instl impressions if imp.Instl == 1 { m.metricEngine.RecordVideoInstlImpsStats(rCtx.PubIDStr, rCtx.ProfileIDStr) @@ -179,6 +185,16 @@ func (m OpenWrap) handleBeforeValidationHook( // provide custom macros for video event trackers requestExt.Prebid.Macros = getVASTEventMacros(rCtx) } + + if rCtx.IsCTVRequest && imp.Video.Ext != nil { + if _, _, _, err := jsonparser.Get(imp.Video.Ext, "adpod"); err == nil { + isAdPodImpression = true + if !isAdPodRequest { + isAdPodRequest = true + rCtx.MetricsEngine.RecordCTVReqCountWithAdPod(rCtx.PubIDStr, rCtx.ProfileIDStr) + } + } + } } div := "" @@ -212,11 +228,6 @@ func (m OpenWrap) handleBeforeValidationHook( continue } - slotType := "banner" - if imp.Video != nil { - slotType = "video" - } - bidderMeta := make(map[string]models.PartnerData) nonMapped := make(map[string]struct{}) for _, partnerConfig := range rCtx.PartnerConfigMap { @@ -248,11 +259,12 @@ func (m OpenWrap) handleBeforeValidationHook( var isRegex bool var slot, kgpv string var bidderParams json.RawMessage + var matchedSlotKeysVAST []string switch prebidBidderCode { case string(openrtb_ext.BidderPubmatic), models.BidderPubMaticSecondaryAlias: slot, kgpv, isRegex, bidderParams, err = bidderparams.PreparePubMaticParamsV25(rCtx, m.cache, *payload.BidRequest, imp, *impExt, partnerID) case models.BidderVASTBidder: - slot, bidderParams, err = bidderparams.PrepareVASTBidderParams(rCtx, m.cache, *payload.BidRequest, imp, *impExt, partnerID, adpodExt) + slot, bidderParams, matchedSlotKeysVAST, err = bidderparams.PrepareVASTBidderParams(rCtx, m.cache, *payload.BidRequest, imp, *impExt, partnerID, adpodExt) default: slot, kgpv, isRegex, bidderParams, err = bidderparams.PrepareAdapterParamsV25(rCtx, m.cache, *payload.BidRequest, imp, *impExt, partnerID) } @@ -276,6 +288,10 @@ func (m OpenWrap) handleBeforeValidationHook( IsRegex: isRegex, // regex pattern } + for _, bidder := range matchedSlotKeysVAST { + bidderMeta[bidder].VASTTagFlags[bidder] = false + } + if alias, ok := partnerConfig[models.IsAlias]; ok && alias == "1" { if prebidPartnerName, ok := partnerConfig[models.PREBID_PARTNER_NAME]; ok { rCtx.Aliases[bidderCode] = adapters.ResolveOWBidder(prebidPartnerName) @@ -318,19 +334,26 @@ func (m OpenWrap) handleBeforeValidationHook( // cache the details for further processing if _, ok := rCtx.ImpBidCtx[imp.ID]; !ok { rCtx.ImpBidCtx[imp.ID] = models.ImpCtx{ + ImpID: imp.ID, TagID: imp.TagID, Div: div, IsRewardInventory: reward, Type: slotType, Banner: imp.Banner != nil, Video: imp.Video, + Native: imp.Native, IncomingSlots: incomingSlots, Bidders: make(map[string]models.PartnerData), BidCtx: make(map[string]models.BidCtx), NewExt: json.RawMessage(newImpExt), + IsAdPodRequest: isAdPodRequest, } } + if isAdPodImpression { + bidderMeta[string(openrtb_ext.BidderOWPrebidCTV)] = models.PartnerData{} + } + impCtx := rCtx.ImpBidCtx[imp.ID] impCtx.Bidders = bidderMeta impCtx.NonMapped = nonMapped @@ -450,7 +473,7 @@ func (m *OpenWrap) applyProfileChanges(rctx models.RequestCtx, bidRequest *openr } adunitconfig.ReplaceAppObjectFromAdUnitConfig(rctx, bidRequest.App) - adunitconfig.ReplaceDeviceTypeFromAdUnitConfig(rctx, bidRequest.Device) + adunitconfig.ReplaceDeviceTypeFromAdUnitConfig(rctx, &bidRequest.Device) bidRequest.Device.IP = rctx.IP bidRequest.Device.Language = getValidLanguage(bidRequest.Device.Language) diff --git a/modules/pubmatic/openwrap/bidderparams/vast.go b/modules/pubmatic/openwrap/bidderparams/vast.go index 33bfa4d8c2d..0cb6225e30c 100644 --- a/modules/pubmatic/openwrap/bidderparams/vast.go +++ b/modules/pubmatic/openwrap/bidderparams/vast.go @@ -13,34 +13,26 @@ import ( "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" ) -func PrepareVASTBidderParams(rctx models.RequestCtx, cache cache.Cache, bidRequest openrtb2.BidRequest, imp openrtb2.Imp, impExt models.ImpExtension, partnerID int, adpodExt *models.AdPod) (string, json.RawMessage, error) { +func PrepareVASTBidderParams(rctx models.RequestCtx, cache cache.Cache, bidRequest openrtb2.BidRequest, imp openrtb2.Imp, impExt models.ImpExtension, partnerID int, adpodExt *models.AdPod) (string, json.RawMessage, []string, error) { if imp.Video == nil { - return "", nil, nil + return "", nil, nil, nil } slots, slotMap, _, _ := getSlotMeta(rctx, cache, bidRequest, imp, impExt, partnerID) if len(slots) == 0 { - return "", nil, nil + return "", nil, nil, nil } pubVASTTags := cache.GetPublisherVASTTagsFromCache(rctx.PubID) if len(pubVASTTags) == 0 { - return "", nil, nil + return "", nil, nil, nil } matchedSlotKeys, err := getVASTBidderSlotKeys(&imp, slots[0], slotMap, pubVASTTags, adpodExt) if len(matchedSlotKeys) == 0 { - return "", nil, err + return "", nil, nil, err } - // NYC_TODO: - //setting flagmap - // bidderWrapper := &BidderWrapper{VASTagFlags: make(map[string]bool)} - // for _, key := range matchedSlotKeys { - // bidderWrapper.VASTagFlags[key] = false - // } - // impWrapper.Bidder[bidderCode] = bidderWrapper - bidParams := adapters.PrepareVASTBidderParamJSON(&bidRequest, &imp, pubVASTTags, matchedSlotKeys, slotMap, adpodExt) /* @@ -50,7 +42,7 @@ func PrepareVASTBidderParams(rctx models.RequestCtx, cache cache.Cache, bidReque //slotMap:map[/15671365/DMDemo1@com.pubmatic.openbid.app@101:map[param1:6005 param2:test param3:example]] //Ext:{"tags":[{"tagid":"101","url":"sample_url_1","dur":15,"price":"15","params":{"param1":"6005","param2":"test","param3":"example"}}]} */ - return slots[0], bidParams, nil + return slots[0], bidParams, matchedSlotKeys, nil } // getVASTBidderSlotKeys returns all slot keys which are matching to vast tag slot key diff --git a/modules/pubmatic/openwrap/cache/gocache/gocache.go b/modules/pubmatic/openwrap/cache/gocache/gocache.go index 8a65faa80a4..05f4522b5c9 100644 --- a/modules/pubmatic/openwrap/cache/gocache/gocache.go +++ b/modules/pubmatic/openwrap/cache/gocache/gocache.go @@ -31,11 +31,10 @@ func key(format string, v ...interface{}) string { // any db or cache should be injectable type cache struct { sync.Map - cache *gocache.Cache - cfg config.Cache - db database.Database - metricEngine metrics.MetricsEngine - partnerConfigExpiry int + cache *gocache.Cache + cfg config.Cache + db database.Database + metricEngine metrics.MetricsEngine } var c *cache @@ -45,11 +44,10 @@ func New(goCache *gocache.Cache, database database.Database, cfg config.Cache, m cOnce.Do( func() { c = &cache{ - cache: goCache, - db: database, - cfg: cfg, - metricEngine: metricEngine, - partnerConfigExpiry: cfg.CacheDefaultExpiry - 60, // Reduced partnerConfig expiry by 1 minute to avoid inconsistent exipry of partnerConfig, adUnitConfig and WrapperConfig + cache: goCache, + db: database, + cfg: cfg, + metricEngine: metricEngine, } }) return c diff --git a/modules/pubmatic/openwrap/cache/gocache/partner_config.go b/modules/pubmatic/openwrap/cache/gocache/partner_config.go index 2d0ef2d0808..bfb44066228 100644 --- a/modules/pubmatic/openwrap/cache/gocache/partner_config.go +++ b/modules/pubmatic/openwrap/cache/gocache/partner_config.go @@ -64,8 +64,6 @@ func (c *cache) GetPartnerConfigMap(pubID, profileID, displayVersion int, endpoi } func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileID, displayVersion int) (err error) { - var errWrapperSlotMapping error - var errAdunitConfig error cacheKey := key(PUB_HB_PARTNER, pubID, profileID, displayVersion) partnerConfigMap, err := c.db.GetActivePartnerConfigurations(pubID, profileID, displayVersion) if err != nil { @@ -77,7 +75,8 @@ func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileI return fmt.Errorf("there are no active partners for pubId:%d, profileId:%d, displayVersion:%d", pubID, profileID, displayVersion) } - if errWrapperSlotMapping = c.populateCacheWithWrapperSlotMappings(pubID, partnerConfigMap, profileID, displayVersion); errWrapperSlotMapping != nil { + c.cache.Set(cacheKey, partnerConfigMap, getSeconds(c.cfg.CacheDefaultExpiry)) + if errWrapperSlotMapping := c.populateCacheWithWrapperSlotMappings(pubID, partnerConfigMap, profileID, displayVersion); errWrapperSlotMapping != nil { err = models.ErrorWrap(err, errWrapperSlotMapping) queryType := models.WrapperSlotMappingsQuery if displayVersion == 0 { @@ -85,7 +84,7 @@ func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileI } c.metricEngine.RecordDBQueryFailure(queryType, strconv.Itoa(pubID), strconv.Itoa(profileID)) } - if errAdunitConfig = c.populateCacheWithAdunitConfig(pubID, profileID, displayVersion); errAdunitConfig != nil { + if errAdunitConfig := c.populateCacheWithAdunitConfig(pubID, profileID, displayVersion); errAdunitConfig != nil { queryType := models.AdunitConfigQuery if displayVersion == 0 { queryType = models.AdunitConfigForLiveVersion @@ -96,8 +95,5 @@ func (c *cache) getActivePartnerConfigAndPopulateWrapperMappings(pubID, profileI c.metricEngine.RecordDBQueryFailure(queryType, strconv.Itoa(pubID), strconv.Itoa(profileID)) err = models.ErrorWrap(err, errAdunitConfig) } - if errWrapperSlotMapping == nil && errAdunitConfig == nil { - c.cache.Set(cacheKey, partnerConfigMap, getSeconds(c.partnerConfigExpiry)) - } return } diff --git a/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go b/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go index 40865e4814d..75780e39c4b 100644 --- a/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go +++ b/modules/pubmatic/openwrap/cache/gocache/partner_config_test.go @@ -115,67 +115,6 @@ func Test_cache_GetPartnerConfigMap(t *testing.T) { wantErr: true, want: nil, }, - { - name: "db_queries_failed_getting_adunitconfig", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 1000, - VASTTagCacheExpiry: 1000, - }, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: 0, - endpoint: models.EndpointAMP, - }, - setup: func(ctrl *gomock.Controller) (*mock_database.MockDatabase, *mock_metrics.MockMetricsEngine) { - mockDatabase := mock_database.NewMockDatabase(ctrl) - mockEngine := mock_metrics.NewMockMetricsEngine(ctrl) - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, 0).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetPublisherSlotNameHash(testPubID).Return(map[string]string{"adunit@728x90": "2aa34b52a9e941c1594af7565e599c8d"}, nil) - mockDatabase.EXPECT().GetPublisherVASTTags(testPubID).Return(nil, nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, 0).Return(nil, adunitconfig.ErrAdUnitUnmarshal) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, 0).Return(nil, nil) - mockEngine.EXPECT().RecordGetProfileDataTime(models.EndpointAMP, "123", gomock.Any()).Return().Times(1) - mockEngine.EXPECT().RecordDBQueryFailure(models.AdUnitFailUnmarshal, "5890", "123").Return().Times(1) - return mockDatabase, mockEngine - }, - wantErr: true, - want: nil, - }, - - { - name: "db_queries_failed_wrapper_slotmappings", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 1000, - VASTTagCacheExpiry: 1000, - }, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: 0, - endpoint: models.EndpointAMP, - }, - setup: func(ctrl *gomock.Controller) (*mock_database.MockDatabase, *mock_metrics.MockMetricsEngine) { - mockDatabase := mock_database.NewMockDatabase(ctrl) - mockEngine := mock_metrics.NewMockMetricsEngine(ctrl) - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, 0).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetPublisherSlotNameHash(testPubID).Return(map[string]string{"adunit@728x90": "2aa34b52a9e941c1594af7565e599c8d"}, nil) - mockDatabase.EXPECT().GetPublisherVASTTags(testPubID).Return(nil, nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, 0).Return(nil, nil) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, 0).Return(nil, fmt.Errorf("Error from the DB")) - mockEngine.EXPECT().RecordGetProfileDataTime(models.EndpointAMP, "123", gomock.Any()).Return().Times(1) - mockEngine.EXPECT().RecordDBQueryFailure(models.WrapperLiveVersionSlotMappings, "5890", "123").Return().Times(1) - return mockDatabase, mockEngine - }, - wantErr: true, - want: nil, - }, { name: "db_queries_failed_getting_adunitconfig_and_wrapper_slotmappings", fields: fields{ @@ -205,7 +144,17 @@ func Test_cache_GetPartnerConfigMap(t *testing.T) { return mockDatabase, mockEngine }, wantErr: true, - want: nil, + want: map[int]map[string]string{ + 1: { + "partnerId": "1", + "prebidPartnerName": "pubmatic", + "serverSideEnabled": "1", + "level": "multi", + "kgp": "_AU_@_W_x_H", + "timeout": "220", + "bidderCode": "pubmatic", + }, + }, }, } for ind := range tests { @@ -215,9 +164,8 @@ func Test_cache_GetPartnerConfigMap(t *testing.T) { defer ctrl.Finish() c := &cache{ - cache: tt.fields.cache, - cfg: tt.fields.cfg, - partnerConfigExpiry: tt.fields.cfg.CacheDefaultExpiry - 60, + cache: tt.fields.cache, + cfg: tt.fields.cfg, } c.db, c.metricEngine = tt.setup(ctrl) @@ -296,9 +244,8 @@ func Test_cache_GetPartnerConfigMap_LockandLoad(t *testing.T) { defer ctrl.Finish() c := &cache{ - cache: tt.fields.cache, - cfg: tt.fields.cfg, - partnerConfigExpiry: tt.fields.cfg.CacheDefaultExpiry - 60, + cache: tt.fields.cache, + cfg: tt.fields.cfg, } c.db, c.metricEngine = tt.setup(ctrl) @@ -355,85 +302,6 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) { want want setup func() }{ - { - name: "error_in_SlotMapping_from_DB", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 100, - }, - db: mockDatabase, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: testVersionID, - }, - want: want{ - cacheEntry: false, - err: fmt.Errorf("Incorrect Slot Mappings from the DB"), - partnerConfigMap: nil, - }, - setup: func() { - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, testVersionID).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, testVersionID).Return(nil, nil) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect Slot Mappings from the DB")) - mockEngine.EXPECT().RecordDBQueryFailure(models.WrapperSlotMappingsQuery, "5890", "123").Return() - }, - }, - { - name: "error_in_AdUnitConfigRead_from_DB", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 100, - }, - db: mockDatabase, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: testVersionID, - }, - want: want{ - cacheEntry: false, - err: fmt.Errorf("Incorrect AdUnit Config from the DB"), - partnerConfigMap: nil, - }, - setup: func() { - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, testVersionID).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect AdUnit Config from the DB")) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, testVersionID).Return(nil, nil) - mockEngine.EXPECT().RecordDBQueryFailure(models.AdunitConfigQuery, "5890", "123").Return() - }, - }, - { - name: "error_in_SlotMapping_And_AdUnitConfigRead_from_DB", - fields: fields{ - cache: gocache.New(100, 100), - cfg: config.Cache{ - CacheDefaultExpiry: 100, - }, - db: mockDatabase, - }, - args: args{ - pubID: testPubID, - profileID: testProfileID, - displayVersion: testVersionID, - }, - want: want{ - cacheEntry: false, - err: models.ErrorWrap(fmt.Errorf("Incorrect Slot Mappings from the DB"), fmt.Errorf("Incorrect AdUnit Config from the DB")), // errors.New("Incorrect AdUnit Config from the DB: Incorrect Slot Mappings from the DB"), - partnerConfigMap: nil, - }, - setup: func() { - mockDatabase.EXPECT().GetActivePartnerConfigurations(testPubID, testProfileID, testVersionID).Return(formTestPartnerConfig(), nil) - mockDatabase.EXPECT().GetAdunitConfig(testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect AdUnit Config from the DB")) - mockDatabase.EXPECT().GetWrapperSlotMappings(formTestPartnerConfig(), testProfileID, testVersionID).Return(nil, fmt.Errorf("Incorrect Slot Mappings from the DB")) - mockEngine.EXPECT().RecordDBQueryFailure(models.AdunitConfigQuery, "5890", "123").Return() - mockEngine.EXPECT().RecordDBQueryFailure(models.WrapperSlotMappingsQuery, "5890", "123").Return() - }, - }, { name: "error_returning_Active_partner_configuration_from_DB", fields: fields{ @@ -503,7 +371,6 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) { }, nil) }, }, - { name: "empty_partnerConfigMap_from_DB", fields: fields{ @@ -535,22 +402,19 @@ func Test_cache_getActivePartnerConfigAndPopulateWrapperMappings(t *testing.T) { tt.setup() } c := &cache{ - cache: tt.fields.cache, - cfg: tt.fields.cfg, - db: tt.fields.db, - metricEngine: mockEngine, - partnerConfigExpiry: tt.fields.cfg.CacheDefaultExpiry - 60, + cache: tt.fields.cache, + cfg: tt.fields.cfg, + db: tt.fields.db, + metricEngine: mockEngine, } err := c.getActivePartnerConfigAndPopulateWrapperMappings(tt.args.pubID, tt.args.profileID, tt.args.displayVersion) - + assert.Equal(t, tt.want.err, err) cacheKey := key(PUB_HB_PARTNER, tt.args.pubID, tt.args.profileID, tt.args.displayVersion) partnerConfigMap, found := c.Get(cacheKey) if tt.want.cacheEntry { - assert.Equal(t, tt.want.err, err) assert.True(t, found) assert.Equal(t, tt.want.partnerConfigMap, partnerConfigMap) } else { - assert.Equal(t, tt.want.err.Error(), err.Error()) assert.False(t, found) assert.Nil(t, partnerConfigMap) } diff --git a/modules/pubmatic/openwrap/database/mysql/adunit_config.go b/modules/pubmatic/openwrap/database/mysql/adunit_config.go index 22435f21523..6f5fce46c0f 100644 --- a/modules/pubmatic/openwrap/database/mysql/adunit_config.go +++ b/modules/pubmatic/openwrap/database/mysql/adunit_config.go @@ -3,7 +3,6 @@ package mysql import ( "database/sql" "encoding/json" - "errors" "strconv" "strings" @@ -22,32 +21,38 @@ func (db *mySqlDB) GetAdunitConfig(profileID, displayVersion int) (*adunitconfig var adunitConfigJSON string err := db.conn.QueryRow(adunitConfigQuery).Scan(&adunitConfigJSON) - if err != nil && !errors.Is(err, sql.ErrNoRows) { + if err != nil { + if err == sql.ErrNoRows { + return nil, nil + } return nil, err } - if len(adunitConfigJSON) > 0 { - adunitConfig := &adunitconfig.AdUnitConfig{} - err = json.Unmarshal([]byte(adunitConfigJSON), &adunitConfig) - if err != nil { - return nil, adunitconfig.ErrAdUnitUnmarshal - } + adunitConfig := &adunitconfig.AdUnitConfig{} + err = json.Unmarshal([]byte(adunitConfigJSON), &adunitConfig) + if err != nil { + return nil, adunitconfig.ErrAdUnitUnmarshal + } - for k, v := range adunitConfig.Config { - adunitConfig.Config[strings.ToLower(k)] = v - // shall we delete the orignal key-val? - } + for k, v := range adunitConfig.Config { + adunitConfig.Config[strings.ToLower(k)] = v + // shall we delete the orignal key-val? + } - if adunitConfig.ConfigPattern == "" { - //Default configPattern value is "_AU_" if not present in db config - adunitConfig.ConfigPattern = models.MACRO_AD_UNIT_ID - } + if adunitConfig.ConfigPattern == "" { + //Default configPattern value is "_AU_" if not present in db config + adunitConfig.ConfigPattern = models.MACRO_AD_UNIT_ID + } - if _, ok := adunitConfig.Config["default"]; !ok { - adunitConfig.Config["default"] = &adunitconfig.AdConfig{} - } + // safe check for old legacy profiles + // new profiles cannot be created as UI-API has config object validation + if adunitConfig.Config == nil { + adunitConfig.Config = make(map[string]*adunitconfig.AdConfig) + } - return adunitConfig, nil + if _, ok := adunitConfig.Config["default"]; !ok { + adunitConfig.Config["default"] = &adunitconfig.AdConfig{} } - return nil, nil + + return adunitConfig, err } diff --git a/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go b/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go index 48447513e5e..0bd64c1530c 100644 --- a/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go +++ b/modules/pubmatic/openwrap/database/mysql/adunit_config_test.go @@ -2,7 +2,6 @@ package mysql import ( "database/sql" - "errors" "regexp" "testing" @@ -42,7 +41,7 @@ func Test_mySqlDB_GetAdunitConfig(t *testing.T) { }, }, { - name: "No rows for given query", + name: "adunitconfig not found for profile(No rows error)", fields: fields{ cfg: config.Database{ Queries: config.Queries{ @@ -61,55 +60,7 @@ func Test_mySqlDB_GetAdunitConfig(t *testing.T) { if err != nil { t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) } - mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+) LIVE")).WillReturnError(sql.ErrNoRows) - return db - }, - }, - { - name: "Error in QueryRow for given query", - fields: fields{ - cfg: config.Database{ - Queries: config.Queries{ - GetAdunitConfigForLiveVersion: "^SELECT (.+) FROM wrapper_media_config (.+) LIVE", - }, - }, - }, - args: args{ - profileID: 5890, - displayVersion: 0, - }, - want: nil, - wantErr: true, - setup: func() *sql.DB { - db, mock, err := sqlmock.New() - if err != nil { - t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) - } - mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+) LIVE")).WillReturnError(errors.New("Error in query execution")) - return db - }, - }, - { - name: "Empty content return", - fields: fields{ - cfg: config.Database{ - Queries: config.Queries{ - GetAdunitConfigForLiveVersion: "^SELECT (.+) FROM wrapper_media_config (.+) LIVE", - }, - }, - }, - args: args{ - profileID: 5890, - displayVersion: 0, - }, - want: nil, - wantErr: false, - setup: func() *sql.DB { - db, mock, err := sqlmock.New() - if err != nil { - t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) - } - rows := sqlmock.NewRows([]string{"adunitConfig"}).AddRow(``) + rows := sqlmock.NewRows([]string{}) mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+) LIVE")).WillReturnRows(rows) return db }, @@ -260,6 +211,36 @@ func Test_mySqlDB_GetAdunitConfig(t *testing.T) { return db }, }, + { + name: "config key not present", + fields: fields{ + cfg: config.Database{ + Queries: config.Queries{ + GetAdunitConfigQuery: "^SELECT (.+) FROM wrapper_media_config (.+)", + }, + }, + }, + args: args{ + profileID: 5890, + displayVersion: 1, + }, + want: &adunitconfig.AdUnitConfig{ + ConfigPattern: "_DIV_", + Config: map[string]*adunitconfig.AdConfig{ + "default": {}, + }, + }, + wantErr: false, + setup: func() *sql.DB { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) + } + rows := sqlmock.NewRows([]string{"adunitConfig"}).AddRow(`{"configPattern": "_DIV_"}`) + mock.ExpectQuery(regexp.QuoteMeta("^SELECT (.+) FROM wrapper_media_config (.+)")).WillReturnRows(rows) + return db + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/modules/pubmatic/openwrap/entrypointhook.go b/modules/pubmatic/openwrap/entrypointhook.go index 85a86224e8b..1b3b2c11fda 100644 --- a/modules/pubmatic/openwrap/entrypointhook.go +++ b/modules/pubmatic/openwrap/entrypointhook.go @@ -18,10 +18,13 @@ import ( ) const ( - OpenWrapAuction = "/pbs/openrtb2/auction" - OpenWrapV25 = "/openrtb/2.5" - OpenWrapV25Video = "/openrtb/2.5/video" - OpenWrapAmp = "/openrtb/amp" + OpenWrapAuction = "/pbs/openrtb2/auction" + OpenWrapV25 = "/openrtb/2.5" + OpenWrapV25Video = "/openrtb/2.5/video" + OpenWrapOpenRTBVideo = "/video/openrtb" + OpenWrapVAST = "/video/vast" + OpenWrapJSON = "/video/json" + OpenWrapAmp = "/amp" ) func (m OpenWrap) handleEntrypointHook( @@ -36,6 +39,7 @@ func (m OpenWrap) handleEntrypointHook( return result, nil } + var pubid int var endpoint string var err error var requestExtWrapper models.RequestExtWrapper @@ -57,8 +61,17 @@ func (m OpenWrap) handleEntrypointHook( requestExtWrapper, err = v25.ConvertVideoToAuctionRequest(payload, &result) endpoint = models.EndpointVideo case OpenWrapAmp: - // requestExtWrapper, err = models.GetQueryParamRequestExtWrapper(payload.Body) + requestExtWrapper, pubid, err = models.GetQueryParamRequestExtWrapper(payload.Request) endpoint = models.EndpointAMP + case OpenWrapOpenRTBVideo: + requestExtWrapper, err = models.GetRequestExtWrapper(payload.Body, "ext", "wrapper") + endpoint = models.EndpointVideo + case OpenWrapVAST: + requestExtWrapper, err = models.GetRequestExtWrapper(payload.Body, "ext", "wrapper") + endpoint = models.EndpointVAST + case OpenWrapJSON: + requestExtWrapper, err = models.GetRequestExtWrapper(payload.Body, "ext", "wrapper") + endpoint = models.EndpointJson default: // we should return from here } @@ -125,6 +138,11 @@ func (m OpenWrap) handleEntrypointHook( rCtx.LoggerImpressionID = uuid.NewV4().String() } + // temp, for AMP, etc + if pubid != 0 { + rCtx.PubID = pubid + } + result.ModuleContext = make(hookstage.ModuleContext) result.ModuleContext["rctx"] = rCtx diff --git a/modules/pubmatic/openwrap/metrics/config/metrics.go b/modules/pubmatic/openwrap/metrics/config/metrics.go index e5f31ee72dd..cf67a7e652a 100644 --- a/modules/pubmatic/openwrap/metrics/config/metrics.go +++ b/modules/pubmatic/openwrap/metrics/config/metrics.go @@ -445,10 +445,3 @@ func (me *MultiMetricsEngine) RecordOWServerPanic(endpoint, methodName, nodeName thisME.RecordOWServerPanic(endpoint, methodName, nodeName, podName) } } - -// RecordCountry records count of requests received with req.device.geo.country -func (me *MultiMetricsEngine) RecordCountry(pubID string) { - for _, thisME := range *me { - thisME.RecordCountry(pubID) - } -} diff --git a/modules/pubmatic/openwrap/metrics/config/metrics_test.go b/modules/pubmatic/openwrap/metrics/config/metrics_test.go index 2523657934a..6f80fed117e 100644 --- a/modules/pubmatic/openwrap/metrics/config/metrics_test.go +++ b/modules/pubmatic/openwrap/metrics/config/metrics_test.go @@ -217,7 +217,6 @@ func TestRecordFunctionForMultiMetricsEngine(t *testing.T) { mockEngine.EXPECT().RecordSendLoggerDataTime("requestType", "profileid", time.Second) mockEngine.EXPECT().RecordRequestTime("requestType", time.Second) mockEngine.EXPECT().RecordOWServerPanic("endpoint", "methodName", "nodeName", "podName") - mockEngine.EXPECT().RecordCountry("pubID") // create the multi-metric engine multiMetricEngine := MultiMetricsEngine{} @@ -280,5 +279,4 @@ func TestRecordFunctionForMultiMetricsEngine(t *testing.T) { multiMetricEngine.RecordSendLoggerDataTime("requestType", "profileid", time.Second) multiMetricEngine.RecordRequestTime("requestType", time.Second) multiMetricEngine.RecordOWServerPanic("endpoint", "methodName", "nodeName", "podName") - multiMetricEngine.RecordCountry("pubID") } diff --git a/modules/pubmatic/openwrap/metrics/metrics.go b/modules/pubmatic/openwrap/metrics/metrics.go index 7848f993bc0..3d00bf73219 100644 --- a/modules/pubmatic/openwrap/metrics/metrics.go +++ b/modules/pubmatic/openwrap/metrics/metrics.go @@ -72,5 +72,4 @@ type MetricsEngine interface { RecordSendLoggerDataTime(requestType, profileid string, sendTime time.Duration) RecordRequestTime(requestType string, requestTime time.Duration) RecordOWServerPanic(endpoint, methodName, nodeName, podName string) - RecordCountry(pubID string) } diff --git a/modules/pubmatic/openwrap/metrics/mock/mock.go b/modules/pubmatic/openwrap/metrics/mock/mock.go index 392ed5ca864..03dc3b0ae19 100644 --- a/modules/pubmatic/openwrap/metrics/mock/mock.go +++ b/modules/pubmatic/openwrap/metrics/mock/mock.go @@ -323,18 +323,6 @@ func (mr *MockMetricsEngineMockRecorder) RecordOWServerPanic(arg0, arg1, arg2, a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordOWServerPanic", reflect.TypeOf((*MockMetricsEngine)(nil).RecordOWServerPanic), arg0, arg1, arg2, arg3) } -// RecordCountry mocks base method. -func (m *MockMetricsEngine) RecordCountry(arg0 string) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "RecordCountry", arg0) -} - -// RecordBids indicates an expected call of RecordCountry. -func (mr *MockMetricsEngineMockRecorder) RecordCountry(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordCountry", reflect.TypeOf((*MockMetricsEngine)(nil).RecordCountry), arg0) -} - // RecordOpenWrapServerPanicStats mocks base method. func (m *MockMetricsEngine) RecordOpenWrapServerPanicStats(arg0, arg1 string) { m.ctrl.T.Helper() diff --git a/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go b/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go index b5d5bd2e631..94e439773bf 100644 --- a/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go +++ b/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go @@ -65,7 +65,6 @@ type Metrics struct { panicCounts *prometheus.CounterVec sendLoggerData *prometheus.HistogramVec owRequestTime *prometheus.HistogramVec - country *prometheus.CounterVec } const ( diff --git a/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb.go b/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb.go index f47e27b0333..bad86d6ce13 100644 --- a/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb.go +++ b/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb.go @@ -107,11 +107,6 @@ func newSSHBMetrics(metrics *Metrics, cfg *config.PrometheusMetrics, promRegistr "Counts the header-bidding server panic.", []string{nodeNameLabel, podNameLabel, methodNameLabel, endpointLabel}) - metrics.country = newCounter(cfg, promRegistry, - "sshb_country", - "Count of requests received with publishers Country by publisher id.", - []string{pubIDLabel}) - preloadLabelValues(metrics) } @@ -199,13 +194,6 @@ func (m *Metrics) RecordOWServerPanic(endpoint, methodName, nodeName, podName st }).Inc() } -// RecordCountry records count of requests received with req.device.geo.country -func (m *Metrics) RecordCountry(pubID string) { - m.country.With(prometheus.Labels{ - pubIDLabel: pubID, - }).Inc() -} - func preloadLabelValues(m *Metrics) { var ( requestStatusValues = requestStatusesAsString() diff --git a/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb_test.go b/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb_test.go index 7a6a7842017..fc3423529ff 100644 --- a/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb_test.go +++ b/modules/pubmatic/openwrap/metrics/prometheus/prometheus_sshb_test.go @@ -387,31 +387,3 @@ func TestRegisterLabelPermutations(t *testing.T) { assert.ElementsMatch(t, test.expectedLabels, resultLabels) } } - -func TestMetricsRecordCountry(t *testing.T) { - m := createMetricsForTesting() - type args struct { - pubID string - } - tests := []struct { - name string - args args - want float64 - }{ - { - name: "call_record_country", - args: args{ - pubID: "1010", - }, - want: 1, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - m.RecordCountry(tt.args.pubID) - assertCounterVecValue(t, "", "country", m.country, tt.want, prometheus.Labels{ - pubIDLabel: tt.args.pubID, - }) - }) - } -} diff --git a/modules/pubmatic/openwrap/metrics/stats/client.go b/modules/pubmatic/openwrap/metrics/stats/client.go index 3f04661074a..c87825f0f54 100644 --- a/modules/pubmatic/openwrap/metrics/stats/client.go +++ b/modules/pubmatic/openwrap/metrics/stats/client.go @@ -123,13 +123,14 @@ func (sc *Client) prepareStatsForPublishing() { // in case of failure, it retries to send for Client.config.Retries number of times. func (sc *Client) publishStatsToServer(statMap map[string]int) int { - sb, err := json.Marshal(statMap) + statJson, err := json.Marshal(statMap) if err != nil { glog.Errorf("[stats_fail] Json unmarshal fail: %v", err) return statusSetupFail } - req, err := http.NewRequest(http.MethodPost, sc.endpoint, bytes.NewBuffer(sb)) + glog.V(3).Infof("[stats] nstats:[%d] data:[%s]", len(statMap), statJson) + req, err := http.NewRequest(http.MethodPost, sc.endpoint, bytes.NewBuffer(statJson)) if err != nil { glog.Errorf("[stats_fail] Failed to form request to sent stats to server: %v", err) return statusSetupFail diff --git a/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go b/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go index bb3b23d2c8c..86a1936e6f5 100644 --- a/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go +++ b/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go @@ -339,4 +339,3 @@ func (st *StatsTCP) RecordCtvUaAccuracy(pubId, status string) func (st *StatsTCP) RecordSendLoggerDataTime(requestType, profileid string, sendTime time.Duration) {} func (st *StatsTCP) RecordRequestTime(requestType string, requestTime time.Duration) {} func (st *StatsTCP) RecordOWServerPanic(endpoint, methodName, nodeName, podName string) {} -func (st *StatsTCP) RecordCountry(pubID string) {} diff --git a/modules/pubmatic/openwrap/models/amp.go b/modules/pubmatic/openwrap/models/amp.go new file mode 100644 index 00000000000..1cd1a513b93 --- /dev/null +++ b/modules/pubmatic/openwrap/models/amp.go @@ -0,0 +1,19 @@ +package models + +import ( + "net/http" + "strconv" +) + +func GetQueryParamRequestExtWrapper(request *http.Request) (RequestExtWrapper, int, error) { + extWrapper := RequestExtWrapper{ + SSAuctionFlag: -1, + } + + values := request.URL.Query() + pubid, _ := strconv.Atoi(values.Get(PUBID_KEY)) + extWrapper.ProfileId, _ = strconv.Atoi(values.Get(PROFILEID_KEY)) + extWrapper.VersionId, _ = strconv.Atoi(values.Get(VERSION_KEY)) + + return extWrapper, pubid, nil +} diff --git a/modules/pubmatic/openwrap/models/constants.go b/modules/pubmatic/openwrap/models/constants.go index 4a601d50128..19fe53f676e 100755 --- a/modules/pubmatic/openwrap/models/constants.go +++ b/modules/pubmatic/openwrap/models/constants.go @@ -396,7 +396,11 @@ const ( ContextOWLoggerKey contextKey = "owlogger" ) -const Pipe = "|" +const ( + Pipe = "|" + BidIdSeparator = "::" +) + const ( EndpointV25 = "v25" EndpointAMP = "amp" @@ -405,6 +409,7 @@ const ( EndpointORTB = "ortb" EndpointVAST = "vast" EndpointOWS2S = "ows2s" + EndPointCTV = "ctv" Openwrap = "openwrap" ImpTypeBanner = "banner" ImpTypeVideo = "video" diff --git a/modules/pubmatic/openwrap/models/openwrap.go b/modules/pubmatic/openwrap/models/openwrap.go index 5bddd629de6..d33107f5469 100644 --- a/modules/pubmatic/openwrap/models/openwrap.go +++ b/modules/pubmatic/openwrap/models/openwrap.go @@ -103,6 +103,7 @@ type ImpCtx struct { IsRewardInventory *int8 Banner bool Video *openrtb2.Video + Native *openrtb2.Native IncomingSlots []string Type string // banner, video, native, etc Bidders map[string]PartnerData @@ -113,6 +114,10 @@ type ImpCtx struct { BannerAdUnitCtx AdUnitCtx VideoAdUnitCtx AdUnitCtx + + //temp + BidderError string + IsAdPodRequest bool } type PartnerData struct { @@ -123,6 +128,8 @@ type PartnerData struct { KGPV string IsRegex bool Params json.RawMessage + VASTTagFlag bool + VASTTagFlags map[string]bool } type BidCtx struct { diff --git a/modules/pubmatic/openwrap/module.go b/modules/pubmatic/openwrap/module.go index e569ffb035a..4c2cfcbd9e4 100644 --- a/modules/pubmatic/openwrap/module.go +++ b/modules/pubmatic/openwrap/module.go @@ -53,6 +53,25 @@ func (m OpenWrap) HandleBeforeValidationHook( return m.handleBeforeValidationHook(ctx, miCtx, payload) } +func (m OpenWrap) HandleAllProcessedBidResponsesHook( + ctx context.Context, + miCtx hookstage.ModuleInvocationContext, + payload hookstage.AllProcessedBidResponsesPayload, +) (hookstage.HookResult[hookstage.AllProcessedBidResponsesPayload], error) { + defer func() { + if r := recover(); r != nil { + m.metricEngine.RecordOpenWrapServerPanicStats(m.cfg.Server.HostName, "HandleAllProcessedBidResponsesHook") + request, err := json.Marshal(payload) + if err != nil { + glog.Error("request:" + string(request) + ". err: " + err.Error() + ". stacktrace:" + string(debug.Stack())) + } + glog.Error("request:" + string(request) + ". stacktrace:" + string(debug.Stack())) + } + }() + + return m.handleAllProcessedBidResponsesHook(ctx, miCtx, payload) +} + func (m OpenWrap) HandleAuctionResponseHook( ctx context.Context, miCtx hookstage.ModuleInvocationContext, diff --git a/modules/pubmatic/openwrap/openwrap.go b/modules/pubmatic/openwrap/openwrap.go index 7e6d970c324..8771338704f 100644 --- a/modules/pubmatic/openwrap/openwrap.go +++ b/modules/pubmatic/openwrap/openwrap.go @@ -20,6 +20,7 @@ import ( metrics "github.com/prebid/prebid-server/modules/pubmatic/openwrap/metrics" metrics_cfg "github.com/prebid/prebid-server/modules/pubmatic/openwrap/metrics/config" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/tbf" ) const ( @@ -66,8 +67,12 @@ func initOpenWrap(rawCfg json.RawMessage, moduleDeps moduledeps.ModuleDeps) (Ope owCache := ow_gocache.New(cache, db, cfg.Cache, &metricEngine) + // Init FSC and related services fullscreenclickability.Init(owCache, cfg.Cache.CacheDefaultExpiry) + // Init TBF (tracking-beacon-first) feature related services + tbf.Init(cfg.Cache.CacheDefaultExpiry, owCache) + return OpenWrap{ cfg: cfg, cache: owCache, diff --git a/modules/pubmatic/openwrap/openwrap_sshb.go b/modules/pubmatic/openwrap/openwrap_sshb.go new file mode 100644 index 00000000000..057cc9d2bbb --- /dev/null +++ b/modules/pubmatic/openwrap/openwrap_sshb.go @@ -0,0 +1,38 @@ +package openwrap + +import ( + cache "github.com/prebid/prebid-server/modules/pubmatic/openwrap/cache" + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/config" + metrics "github.com/prebid/prebid-server/modules/pubmatic/openwrap/metrics" +) + +// GetConfig Temporary function to expose config to SSHB +func (ow OpenWrap) GetConfig() config.Config { + return ow.cfg + +} + +// GetCache Temporary function to expose cache to SSHB +func (ow OpenWrap) GetCache() cache.Cache { + return ow.cache +} + +// GetMetricEngine Temporary function to expose mertics to SSHB +func (ow OpenWrap) GetMetricEngine() metrics.MetricsEngine { + return ow.metricEngine +} + +// SetConfig Temporary function to expose config to SSHB +func (ow *OpenWrap) SetConfig(c config.Config) { + ow.cfg = c +} + +// GetCache Temporary function to expose cache to SSHB +func (ow *OpenWrap) SetCache(c cache.Cache) { + ow.cache = c +} + +// GetMetricEngine Temporary function to expose mertics to SSHB +func (ow *OpenWrap) SetMetricEngine(m metrics.MetricsEngine) { + ow.metricEngine = m +} diff --git a/modules/pubmatic/openwrap/targeting.go b/modules/pubmatic/openwrap/targeting.go index c749c6c53e3..60558e708a3 100644 --- a/modules/pubmatic/openwrap/targeting.go +++ b/modules/pubmatic/openwrap/targeting.go @@ -7,6 +7,7 @@ import ( "github.com/prebid/openrtb/v19/openrtb2" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/fullscreenclickability" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/utils" "github.com/prebid/prebid-server/openrtb_ext" ) @@ -27,11 +28,31 @@ func allowTargetingKey(key string) bool { return strings.HasPrefix(key, models.HbBuyIdPrefix) } -func addPWTTargetingForBid(rctx models.RequestCtx, bidResponse *openrtb2.BidResponse) (droppedBids map[string][]openrtb2.Bid, warnings []string) { - if rctx.Platform != models.PLATFORM_APP { - return +func addInAppTargettingKeys(targeting map[string]string, seat string, ecpm float64, bid *openrtb2.Bid, isWinningBid bool) { + if isWinningBid { + targeting[models.CreatePartnerKey(seat, models.PWT_SLOTID)] = utils.GetOriginalBidId(bid.ID) + targeting[models.CreatePartnerKey(seat, models.PWT_SZ)] = models.GetSize(bid.W, bid.H) + targeting[models.CreatePartnerKey(seat, models.PWT_PARTNERID)] = seat + targeting[models.CreatePartnerKey(seat, models.PWT_ECPM)] = fmt.Sprintf("%.2f", ecpm) + targeting[models.CreatePartnerKey(seat, models.PWT_PLATFORM)] = getPlatformName(models.PLATFORM_APP) + targeting[models.CreatePartnerKey(seat, models.PWT_BIDSTATUS)] = "1" + if len(bid.DealID) != 0 { + targeting[models.CreatePartnerKey(seat, models.PWT_DEALID)] = bid.DealID + } } + targeting[models.PWT_SLOTID] = utils.GetOriginalBidId(bid.ID) + targeting[models.PWT_BIDSTATUS] = "1" + targeting[models.PWT_SZ] = models.GetSize(bid.W, bid.H) + targeting[models.PWT_PARTNERID] = seat + targeting[models.PWT_ECPM] = fmt.Sprintf("%.2f", ecpm) + targeting[models.PWT_PLATFORM] = getPlatformName(models.PLATFORM_APP) + if len(bid.DealID) != 0 { + targeting[models.PWT_DEALID] = bid.DealID + } +} + +func addPWTTargetingForBid(rctx models.RequestCtx, bidResponse *openrtb2.BidResponse) (droppedBids map[string][]openrtb2.Bid, warnings []string) { if !rctx.SendAllBids { droppedBids = make(map[string][]openrtb2.Bid) } @@ -70,16 +91,10 @@ func addPWTTargetingForBid(rctx models.RequestCtx, bidResponse *openrtb2.BidResp delete(bidCtx.Prebid.Targeting, key) } - bidCtx.Prebid.Targeting = newTargeting - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_SLOTID)] = bid.ID - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_SZ)] = models.GetSize(bid.W, bid.H) - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_PARTNERID)] = seatBid.Seat - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_ECPM)] = fmt.Sprintf("%.2f", bidCtx.NetECPM) - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_PLATFORM)] = getPlatformName(rctx.Platform) - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_BIDSTATUS)] = "1" - if len(bid.DealID) != 0 { - bidCtx.Prebid.Targeting[models.CreatePartnerKey(seatBid.Seat, models.PWT_DEALID)] = bid.DealID + if rctx.Platform == models.PLATFORM_APP { + addInAppTargettingKeys(newTargeting, seatBid.Seat, bidCtx.NetECPM, &bid, isWinningBid) } + bidCtx.Prebid.Targeting = newTargeting if isWinningBid { if rctx.SendAllBids { @@ -88,17 +103,8 @@ func addPWTTargetingForBid(rctx models.RequestCtx, bidResponse *openrtb2.BidResp if fullscreenclickability.IsFscApplicable(rctx.PubID, seatBid.Seat, bidCtx.DspId) { bidCtx.Fsc = 1 } - bidCtx.Prebid.Targeting[models.PWT_SLOTID] = bid.ID - bidCtx.Prebid.Targeting[models.PWT_BIDSTATUS] = "1" - bidCtx.Prebid.Targeting[models.PWT_SZ] = models.GetSize(bid.W, bid.H) - bidCtx.Prebid.Targeting[models.PWT_PARTNERID] = seatBid.Seat - bidCtx.Prebid.Targeting[models.PWT_ECPM] = fmt.Sprintf("%.2f", bidCtx.NetECPM) - bidCtx.Prebid.Targeting[models.PWT_PLATFORM] = getPlatformName(rctx.Platform) - if len(bid.DealID) != 0 { - bidCtx.Prebid.Targeting[models.PWT_DEALID] = bid.DealID - } } else if !rctx.SendAllBids { - warnings = append(warnings, "dropping bid "+bid.ID+" as sendAllBids is disabled") + warnings = append(warnings, "dropping bid "+utils.GetOriginalBidId(bid.ID)+" as sendAllBids is disabled") } // cache for bid details for logger and tracker diff --git a/modules/pubmatic/openwrap/tracker/create.go b/modules/pubmatic/openwrap/tracker/create.go index 35303c3a1e3..c20eeb70b49 100644 --- a/modules/pubmatic/openwrap/tracker/create.go +++ b/modules/pubmatic/openwrap/tracker/create.go @@ -9,6 +9,7 @@ import ( "github.com/prebid/openrtb/v19/openrtb2" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/bidderparams" "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/utils" ) func CreateTrackers(rctx models.RequestCtx, bidResponse *openrtb2.BidResponse) map[string]models.OWTracker { @@ -132,8 +133,8 @@ func CreateTrackers(rctx models.RequestCtx, bidResponse *openrtb2.BidResponse) m tracker.PartnerInfo = models.Partner{ PartnerID: partnerID, BidderCode: seatBid.Seat, - BidID: bid.ID, - OrigBidID: bid.ID, + BidID: utils.GetOriginalBidId(bid.ID), + OrigBidID: utils.GetOriginalBidId(bid.ID), KGPV: kgpv, NetECPM: float64(netECPM), GrossECPM: models.GetGrossEcpm(price), diff --git a/modules/pubmatic/openwrap/utils/bid.go b/modules/pubmatic/openwrap/utils/bid.go new file mode 100644 index 00000000000..166976179f3 --- /dev/null +++ b/modules/pubmatic/openwrap/utils/bid.go @@ -0,0 +1,17 @@ +package utils + +import ( + "regexp" + + "github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" +) + +var bidIDRegx = regexp.MustCompile("(" + models.BidIdSeparator + ")") + +func GetOriginalBidId(bidID string) string { + return bidIDRegx.Split(bidID, -1)[0] +} + +func SetUniqueBidID(originalBidID, generatedBidID string) string { + return originalBidID + models.BidIdSeparator + generatedBidID +} diff --git a/modules/pubmatic/openwrap/utils/bid_test.go b/modules/pubmatic/openwrap/utils/bid_test.go new file mode 100644 index 00000000000..d115fe1d1be --- /dev/null +++ b/modules/pubmatic/openwrap/utils/bid_test.go @@ -0,0 +1,125 @@ +package utils + +import ( + "testing" +) + +func TestGetOriginalBidId(t *testing.T) { + type args struct { + bidId string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "Split the bid Id when get valid bidId", + args: args{ + bidId: "original-id::gen-id", + }, + want: "original-id", + }, + { + name: "Empty BidId", + args: args{ + bidId: "", + }, + want: "", + }, + { + name: "Partial BidId", + args: args{ + bidId: "::gen-id", + }, + want: "", + }, + { + name: "Partial BidId without generated and separator", + args: args{ + bidId: "original-bid-1", + }, + want: "original-bid-1", + }, + { + name: "Partial BidId without generated", + args: args{ + bidId: "original-bid::", + }, + want: "original-bid", + }, + { + name: "BidId with single colon in origin Id", + args: args{ + bidId: "original-bid:2::generated-bid", + }, + want: "original-bid:2", + }, + { + name: "BidId with single colon in generated Id", + args: args{ + bidId: "original-bid:2::generated-bid:3", + }, + want: "original-bid:2", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetOriginalBidId(tt.args.bidId); got != tt.want { + t.Errorf("GetOriginalBidId() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSetUniqueBidID(t *testing.T) { + type args struct { + originalBidID string + generatedBidID string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "Unique bid id will be generated", + args: args{ + originalBidID: "orig-bid", + generatedBidID: "gen-bid", + }, + want: "orig-bid::gen-bid", + }, + { + name: "Original Bid Id empty", + args: args{ + originalBidID: "", + generatedBidID: "gen-bid", + }, + want: "::gen-bid", + }, + { + name: "generated BidId empty", + args: args{ + originalBidID: "orig-bid", + generatedBidID: "", + }, + want: "orig-bid::", + }, + { + name: "Both Id empty", + args: args{ + originalBidID: "", + generatedBidID: "", + }, + want: "::", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := SetUniqueBidID(tt.args.originalBidID, tt.args.generatedBidID); got != tt.want { + t.Errorf("SetUniqueBidId() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/modules/pubmatic/vastunwrap/hook_raw_bidder_response.go b/modules/pubmatic/vastunwrap/hook_raw_bidder_response.go index a87f7bf2960..a51a85d7b98 100644 --- a/modules/pubmatic/vastunwrap/hook_raw_bidder_response.go +++ b/modules/pubmatic/vastunwrap/hook_raw_bidder_response.go @@ -9,8 +9,7 @@ import ( "github.com/prebid/prebid-server/hooks/hookstage" ) -func handleRawBidderResponseHook( - m VastUnwrapModule, +func (m VastUnwrapModule) handleRawBidderResponseHook( miCtx hookstage.ModuleInvocationContext, payload hookstage.RawBidderResponsePayload, unwrapURL string, @@ -33,7 +32,7 @@ func handleRawBidderResponseHook( wg.Add(1) go func(bid *adapters.TypedBid) { defer wg.Done() - doUnwrapandUpdateBid(m, bid, vastRequestContext.UA, unwrapURL, miCtx.AccountID, payload.Bidder) + m.doUnwrapandUpdateBid(bid, vastRequestContext.UA, unwrapURL, miCtx.AccountID, payload.Bidder) }(bid) } } diff --git a/modules/pubmatic/vastunwrap/hook_raw_bidder_response_test.go b/modules/pubmatic/vastunwrap/hook_raw_bidder_response_test.go index 98f0a32bfeb..933c1123ec6 100644 --- a/modules/pubmatic/vastunwrap/hook_raw_bidder_response_test.go +++ b/modules/pubmatic/vastunwrap/hook_raw_bidder_response_test.go @@ -2,6 +2,7 @@ package vastunwrap import ( "fmt" + "net/http" "testing" @@ -20,25 +21,28 @@ func TestHandleRawBidderResponseHook(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() mockMetricsEngine := mock_stats.NewMockMetricsEngine(ctrl) + VastUnWrapModule := VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1500}}, MetricsEngine: mockMetricsEngine} type args struct { module VastUnwrapModule payload hookstage.RawBidderResponsePayload moduleInvocationCtx hookstage.ModuleInvocationContext unwrapTimeout int url string + wantAdM bool } tests := []struct { - name string - args args - wantResult hookstage.HookResult[hookstage.RawBidderResponsePayload] - expectedBids []*adapters.TypedBid - setup func() - wantErr bool + name string + args args + wantResult hookstage.HookResult[hookstage.RawBidderResponsePayload] + expectedBids []*adapters.TypedBid + setup func() + wantErr bool + unwrapRequest func(w http.ResponseWriter, req *http.Request) }{ { name: "Empty Request Context", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnWrapModule, }, wantResult: hookstage.HookResult[hookstage.RawBidderResponsePayload]{DebugMessages: []string{"error: request-ctx not found in handleRawBidderResponseHook()"}}, wantErr: false, @@ -46,7 +50,7 @@ func TestHandleRawBidderResponseHook(t *testing.T) { { name: "Set Vast Unwrapper to false in request context with type video", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnWrapModule, payload: hookstage.RawBidderResponsePayload{ Bids: []*adapters.TypedBid{ { @@ -64,24 +68,12 @@ func TestHandleRawBidderResponseHook(t *testing.T) { moduleInvocationCtx: hookstage.ModuleInvocationContext{ModuleContext: hookstage.ModuleContext{"rctx": models.RequestCtx{VastUnwrapEnabled: false}}}, }, wantResult: hookstage.HookResult[hookstage.RawBidderResponsePayload]{Reject: false}, - expectedBids: []*adapters.TypedBid{{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: "
This is an Ad
", - CrID: "Cr-234", - W: 100, - H: 50, - }, - BidType: "video", - }}, - wantErr: false, + wantErr: false, }, { name: "Set Vast Unwrapper to true in request context with invalid vast xml", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnWrapModule, payload: hookstage.RawBidderResponsePayload{ Bids: []*adapters.TypedBid{ { @@ -103,28 +95,21 @@ func TestHandleRawBidderResponseHook(t *testing.T) { url: UnwrapURL, }, wantResult: hookstage.HookResult[hookstage.RawBidderResponsePayload]{Reject: false}, - expectedBids: []*adapters.TypedBid{{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: invalidVastXMLAdM, - CrID: "Cr-234", - W: 100, - H: 50, - }, - BidType: "video", - }}, setup: func() { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "1").AnyTimes() mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()).AnyTimes() }, + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "1") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(invalidVastXMLAdM)) + }, wantErr: true, }, { name: "Set Vast Unwrapper to true in request context with type video", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnWrapModule, payload: hookstage.RawBidderResponsePayload{ Bids: []*adapters.TypedBid{ { @@ -146,29 +131,23 @@ func TestHandleRawBidderResponseHook(t *testing.T) { url: UnwrapURL, }, wantResult: hookstage.HookResult[hookstage.RawBidderResponsePayload]{Reject: false}, - expectedBids: []*adapters.TypedBid{{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: inlineXMLAdM, - CrID: "Cr-234", - W: 100, - H: 50, - }, - BidType: "video", - }}, setup: func() { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "0").AnyTimes() mockMetricsEngine.EXPECT().RecordWrapperCount("pubmatic", "1").AnyTimes() mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()).AnyTimes() }, + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "0") + w.Header().Add("unwrap-count", "1") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(inlineXMLAdM)) + }, wantErr: false, }, { name: "Set Vast Unwrapper to true in request context for multiple bids with type video", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnWrapModule, payload: hookstage.RawBidderResponsePayload{ Bids: []*adapters.TypedBid{ { @@ -199,44 +178,26 @@ func TestHandleRawBidderResponseHook(t *testing.T) { }, moduleInvocationCtx: hookstage.ModuleInvocationContext{AccountID: "5890", ModuleContext: hookstage.ModuleContext{"rctx": models.RequestCtx{VastUnwrapEnabled: true}}}, url: UnwrapURL, + wantAdM: true, }, wantResult: hookstage.HookResult[hookstage.RawBidderResponsePayload]{Reject: false}, - expectedBids: []*adapters.TypedBid{{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: inlineXMLAdM, - CrID: "Cr-234", - W: 100, - H: 50, - }, - BidType: "video", - }, - { - Bid: &openrtb2.Bid{ - ID: "Bid-456", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: inlineXMLAdM, - CrID: "Cr-789", - W: 100, - H: 50, - }, - BidType: "video", - }, - }, setup: func() { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "0").AnyTimes() mockMetricsEngine.EXPECT().RecordWrapperCount("pubmatic", "1").AnyTimes() mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()).AnyTimes() }, + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "0") + w.Header().Add("unwrap-count", "1") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(inlineXMLAdM)) + }, wantErr: false, }, { name: "Set Vast Unwrapper to true in request context for multiple bids with different type", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnWrapModule, payload: hookstage.RawBidderResponsePayload{ Bids: []*adapters.TypedBid{ { @@ -299,6 +260,12 @@ func TestHandleRawBidderResponseHook(t *testing.T) { mockMetricsEngine.EXPECT().RecordWrapperCount("pubmatic", "0").AnyTimes() mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()).AnyTimes() }, + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "0") + w.Header().Add("unwrap-count", "0") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(inlineXMLAdM)) + }, wantErr: false, }, } @@ -307,12 +274,18 @@ func TestHandleRawBidderResponseHook(t *testing.T) { if tt.setup != nil { tt.setup() } - _, err := handleRawBidderResponseHook(tt.args.module, tt.args.moduleInvocationCtx, tt.args.payload, "test") + m := VastUnwrapModule{ + Cfg: tt.args.module.Cfg, + Enabled: true, + MetricsEngine: mockMetricsEngine, + unwrapRequest: tt.unwrapRequest, + } + _, err := m.handleRawBidderResponseHook(tt.args.moduleInvocationCtx, tt.args.payload, "test") if !assert.NoError(t, err, tt.wantErr) { return } - if tt.args.moduleInvocationCtx.ModuleContext != nil { - assert.Equal(t, tt.expectedBids[0].Bid.AdM, tt.args.payload.Bids[0].Bid.AdM, "AdM is not updated correctly after executing RawBidderResponse hook.") + if tt.args.moduleInvocationCtx.ModuleContext != nil && tt.args.wantAdM { + assert.Equal(t, inlineXMLAdM, tt.args.payload.Bids[0].Bid.AdM, "AdM is not updated correctly after executing RawBidderResponse hook.") } }) } diff --git a/modules/pubmatic/vastunwrap/module.go b/modules/pubmatic/vastunwrap/module.go index 25c5335643b..aca37238997 100644 --- a/modules/pubmatic/vastunwrap/module.go +++ b/modules/pubmatic/vastunwrap/module.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/http" "time" vastunwrap "git.pubmatic.com/vastunwrap" @@ -20,6 +21,7 @@ type VastUnwrapModule struct { TrafficPercentage int `mapstructure:"traffic_percentage" json:"traffic_percentage"` Enabled bool `mapstructure:"enabled" json:"enabled"` MetricsEngine metrics.MetricsEngine + unwrapRequest func(w http.ResponseWriter, r *http.Request) } func Builder(rawCfg json.RawMessage, deps moduledeps.ModuleDeps) (interface{}, error) { @@ -44,6 +46,7 @@ func initVastUnwrap(rawCfg json.RawMessage, deps moduledeps.ModuleDeps) (VastUnw TrafficPercentage: vastUnwrapModuleCfg.TrafficPercentage, Enabled: vastUnwrapModuleCfg.Enabled, MetricsEngine: metricEngine, + unwrapRequest: vastunwrap.UnwrapRequest, }, nil } @@ -54,7 +57,7 @@ func (m VastUnwrapModule) HandleRawBidderResponseHook( payload hookstage.RawBidderResponsePayload, ) (hookstage.HookResult[hookstage.RawBidderResponsePayload], error) { if m.Enabled { - return handleRawBidderResponseHook(m, miCtx, payload, UnwrapURL) + return m.handleRawBidderResponseHook(miCtx, payload, UnwrapURL) } return hookstage.HookResult[hookstage.RawBidderResponsePayload]{}, nil } diff --git a/modules/pubmatic/vastunwrap/module_test.go b/modules/pubmatic/vastunwrap/module_test.go index a94d1c8dc07..749cf4c4aaf 100644 --- a/modules/pubmatic/vastunwrap/module_test.go +++ b/modules/pubmatic/vastunwrap/module_test.go @@ -48,7 +48,7 @@ func TestVastUnwrapModuleHandleEntrypointHook(t *testing.T) { fields: fields{cfg: VastUnwrapModule{Enabled: true, Cfg: unWrapCfg.VastUnWrapCfg{ HTTPConfig: unWrapCfg.HttpConfig{MaxIdleConns: 100, MaxIdleConnsPerHost: 1, IdleConnTimeout: 300}, APPConfig: unWrapCfg.AppConfig{Host: "", Port: 0, UnwrapDefaultTimeout: 100, Debug: 1}, - StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, + StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, ServerConfig: unWrapCfg.ServerConfig{ServerName: "", DCName: "OW_DC"}, }, TrafficPercentage: 2}}, @@ -69,7 +69,7 @@ func TestVastUnwrapModuleHandleEntrypointHook(t *testing.T) { cfg: VastUnwrapModule{Enabled: false, Cfg: unWrapCfg.VastUnWrapCfg{ HTTPConfig: unWrapCfg.HttpConfig{MaxIdleConns: 100, MaxIdleConnsPerHost: 1, IdleConnTimeout: 300}, APPConfig: unWrapCfg.AppConfig{Host: "", Port: 0, UnwrapDefaultTimeout: 100, Debug: 1}, - StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, + StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, ServerConfig: unWrapCfg.ServerConfig{ServerName: "", DCName: "OW_DC"}, }, TrafficPercentage: 2}}, @@ -119,15 +119,16 @@ func TestVastUnwrapModuleHandleRawBidderResponseHook(t *testing.T) { in0 context.Context miCtx hookstage.ModuleInvocationContext payload hookstage.RawBidderResponsePayload + wantAdM bool } tests := []struct { - name string - fields fields - args args - expectedBids []*adapters.TypedBid - want hookstage.HookResult[hookstage.RawBidderResponsePayload] - wantErr bool - setup func() + name string + fields fields + args args + want hookstage.HookResult[hookstage.RawBidderResponsePayload] + wantErr bool + setup func() + unwrapRequest func(w http.ResponseWriter, req *http.Request) }{ { name: "Vast unwrap is enabled in the config", @@ -135,7 +136,7 @@ func TestVastUnwrapModuleHandleRawBidderResponseHook(t *testing.T) { MaxWrapperSupport: 5, HTTPConfig: unWrapCfg.HttpConfig{MaxIdleConns: 100, MaxIdleConnsPerHost: 1, IdleConnTimeout: 300}, APPConfig: unWrapCfg.AppConfig{Host: "", Port: 0, UnwrapDefaultTimeout: 1000, Debug: 1}, - StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, + StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, ServerConfig: unWrapCfg.ServerConfig{ServerName: "", DCName: "OW_DC"}, }, TrafficPercentage: 2}}, @@ -156,24 +157,21 @@ func TestVastUnwrapModuleHandleRawBidderResponseHook(t *testing.T) { BidType: "video", }}, Bidder: "pubmatic", - }}, + }, + wantAdM: true, + }, setup: func() { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "0") mockMetricsEngine.EXPECT().RecordWrapperCount("pubmatic", "1") mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()) }, - expectedBids: []*adapters.TypedBid{{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: inlineXMLAdM, - CrID: "Cr-234", - W: 100, - H: 50, - }, - BidType: "video", - }}, + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "0") + w.Header().Add("unwrap-count", "1") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(inlineXMLAdM)) + }, + want: hookstage.HookResult[hookstage.RawBidderResponsePayload]{}, }, { @@ -181,7 +179,7 @@ func TestVastUnwrapModuleHandleRawBidderResponseHook(t *testing.T) { fields: fields{cfg: VastUnwrapModule{Enabled: false, Cfg: unWrapCfg.VastUnWrapCfg{ HTTPConfig: unWrapCfg.HttpConfig{MaxIdleConns: 100, MaxIdleConnsPerHost: 1, IdleConnTimeout: 300}, APPConfig: unWrapCfg.AppConfig{Host: "", Port: 0, UnwrapDefaultTimeout: 100, Debug: 1}, - StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, + StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, ServerConfig: unWrapCfg.ServerConfig{ServerName: "", DCName: "OW_DC"}, }, TrafficPercentage: 2}}, @@ -203,18 +201,6 @@ func TestVastUnwrapModuleHandleRawBidderResponseHook(t *testing.T) { }}, }}, want: hookstage.HookResult[hookstage.RawBidderResponsePayload]{}, - expectedBids: []*adapters.TypedBid{{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - AdM: "
This is an Ad
", - CrID: "Cr-234", - W: 100, - H: 50, - }, - BidType: "video", - }}, }, } for _, tt := range tests { @@ -226,12 +212,15 @@ func TestVastUnwrapModuleHandleRawBidderResponseHook(t *testing.T) { Cfg: tt.fields.cfg.Cfg, Enabled: tt.fields.cfg.Enabled, MetricsEngine: mockMetricsEngine, + unwrapRequest: tt.unwrapRequest, } _, err := m.HandleRawBidderResponseHook(tt.args.in0, tt.args.miCtx, tt.args.payload) if !assert.NoError(t, err, tt.wantErr) { return } - assert.Equal(t, tt.expectedBids[0].Bid.AdM, tt.args.payload.Bids[0].Bid.AdM, "got, tt.want AdM is not updatd correctly after executing RawBidderResponse hook.") + if tt.args.wantAdM { + assert.Equal(t, inlineXMLAdM, tt.args.payload.Bids[0].Bid.AdM, "got, tt.want AdM is not updatd correctly after executing RawBidderResponse hook.") + } }) } } @@ -271,7 +260,7 @@ func TestBuilder(t *testing.T) { MaxWrapperSupport: 5, HTTPConfig: unWrapCfg.HttpConfig{MaxIdleConns: 100, MaxIdleConnsPerHost: 1, IdleConnTimeout: 300}, APPConfig: unWrapCfg.AppConfig{Host: "", Port: 0, UnwrapDefaultTimeout: 100, Debug: 1}, - StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, + StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, ServerConfig: unWrapCfg.ServerConfig{ServerName: "", DCName: "OW_DC"}, }, }, @@ -300,7 +289,7 @@ func TestBuilder(t *testing.T) { MaxWrapperSupport: 5, HTTPConfig: unWrapCfg.HttpConfig{MaxIdleConns: 100, MaxIdleConnsPerHost: 1, IdleConnTimeout: 300}, APPConfig: unWrapCfg.AppConfig{Host: "", Port: 0, UnwrapDefaultTimeout: 100, Debug: 1}, - StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, + StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, ServerConfig: unWrapCfg.ServerConfig{ServerName: "", DCName: "OW_DC"}, }, }, diff --git a/modules/pubmatic/vastunwrap/unwrap_service.go b/modules/pubmatic/vastunwrap/unwrap_service.go index 51168ba09a2..186a5100ce6 100644 --- a/modules/pubmatic/vastunwrap/unwrap_service.go +++ b/modules/pubmatic/vastunwrap/unwrap_service.go @@ -7,12 +7,11 @@ import ( "strings" "time" - unwrapper "git.pubmatic.com/vastunwrap" "github.com/golang/glog" "github.com/prebid/prebid-server/adapters" ) -func doUnwrapandUpdateBid(m VastUnwrapModule, bid *adapters.TypedBid, userAgent string, unwrapURL string, accountID string, bidder string) { +func (m VastUnwrapModule) doUnwrapandUpdateBid(bid *adapters.TypedBid, userAgent string, unwrapURL string, accountID string, bidder string) { startTime := time.Now() var wrapperCnt int64 var respStatus string @@ -40,7 +39,7 @@ func doUnwrapandUpdateBid(m VastUnwrapModule, bid *adapters.TypedBid, userAgent } httpReq.Header = headers httpResp := NewCustomRecorder() - unwrapper.UnwrapRequest(httpResp, httpReq) + m.unwrapRequest(httpResp, httpReq) respStatus = httpResp.Header().Get(UnwrapStatus) wrapperCnt, _ = strconv.ParseInt(httpResp.Header().Get(UnwrapCount), 10, 0) respBody := httpResp.Body.Bytes() diff --git a/modules/pubmatic/vastunwrap/unwrap_service_test.go b/modules/pubmatic/vastunwrap/unwrap_service_test.go index 0d8710a955c..453861b3700 100644 --- a/modules/pubmatic/vastunwrap/unwrap_service_test.go +++ b/modules/pubmatic/vastunwrap/unwrap_service_test.go @@ -2,6 +2,7 @@ package vastunwrap import ( "fmt" + "net/http" "testing" "git.pubmatic.com/vastunwrap/config" @@ -24,17 +25,18 @@ func TestDoUnwrap(t *testing.T) { userAgent string unwrapDefaultTimeout int url string + wantAdM bool } tests := []struct { - name string - args args - expectedBid *adapters.TypedBid - setup func() + name string + args args + setup func() + unwrapRequest func(w http.ResponseWriter, r *http.Request) }{ { name: "doUnwrap for adtype video with Empty Bid", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, bid: &adapters.TypedBid{ Bid: &openrtb2.Bid{}, }, @@ -45,7 +47,7 @@ func TestDoUnwrap(t *testing.T) { { name: "doUnwrap for adtype video with Empty ADM", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, bid: &adapters.TypedBid{ Bid: &openrtb2.Bid{ ID: "Bid-123", @@ -60,23 +62,11 @@ func TestDoUnwrap(t *testing.T) { userAgent: "testUA", url: UnwrapURL, }, - expectedBid: &adapters.TypedBid{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - CrID: "Cr-234", - AdM: "", - W: 100, - H: 50, - }, - BidType: "video", - }, }, { name: "doUnwrap for adtype video with invalid URL and timeout", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 100}}, MetricsEngine: mockMetricsEngine}, + module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 2}}, MetricsEngine: mockMetricsEngine}, bid: &adapters.TypedBid{ Bid: &openrtb2.Bid{ ID: "Bid-123", @@ -96,23 +86,16 @@ func TestDoUnwrap(t *testing.T) { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "2") mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()) }, - expectedBid: &adapters.TypedBid{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - CrID: "Cr-234", - AdM: vastXMLAdM, - W: 100, - H: 50, - }, - BidType: "video", + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "2") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(vastXMLAdM)) }, }, { name: "doUnwrap for adtype video", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1500}}, MetricsEngine: mockMetricsEngine}, bid: &adapters.TypedBid{ Bid: &openrtb2.Bid{ ID: "Bid-123", @@ -127,29 +110,24 @@ func TestDoUnwrap(t *testing.T) { }, userAgent: "testUA", url: UnwrapURL, + wantAdM: true, }, setup: func() { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "0") mockMetricsEngine.EXPECT().RecordWrapperCount("pubmatic", "1") mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()) }, - expectedBid: &adapters.TypedBid{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - CrID: "Cr-234", - AdM: inlineXMLAdM, - W: 100, - H: 50, - }, - BidType: "video", + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "0") + w.Header().Add("unwrap-count", "1") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(inlineXMLAdM)) }, }, { name: "doUnwrap for adtype video with invalid vast xml", args: args{ - module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Host: "10.172.141.13", Port: 8080, RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, + module: VastUnwrapModule{Cfg: config.VastUnWrapCfg{MaxWrapperSupport: 5, StatConfig: unWrapCfg.StatConfig{Endpoint: "http://10.172.141.13:8080", RefershIntervalInSec: 1}, APPConfig: config.AppConfig{UnwrapDefaultTimeout: 1000}}, MetricsEngine: mockMetricsEngine}, bid: &adapters.TypedBid{ Bid: &openrtb2.Bid{ ID: "Bid-123", @@ -164,33 +142,34 @@ func TestDoUnwrap(t *testing.T) { }, userAgent: "testUA", url: UnwrapURL, + wantAdM: false, }, setup: func() { mockMetricsEngine.EXPECT().RecordRequestStatus("pubmatic", "1") mockMetricsEngine.EXPECT().RecordRequestTime("pubmatic", gomock.Any()) }, - expectedBid: &adapters.TypedBid{ - Bid: &openrtb2.Bid{ - ID: "Bid-123", - ImpID: fmt.Sprintf("div-adunit-%d", 123), - Price: 2.1, - CrID: "Cr-234", - AdM: invalidVastXMLAdM, - W: 100, - H: 50, - }, - BidType: "video", + unwrapRequest: func(w http.ResponseWriter, req *http.Request) { + w.Header().Add("unwrap-status", "1") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(invalidVastXMLAdM)) }, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.setup != nil { tt.setup() } - doUnwrapandUpdateBid(tt.args.module, tt.args.bid, tt.args.userAgent, tt.args.url, "5890", "pubmatic") - if tt.args.bid.Bid.AdM != "" { - assert.Equal(t, tt.expectedBid.Bid.AdM, tt.args.bid.Bid.AdM, "AdM is not updated correctly after executing RawBidderResponse hook.") + m := VastUnwrapModule{ + Cfg: tt.args.module.Cfg, + Enabled: true, + MetricsEngine: mockMetricsEngine, + unwrapRequest: tt.unwrapRequest, + } + m.doUnwrapandUpdateBid(tt.args.bid, tt.args.userAgent, tt.args.url, "5890", "pubmatic") + if tt.args.bid.Bid.AdM != "" && tt.args.wantAdM { + assert.Equal(t, inlineXMLAdM, tt.args.bid.Bid.AdM, "AdM is not updated correctly after executing RawBidderResponse hook.") } }) } diff --git a/scripts/coverage.sh b/scripts/coverage.sh index 699f484e7b5..3b3e2a95dd5 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -67,6 +67,10 @@ generate_cover_data() { cover+=" -coverpkg=github.com/prebid/prebid-server/modules/pubmatic/openwrap/metrics/stats" fi + if [[ "$pkg" =~ ^github\.com\/PubMatic\-OpenWrap\/prebid\-server\/modules\/pubmatic\/openwrap\/models$ ]]; then + cover+=" -coverpkg=github.com/prebid/prebid-server/modules/pubmatic/openwrap/models" + fi + go test ${cover} "$pkg" done