Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

default request attributes are getting higher priority over attributes defined in stored requests #1603

Closed
ishihanvcs opened this issue Nov 29, 2021 · 4 comments · Fixed by #1629
Closed

default request attributes are getting higher priority over attributes defined in stored requests #1603

ishihanvcs opened this issue Nov 29, 2021 · 4 comments · Fixed by #1629
Assignees
@rpanchyk

Comments

@ishihanvcs
Copy link
Contributor

In Line 250 of StoredRequestProcessor I can see defaultBidRequest is passed as 2nd parameter of jsonMerger.merge method, which is actually replacing attributes from bidRequest with defaultBidRequest attributes. IMHO, this is not the behaviour which is documented for default-request.file.path config.

I've evidently found that with following contents of default request, the ext.prebid.targeting.pricegranularity.ranges and cur attributes are replaced with attributes from default, event though these were explicitly set in a bid request that uses server side stored request.

{
  "ext": {
    "prebid": {
      "targeting": {
        "pricegranularity": {
          "precision": 2,
          "ranges": [
            {
              "max": 20,
              "increment": 0.01
            },
            {
              "max": 100,
              "increment": 0.1
            }
          ]
        }
      }
    }
  },
  "cur": [
    "USD"
  ]
}

IMHO, the fix should be easy implemented by swapping the first & second parameters of jsonMerger.merge method call here.

Right now, this is a blocking issue for us and I'm hoping to get it fixed at your earliest.

Thanks in advance & keep up the good work as always.
@rpanchyk rpanchyk self-assigned this Nov 29, 2021
@rpanchyk
Copy link
Contributor

Hi, @ishihanvcs ! Sorry but i cannot reproduce this.
I see that default request and stored request both are overridden by original (incoming) request.
May you please pointing me with examples if any.

Here are running org.prebid.server.auction.StoredRequestProcessorTest#shouldReturnMergedDefaultAndBidRequest test with some modifications.

Original vs Default:
2021-11-30_17-25

Original vs Stored:
2021-11-30_17-35

@ishihanvcs
Copy link
Contributor Author

ishihanvcs commented Dec 6, 2021 •
edited
Loading

Hi @rpanchyk.

Thanks a lot for looking into this. After digging more with the info in your reply and our use case, I've found that the description of my issue post is actually misleading and confusing. The use case is something below:

  • We have a default request set via default-request.file.path config, that has following content:
{
  "ext": {
    "prebid": {
      "targeting": {
        "pricegranularity": {
          "precision": 2,
          "ranges": [
            {
              "max": 20,
              "increment": 0.01
            },
            {
              "max": 100,
              "increment": 0.1
            }
          ]
        }
      }
    }
  },
  "cur": [
    "USD"
  ]
}
  • We have a server side stored request defined as below:
{
  "ext": {
    "prebid": {
      "targeting": {
        "pricegranularity": {
          "precision": 3,
          "ranges": [
            {
              "max": 0.2,
              "increment": 0.001
            },
            {
              "max": 0.5,
              "increment": 0.005
            },
            {
            "max": 1,
            "increment": 0.01
            },
            {
              "max": 5,
              "increment": 0.05
            }
          ]
        }
      }
    }
  },
  "cur": [
    "EUR"
  ]
}
  • We are sending a bid request from client with below content:
{
  "id": "7ea1c355-fb3c-44a4-83d5-9c12f3938f8e",
  "site": {
    "domain": "example.com",
    "cat": [],
    "page": "https://example.com/pages/page-1",
    "publisher": {
      "id": "1001"
    }
  },
  "regs": {
    "ext": {
      "gdpr": 1
    }
  },
  "user": {
    "ext": {
      "consent": "CPMd8p0PMd8p0AcABBENBiCsAP_AAH_AAChQIENf_X__b3_j-_59f_t0eY1P9_7_v-0zjhfdt-8N2f_X_L8X42M7vF36pq4KuR4Eu3LBIQdlHOHcTUmw6okVrzPsbk2Mr7NKJ7PEmnMbO2dYGH9_n93TuZKY7__8___z__-v_v____f_r-3_3__59X---_e_V399zLv9_____9nN__wQEAJMNS-AC7EscGTaNKoUQIwrCQ6AUAFFAMLRNYQMrgp2VwEeoIWACE1ARgRAgxBRiwCAAQCAJCIgJADwQCIAiAQAAgBUgIQAEbAILACwMAgAFANCxAigCECQgyOCo5TAgIkWignsrAEou9jTCEMosAKBR_RUYCJQggWBkJCwcxwBIAAA.f_gAD_gAAAAA"
    }
  },
  "source": {
    "tid": "9a71ffeb-319a-4a6e-95bc-01ce82567964",
    "ext": {
      "schain": {
        "ver": "1.0",
        "complete": 1,
        "nodes": [
          {
            "asi": "headerlift.com",
            "hp": 1,
            "sid": "1234abc"
          }
        ]
      }
    }
  },
  "tmax": 100000,
  "test": 1,
  "device": {
    "w": 1280,
    "h": 600
  },
  "imp": [
    {
      "id": "banner",
      "ext": {
        "improvedigital": {
          "placementId": 1094513
        }
      },
      "banner": {
        "format": [
          {
            "w": 970,
            "h": 250
          }
        ]
      }
    }
  ],
  "ext": {
    "prebid": {
      "auctiontimestamp": 1638796676,
      "targeting": {
        "includewinners": true,
        "includeformat": true
      },
      "storedrequest": {
        "id": "banner"
      }
    }
  }
}
  • The response we receive from PBS has below content:
{
  "id": "7ea1c355-fb3c-44a4-83d5-9c12f3938f8e",
  "seatbid": [
    {
      "bid": [
        {
          "id": "4ecac4d9-f97e-43b8-954c-b5f99a8f3fa0",
          "impid": "banner",
          "price": 1.14240470535007,
          "adm": "<html><body style=\"margin:0%\"><a href=\"http://euw-ice.360yield.com/click/Q6iuNUSqHO7Hefw.COyoEQc-cjO5y-rTry67hcDqeLDaeTbul7elThTHxFtP2ITaINp4P1um.rpbSwSLhMFj8AgeQVr00WX8aFgiiOUTINH1PjEDqOOQ8PRf7sSi7HCqkT-TagvIloNb-MgXLwVu48yh3qFgAcnlG3e2rGo0rrVqGNr2igVPO1oK7Rb1osNjtm6CkS-AraLcTgpyaE6BG7u7kQXCLoJ.Gfii0smsxJkyWNRfC9Dy1EvJYuWfLDoa4GxPauSgC62-HMTUcDkbC2pKl5j9v9zpj2TDbxmnN5m3RB-Bmkk-EetEjo-NbAHOUnru3e5.-NcSq7qvg91OSMhYCKb.hcDrnjl6DPFeY44hW7zP0FcAIZBp3VDddqnxhTJJ9c45gV7lyxgYm80pYbxcFmxgSaeosvBwYYhapgEP5HMFIioImhnPtwCiv0r2E6rcXW9oiWOobx.NaChM1ZtXok9OxcHrxj.YL.5N3NaXXcG1gyO2cbhn7w0=//http%3A%2F%2Fwww.improvedigital.com\" target=\"_blank\"><img style=\"border: 0;\" border=\"0\" width=970 height=250 src=\"http://creative.360yield.com/file/226316/adinadimage.jpg\" alt=\" \"/></a><img src=\"http://euw-ice.360yield.com/imp_pixel?ic=Q6iuNY.6STMW.UrnTGmEN4Ulopl-qdIclztWjOj6PXhpSisJuNYUsq53ovfuHezA0suThyF0Uh.Lk2yNVy2yAoqn3VpOLQMtgQlfGngwYZouPlNOM9zW4cMGXCr.q0e0li6YUlB5S0RNFxtkGWkjwi.p5xTQ.cCWsX7l3ZsN4q-dWuR4RZgo2T73yTDkG8OFrd1NwZPcZ9fv8Vcf7ZTcr5VmDV0nLj3TUwqs0sqFDAQr7GbFMjMChMtQSHeRt.DczcX-O4YSSqc1RVAwLAL4XSvlubo-Zc2MWbR3je5rswYmHN35XKjkRMM7B2L8.3Fd5FP06wx73ExmKZ5qwHxXNRpEURYAIgRqZJTcEgpHktjZqkDPft4yhXTd-NsR.LagZrlbF0yjZC0qu8vEGPf22QfkaU2NHp957aZcXRgNZRu87ao93QpmP0xCrwXmoA4RSeF81DmaJp03SLfpf1JU00mLEcAHrbartm2miQmH96jkWaE3xx4I1cHL-5I=\" alt=\" \" style=\"display:none\"/><improvedigital_ad_output_information tp_id=\"\" buyer_id=\"0\" rtb_advertiser=\"\" campaign_id=\"103998\" line_item_id=\"279820\" creative_id=\"431739\" crid=\"0\" placement_id=\"1094513\"></improvedigital_ad_output_information></body></html>",
          "cid": "103998",
          "crid": "431739",
          "dealid": "279820",
          "w": 970,
          "h": 250,
          "ext": {
            "improvedigital": {
              "line_item_id": 279820,
              "bidder_id": 0,
              "brand_name": "",
              "buying_type": "classic",
              "agency_id": "0"
            },
            "origbidcpm": 1.14240470535007,
            "origbidcur": "USD",
            "prebid": {
              "type": "banner",
              "targeting": {
                "hb_pb": "1.14",
                "hb_size": "970x250",
                "hb_deal_improvedigit": "279820",
                "hb_bidder": "improvedigital",
                "hb_format_improvedig": "banner",
                "hb_size_improvedigit": "970x250",
                "hb_format": "banner",
                "hb_pb_improvedigital": "1.14",
                "hb_deal": "279820",
                "hb_bidder_improvedig": "improvedigital"
              }
            }
          }
        }
      ],
      "seat": "improvedigital",
      "group": 0
    }
  ],
  "cur": "USD",
  "ext": {
    "debug": {
      "httpcalls": {
        "improvedigital": [
          {
            "uri": "http://ad.360yield.com/pbs",
            "requestbody": "{\"id\":\"7ea1c355-fb3c-44a4-83d5-9c12f3938f8e\",\"imp\":[{\"id\":\"banner\",\"banner\":{\"format\":[{\"w\":970,\"h\":250}]},\"ext\":{\"bidder\":{\"placementId\":1094513}}}],\"site\":{\"domain\":\"example.com\",\"cat\":[],\"page\":\"https://example.com/pages/page-1\",\"publisher\":{\"id\":\"1001\",\"domain\":\"example.com\"},\"ext\":{\"amp\":0}},\"device\":{\"ua\":\"insomnia/2021.6.0\",\"ip\":\"45.118.61.6\",\"h\":600,\"w\":1280},\"user\":{\"ext\":{\"consent\":\"CPMd8p0PMd8p0AcABBENBiCsAP_AAH_AAChQIENf_X__b3_j-_59f_t0eY1P9_7_v-0zjhfdt-8N2f_X_L8X42M7vF36pq4KuR4Eu3LBIQdlHOHcTUmw6okVrzPsbk2Mr7NKJ7PEmnMbO2dYGH9_n93TuZKY7__8___z__-v_v____f_r-3_3__59X---_e_V399zLv9_____9nN__wQEAJMNS-AC7EscGTaNKoUQIwrCQ6AUAFFAMLRNYQMrgp2VwEeoIWACE1ARgRAgxBRiwCAAQCAJCIgJADwQCIAiAQAAgBUgIQAEbAILACwMAgAFANCxAigCECQgyOCo5TAgIkWignsrAEou9jTCEMosAKBR_RUYCJQggWBkJCwcxwBIAAA.f_gAD_gAAAAA\"}},\"test\":1,\"at\":1,\"tmax\":5000,\"cur\":[\"USD\"],\"source\":{\"tid\":\"9a71ffeb-319a-4a6e-95bc-01ce82567964\",\"ext\":{\"schain\":{\"ver\":\"1.0\",\"complete\":1,\"nodes\":[{\"asi\":\"headerlift.com\",\"sid\":\"1234abc\",\"hp\":1}]}}},\"regs\":{\"ext\":{\"gdpr\":1}},\"ext\":{\"prebid\":{\"targeting\":{\"pricegranularity\":{\"precision\":2,\"ranges\":[{\"max\":20,\"increment\":0.01},{\"max\":100,\"increment\":0.1}]},\"includewinners\":true,\"includebidderkeys\":true,\"includeformat\":true},\"storedrequest\":{\"id\":\"banner\"},\"auctiontimestamp\":1638796676,\"channel\":{\"name\":\"web\"},\"pbs\":{\"endpoint\":\"/openrtb2/auction\"}}}}",
            "responsebody": "{\"id\":\"7ea1c355-fb3c-44a4-83d5-9c12f3938f8e\",\"cur\":\"USD\",\"seatbid\":[{\"bid\":[{\"ext\":{\"improvedigital\":{\"line_item_id\":279820,\"bidder_id\":0,\"brand_name\":\"\",\"buying_type\":\"classic\",\"agency_id\":\"0\"}},\"exp\":120,\"crid\":\"431739\",\"price\":1.14240470535007,\"id\":\"4ecac4d9-f97e-43b8-954c-b5f99a8f3fa0\",\"w\":970,\"impid\":\"banner\",\"h\":250,\"adm\":\"<html><body style=\\\"margin:0%\\\"><a href=\\\"http://euw-ice.360yield.com/click/Q6iuNUSqHO7Hefw.COyoEQc-cjO5y-rTry67hcDqeLDaeTbul7elThTHxFtP2ITaINp4P1um.rpbSwSLhMFj8AgeQVr00WX8aFgiiOUTINH1PjEDqOOQ8PRf7sSi7HCqkT-TagvIloNb-MgXLwVu48yh3qFgAcnlG3e2rGo0rrVqGNr2igVPO1oK7Rb1osNjtm6CkS-AraLcTgpyaE6BG7u7kQXCLoJ.Gfii0smsxJkyWNRfC9Dy1EvJYuWfLDoa4GxPauSgC62-HMTUcDkbC2pKl5j9v9zpj2TDbxmnN5m3RB-Bmkk-EetEjo-NbAHOUnru3e5.-NcSq7qvg91OSMhYCKb.hcDrnjl6DPFeY44hW7zP0FcAIZBp3VDddqnxhTJJ9c45gV7lyxgYm80pYbxcFmxgSaeosvBwYYhapgEP5HMFIioImhnPtwCiv0r2E6rcXW9oiWOobx.NaChM1ZtXok9OxcHrxj.YL.5N3NaXXcG1gyO2cbhn7w0=//http%3A%2F%2Fwww.improvedigital.com\\\" target=\\\"_blank\\\"><img style=\\\"border: 0;\\\" border=\\\"0\\\" width=970 height=250 src=\\\"http://creative.360yield.com/file/226316/adinadimage.jpg\\\" alt=\\\" \\\"/></a><img src=\\\"http://euw-ice.360yield.com/imp_pixel?ic=Q6iuNY.6STMW.UrnTGmEN4Ulopl-qdIclztWjOj6PXhpSisJuNYUsq53ovfuHezA0suThyF0Uh.Lk2yNVy2yAoqn3VpOLQMtgQlfGngwYZouPlNOM9zW4cMGXCr.q0e0li6YUlB5S0RNFxtkGWkjwi.p5xTQ.cCWsX7l3ZsN4q-dWuR4RZgo2T73yTDkG8OFrd1NwZPcZ9fv8Vcf7ZTcr5VmDV0nLj3TUwqs0sqFDAQr7GbFMjMChMtQSHeRt.DczcX-O4YSSqc1RVAwLAL4XSvlubo-Zc2MWbR3je5rswYmHN35XKjkRMM7B2L8.3Fd5FP06wx73ExmKZ5qwHxXNRpEURYAIgRqZJTcEgpHktjZqkDPft4yhXTd-NsR.LagZrlbF0yjZC0qu8vEGPf22QfkaU2NHp957aZcXRgNZRu87ao93QpmP0xCrwXmoA4RSeF81DmaJp03SLfpf1JU00mLEcAHrbartm2miQmH96jkWaE3xx4I1cHL-5I=\\\" alt=\\\" \\\" style=\\\"display:none\\\"/><improvedigital_ad_output_information tp_id=\\\"\\\" buyer_id=\\\"0\\\" rtb_advertiser=\\\"\\\" campaign_id=\\\"103998\\\" line_item_id=\\\"279820\\\" creative_id=\\\"431739\\\" crid=\\\"0\\\" placement_id=\\\"1094513\\\"></improvedigital_ad_output_information></body></html>\",\"cid\":\"103998\"}],\"seat\":\"improvedigital\"}]}",
            "requestheaders": {
              "Accept": [
                "application/json"
              ],
              "x-prebid": [
                "pbs-java/1.79.0"
              ],
              "Content-Type": [
                "application/json;charset=utf-8"
              ]
            },
            "status": 200
          }
        ]
      },
      "resolvedrequest": {
        "id": "7ea1c355-fb3c-44a4-83d5-9c12f3938f8e",
        "imp": [
          {
            "id": "banner",
            "banner": {
              "format": [
                {
                  "w": 970,
                  "h": 250
                }
              ]
            },
            "ext": {
              "prebid": {
                "bidder": {
                  "improvedigital": {
                    "placementId": 1094513
                  }
                }
              }
            }
          }
        ],
        "site": {
          "domain": "example.com",
          "cat": [],
          "page": "https://example.com/pages/page-1",
          "publisher": {
            "id": "1001",
            "domain": "example.com"
          },
          "ext": {
            "amp": 0
          }
        },
        "device": {
          "ua": "insomnia/2021.6.0",
          "ip": "45.118.61.6",
          "h": 600,
          "w": 1280
        },
        "user": {
          "ext": {
            "consent": "CPMd8p0PMd8p0AcABBENBiCsAP_AAH_AAChQIENf_X__b3_j-_59f_t0eY1P9_7_v-0zjhfdt-8N2f_X_L8X42M7vF36pq4KuR4Eu3LBIQdlHOHcTUmw6okVrzPsbk2Mr7NKJ7PEmnMbO2dYGH9_n93TuZKY7__8___z__-v_v____f_r-3_3__59X---_e_V399zLv9_____9nN__wQEAJMNS-AC7EscGTaNKoUQIwrCQ6AUAFFAMLRNYQMrgp2VwEeoIWACE1ARgRAgxBRiwCAAQCAJCIgJADwQCIAiAQAAgBUgIQAEbAILACwMAgAFANCxAigCECQgyOCo5TAgIkWignsrAEou9jTCEMosAKBR_RUYCJQggWBkJCwcxwBIAAA.f_gAD_gAAAAA"
          }
        },
        "test": 1,
        "at": 1,
        "tmax": 5000,
        "cur": [
          "USD"
        ],
        "source": {
          "tid": "9a71ffeb-319a-4a6e-95bc-01ce82567964",
          "ext": {
            "schain": {
              "ver": "1.0",
              "complete": 1,
              "nodes": [
                {
                  "asi": "headerlift.com",
                  "sid": "1234abc",
                  "hp": 1
                }
              ]
            }
          }
        },
        "regs": {
          "ext": {
            "gdpr": 1
          }
        },
        "ext": {
          "prebid": {
            "targeting": {
              "pricegranularity": {
                "precision": 2,
                "ranges": [
                  {
                    "max": 20,
                    "increment": 0.01
                  },
                  {
                    "max": 100,
                    "increment": 0.1
                  }
                ]
              },
              "includewinners": true,
              "includebidderkeys": true,
              "includeformat": true
            },
            "storedrequest": {
              "id": "banner"
            },
            "auctiontimestamp": 1638796676,
            "channel": {
              "name": "web"
            },
            "pbs": {
              "endpoint": "/openrtb2/auction"
            }
          }
        }
      }
    },
    "responsetimemillis": {
      "improvedigital": 866
    },
    "tmaxrequest": 5000,
    "prebid": {
      "auctiontimestamp": 1638796676
    }
  }
}
  • Now if we decode the content of ext.debug.httpcalls.improvedigital[0].requestbody attribute in above PBS response, we get:
{
  "id": "7ea1c355-fb3c-44a4-83d5-9c12f3938f8e",
  "imp": [
    {
      "id": "banner",
      "banner": { "format": [{ "w": 970, "h": 250 }] },
      "ext": { "bidder": { "placementId": 1094513 } }
    }
  ],
  "site": {
    "domain": "example.com",
    "cat": [],
    "page": "https://example.com/pages/page-1",
    "publisher": { "id": "1001", "domain": "example.com" },
    "ext": { "amp": 0 }
  },
  "device": {
    "ua": "insomnia/2021.6.0",
    "ip": "45.118.61.6",
    "h": 600,
    "w": 1280
  },
  "user": {
    "ext": {
      "consent": "CPMd8p0PMd8p0AcABBENBiCsAP_AAH_AAChQIENf_X__b3_j-_59f_t0eY1P9_7_v-0zjhfdt-8N2f_X_L8X42M7vF36pq4KuR4Eu3LBIQdlHOHcTUmw6okVrzPsbk2Mr7NKJ7PEmnMbO2dYGH9_n93TuZKY7__8___z__-v_v____f_r-3_3__59X---_e_V399zLv9_____9nN__wQEAJMNS-AC7EscGTaNKoUQIwrCQ6AUAFFAMLRNYQMrgp2VwEeoIWACE1ARgRAgxBRiwCAAQCAJCIgJADwQCIAiAQAAgBUgIQAEbAILACwMAgAFANCxAigCECQgyOCo5TAgIkWignsrAEou9jTCEMosAKBR_RUYCJQggWBkJCwcxwBIAAA.f_gAD_gAAAAA"
    }
  },
  "test": 1,
  "at": 1,
  "tmax": 5000,
  "cur": ["USD"],
  "source": {
    "tid": "9a71ffeb-319a-4a6e-95bc-01ce82567964",
    "ext": {
      "schain": {
        "ver": "1.0",
        "complete": 1,
        "nodes": [{ "asi": "headerlift.com", "sid": "1234abc", "hp": 1 }]
      }
    }
  },
  "regs": { "ext": { "gdpr": 1 } },
  "ext": {
    "prebid": {
      "targeting": {
        "pricegranularity": {
          "precision": 2,
          "ranges": [
            { "max": 20, "increment": 0.01 },
            { "max": 100, "increment": 0.1 }
          ]
        },
        "includewinners": true,
        "includebidderkeys": true,
        "includeformat": true
      },
      "storedrequest": { "id": "banner" },
      "auctiontimestamp": 1638796676,
      "channel": { "name": "web" },
      "pbs": { "endpoint": "/openrtb2/auction" }
    }
  }
}
  • Now if we compare this request to SSP with the content of default request and stored request (specified in actual bid request), we can clearly see that the default request is getting higher precedence over the stored request and both cur and ext.prebid.targeting.pricegranularity attributes are coming from the default request.

In our understanding, the order of precedence or priority of merging of attributes should be: bidRequest > storedRequest > defaultRequest and hence we are facing this issue.

With further digging into the code, I've found that this is the place where mergeDefaultRequest(bidRequest) is called before merging storedRequest to compute the final bidRequest.

In my understanding, to apply the semantic meaning of default, storedRequest should be merged to original bidRequest first and then the defaultRequest should be merged, to compute the final bidRequest.

Please correct me, if my understanding is wrong and we'll figure out a work around by not using defaultRequest entirely and replicate the default attributes in all of our server side stored requests.

Thanks again for putting the time to dig into my issue and please accept my sincere apology for misleading/confusing description at the first post.

Eagerly waiting for your response on this.

Update: I've updated the issue title to match the actual issue described in this comment.

@ishihanvcs ishihanvcs changed the title default stored request attributes are replacing attributes in bid request default request attributes are getting higher priority over attributes defined in stored requests Dec 7, 2021
@rpanchyk rpanchyk mentioned this issue Dec 13, 2021
Merged
@rpanchyk
Copy link
Contributor

Thank you so much for catching this @ishihanvcs !
The bug was really old since #850 implemented.

@rpanchyk
Copy link
Contributor

Seems this was closed automatically after #1629 merged. Reopen if any please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rpanchyk rpanchyk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Stored vs Default request merging prebid/prebid-server-java
2 participants
@rpanchyk @ishihanvcs