prebid-server-java icon indicating copy to clipboard operation
prebid-server-java copied to clipboard

Bug when using stored requests and stored impressions

Open ollyburns opened this issue 11 months ago • 8 comments

Hi,

I think I have found a bug in the current version of PBS, albeit an edge case. I have patched it, but I am sure the team has a more elegant solution than mine.

The problem occurs when using both stored impressions and a stored request. Then stored impressions are looked up using an object map that is populated before the stored impression is applied. Once the stored impression is merged the lookup fails, because the impression objects are now different.

The issue occurs in this function.

And this is my (far from elegant) fix:

     private BidRequest mergeImps(BidRequest bidRequest,
                                 Map<Imp, String> impToStoredId,
                                 StoredDataResult storedDataResult) {
        if (impToStoredId.isEmpty()) {
            return bidRequest;
        }
        final List<Imp> mergedImps = new ArrayList<>(bidRequest.getImp());
        for (int i = 0; i < mergedImps.size(); i++) {
            final Imp imp = mergedImps.get(i);
            final String storedRequestId = getStoredRequestId(imp);
            if (storedRequestId != null) {
                final String storedImp = storedDataResult.getStoredIdToImp().get(storedRequestId);
                final Imp mergedImp = jsonMerger.merge(imp, storedImp, storedRequestId, Imp.class);
                mergedImps.set(i, mergedImp);
            }
        }
        return bidRequest.toBuilder().imp(mergedImps).build();
    }
    private static String getStoredRequestId(Imp imp) {
        final ObjectNode extImp = imp.getExt();
        String storedRequestId = null;
        if (extImp != null) {
            final JsonNode prebid = extImp.get("prebid");
            if (prebid != null) {
                final JsonNode storedrequest = prebid.get("storedrequest");
                if (storedrequest != null) {
                    final JsonNode id = storedrequest.get("id");
                    if (id != null) {
                        storedRequestId = id.asText();
                    }
                }
            }
        }
        return storedRequestId;
    }

The specific use case where I came across this was when trying to augment the list of passed in bidders with additional bidders from a stored impression. This was working fine when a stored request was not present, but stopped working with a stored request present.

Thanks for taking a look.

Olly.

ollyburns avatar Feb 14 '25 17:02 ollyburns

@ollyburns - can you provide an example request that fails?

It is absolutely not supported to have any kind of recursion in stored requests:

  1. AMP: the body of the AMP stored request must be complete. It cannot refer to any other stored request.
  2. Mobile: ext.prebid.storedrequest cannot add or modify an imp object.

The only kind of recursion supported is that a storedrequest may contain a reference to a storedresponse.

imp: [{
    ext: {
        prebid: {
            storedrequest: { id: "sreq123" }
        }
    }
}]

sreq123 can be defined as

{
		"ext": {
				"prebid": {
						"storedbidresponse": [
								{
										"id": "sbr-300x250-imp-1",
										"bidder": "bidderA",
										"replaceimpid": true
								}
						]
				}
		}
}

Which will then recursively obtain sbr-300x250-imp-1 from the stored_response table.

But it's not allowed to have a stored request pull another stored request.

bretg avatar Mar 03 '25 16:03 bretg

Hi @bretg, thanks for looking into this. There is no recursion involved - I came across this when using a stored request and stored impression in the same payload. Here is an example - I've highlighted (apologies for clumsy highlighting) one of the stored impressions, one of the passed in bidders, and the stored request:

{
    "imp": [
        {
            "ext": {
                "prebid": {
                    "storedrequest": {
                        **"id": "td-default"**
                    },
                    "bidder": {
                        "appnexus": {
                            **"placementId": 25398888,**
                            "reserve": 0.08
                        },
                        "rubicon": {
                            "accountId": 8917,
                            "siteId": 14801,
                            "zoneId": 2434894,
                            "floor": 0.08
                        },
                        "sovrn": {
                            "tagid": "1104554",
                            "bidfloor": ".08"
                        },
                        "pubmatic": {
                            "publisherId": "163194",
                            "adSlot": "5100926",
                            "kadfloor": ".08"
                        },
                        "sharethrough": {
                            "pkey": "6qucXsb0ViTmlEGLE2LINbq4"
                        }
                    },
                    "adunitcode": "/1018099/tigerdroppings_leaderboard"
                },
                "tid": "9e6e15c8-886b-4242-89f0-3b3bf5389481",
                "data": {
                    "adserver": {
                        "name": "gam",
                        "adslot": "/1018099/tigerdroppings_leaderboard"
                    },
                    "pbadslot": "/1018099/tigerdroppings_leaderboard"
                },
                "gpid": "/1018099/tigerdroppings_leaderboard"
            },
            "id": "/1018099/tigerdroppings_leaderboard",
            "banner": {
                "topframe": 1,
                "format": [
                    {
                        "w": 970,
                        "h": 90
                    },
                    {
                        "w": 728,
                        "h": 90
                    },
                    {
                        "w": 970,
                        "h": 66
                    }
                ]
            },
            "secure": 1
        },
        {
            "ext": {
                "prebid": {
                    "storedrequest": {
                        "id": "td-default"
                    },
                    "bidder": {
                        "appnexus": {
                            "placementId": 25398889,
                            "reserve": 0.08
                        },
                        "rubicon": {
                            "accountId": 8917,
                            "siteId": 14801,
                            "zoneId": 2434896,
                            "floor": 0.08
                        },
                        "sovrn": {
                            "tagid": "1104556",
                            "bidfloor": ".08"
                        },
                        "pubmatic": {
                            "publisherId": "163194",
                            "adSlot": "5100927",
                            "kadfloor": ".08",
                            "outstreamAU": "tigerdroppings_right_rail_1"
                        },
                        "sharethrough": {
                            "pkey": "gkFaSuAg6gh553BzduGoEx4N"
                        }
                    },
                    "adunitcode": "/1018099/tigerdroppings_right_rail_1"
                },
                "tid": "3402c6bf-9780-4126-a319-9564e0b23d71",
                "data": {
                    "adserver": {
                        "name": "gam",
                        "adslot": "/1018099/tigerdroppings_right_rail_1"
                    },
                    "pbadslot": "/1018099/tigerdroppings_right_rail_1"
                },
                "gpid": "/1018099/tigerdroppings_right_rail_1"
            },
            "id": "/1018099/tigerdroppings_right_rail_1",
            "banner": {
                "topframe": 1,
                "format": [
                    {
                        "w": 336,
                        "h": 280
                    },
                    {
                        "w": 300,
                        "h": 250
                    }
                ]
            },
            "secure": 1
        },
        {
            "ext": {
                "prebid": {
                    "storedrequest": {
                        "id": "td-default"
                    },
                    "bidder": {
                        "appnexus": {
                            "placementId": 25398891,
                            "reserve": 0.1
                        },
                        "rubicon": {
                            "accountId": 8917,
                            "siteId": 14801,
                            "zoneId": 2434900,
                            "floor": 0.1
                        },
                        "sovrn": {
                            "tagid": "1104558",
                            "bidfloor": "0.10"
                        },
                        "pubmatic": {
                            "publisherId": "163194",
                            "adSlot": "5100885",
                            "kadfloor": "0.10"
                        },
                        "sharethrough": {
                            "pkey": "O1uQTzZpT3kBx0VlGkllMFYq"
                        }
                    },
                    "adunitcode": "/1018099/tigerdroppings_skyscraper_left"
                },
                "tid": "d5058b30-c20e-4659-9f28-ef136d83143a",
                "data": {
                    "adserver": {
                        "name": "gam",
                        "adslot": "/1018099/tigerdroppings_skyscraper_left"
                    },
                    "pbadslot": "/1018099/tigerdroppings_skyscraper_left"
                },
                "gpid": "/1018099/tigerdroppings_skyscraper_left"
            },
            "id": "/1018099/tigerdroppings_skyscraper_left",
            "banner": {
                "topframe": 1,
                "format": [
                    {
                        "w": 160,
                        "h": 600
                    }
                ]
            },
            "secure": 1
        }
    ],
    "source": {
        "tid": "c64d1b03-f956-4705-a044-c4aa3a7ca656"
    },
    "ext": {
        "prebid": {
            "auctiontimestamp": 1741071710973,
            "targeting": {
                "includewinners": true,
                "includebidderkeys": false
            },
            "storedrequest": {
                **"id": "tigerdroppings"**
            },
            "channel": {
                "name": "pbjs",
                "version": "v9.30.0"
            }
        },
        "tmaxmax": 1500
    },
    "regs": {
        "ext": {
            "gdpr": 1,
            "us_privacy": "1---"
        }
    },
    "user": {
        "ext": {
            "consent": "CQMj2AAQMj2AAEsACBENBcFoAP_gAEPgACiQKxJD7C7FbSFDwH53aLsAMAhHxsBAQoQAAASBAmABQAKQIAQCgkAYFASgBAACAAAAICRBIQIECAAAAUAAQAAAAAAEAAAAAAAKIAAAgAEAAAAIAAACAIAAEAAIAAAAEAAAmAgAAIIACAAAgAAAAAAAAAAAABAAAgCAAAAAAAAAAAAAAAAAgQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABBBNIPYXYraQoeQ8K7BdgBgEK-NgACFCAAACQIEwAKABSBACAUkgCAIgUAAAAAAAABASIJAABAAEAAAgAKAAAAAAAgAAAAAABBAAAEAAgAAAAAAAAQBAAAgABAAAAAgAAESEAABBAAQAAAAAABAAAAAAAAAIAAEAAAAAAAAAAAAAAAAAAEAA.f_wAAAAAAAA",
            "eids": [
                {
                    "source": "audigent.com",
                    "uids": [
                        {
                            "id": "060ixebj2g5kk6hbb6jdiclaeha6fg7kle8uomgwsqyyy0sgg0wlui0emse0oq2y0",
                            "atype": 1
                        }
                    ]
                },
                {
                    "source": "id5-sync.com",
                    "uids": [
                        {
                            "id": "ID5*zOg61b_ZsvJg3uI3mEGzB_9hPFfCwwPoZvRcq_bJWboBOmAc0ODwu_Ps_T_7NCUc_3hMFvtyGsWbr4vpi_AYbP96KVdRY92d8MjpFKFLG8L_eztpJStZIQeIYz_ys_1W_3wfDlkYCIp2tGgNcK-D0v-BU7LCAn_1XX0yPgWiTrf_hAtnThlH7pBTFvtcl66A_4op9k2sWUXgy2q09tw27_-MOX-OcO9C31YdVH8N9uv_t3iE7V2LA3nT7Q5cBNx5_7aN0ULZ6K5pGVOx7IPtiv-6wwRpDljgw9y_KxvNlHX_v9c0Rf760WaxiwWfNGHk_8F-yUdbiqQfYnBkb26BqQ",
                            "atype": 1,
                            "ext": {
                                "linkType": 2,
                                "pba": "h3oxBqt7dIyGlcImbNiRi273qfEsivsbXlZxh0ZLZZ4="
                            }
                        }
                    ]
                },
                {
                    "source": "crwdcntrl.net",
                    "uids": [
                        {
                            "id": "78e945c20ddc8f164462bd85302e4945a7021dfa321cd5c603fdf5ad08684518",
                            "atype": 1
                        }
                    ]
                },
                {
                    "source": "pubcid.org",
                    "uids": [
                        {
                            "id": "ed84fd73-5f62-48f2-abb2-aa6f25c6559e",
                            "atype": 1
                        }
                    ]
                }
            ],
            "ConsentedProvidersSettings": {
                "consented_providers": "2~70.89.93.108.122.149.196.236.259.311.313.323.358.415.442.486.494.495.540.574.609.864.981.1029.1048.1051.1095.1097.1126.1205.1276.1301.1365.1415.1449.1514.1570.1577.1598.1651.1716.1735.1753.1765.1870.1878.1889.1958.1960.2072.2253.2299.2373.2415.2506.2526.2531.2568.2571.2575.2595.2624.2677.2778~dv."
            }
        }
    },
    "site": {
        "domain": "tigerdroppings.com",
        "keywords": "LSUFootball,LSURecruiting,LSUBasketball,LSUBaseball,LSUSports,News,Tickets,BrianKelly,SEC,MessageBoard",
        "publisher": {
            "domain": "tigerdroppings.com",
            "id": "tigerdroppings"
        },
        "page": "https://www.tigerdroppings.com/"
    },
    "device": {
        "w": 1664,
        "h": 1110,
        "dnt": 0,
        "ua": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Safari/537.36",
        "language": "en",
        "ext": {
            "vpw": 1664,
            "vph": 941
        },
        "sua": {
            "source": 1,
            "platform": {
                "brand": "Windows"
            },
            "browsers": [
                {
                    "brand": "Not(A:Brand",
                    "version": [
                        "99"
                    ]
                },
                {
                    "brand": "Google Chrome",
                    "version": [
                        "133"
                    ]
                },
                {
                    "brand": "Chromium",
                    "version": [
                        "133"
                    ]
                }
            ],
            "mobile": 0
        }
    },
    "id": "6a6f51bd-f875-467b-ac6c-d0ccbab38f94",
    "test": 0,
    "tmax": 1300
}

When the stored impression includes additional bidders, they are ignored. Here is an example stored impression with additional bidders:

{
  "id": "td-default",
  "ext": {
    "prebid": {
      "bidder": {
        "amx": {
          "tagId": "4kMMcv6g1"
        },
        "rise": {
          "org": "677bc507f3a6d40001b51a16"
        },
        "insticator": {
          "publisherId": "5c317862-a09c-4129-9969-c07d71fd0d86",
          "adUnitId": "01JJ5BKC7A0N8KR2F4KTWE2NMA"
        },
        "medianet": {
          "cid": "8CU4Z6GJW",
          "crid": "397773531"
        },
        "colossus": {
          "groupId": "1201"
        }
      }
    }
  }
}

ollyburns avatar Mar 04 '25 07:03 ollyburns

@ollyburns - what is the contents of the global stored request tigerdroppings? It's not allowed to contain the imp array.

What I hear you saying is that the impression-level stored request works to add bidders to the imp UNLESS there's also a global stored request present.

If the global stored request doesn't contain an imp array, then I would agree this is a bug.

bretg avatar Mar 04 '25 15:03 bretg

It just has an schains object (ext.prebid.schains) in it, no imp array.

You hear correct - the additional bidders stop being invoked if the global stored request is present.

ollyburns avatar Mar 04 '25 15:03 ollyburns

@ollyburns - this worked for me on a local test case. Please try removing "id": "td-default", from your impression-level stored request?

bretg avatar Mar 04 '25 16:03 bretg

@ollyburns - let me know if you get a chance to try this again without the id

bretg avatar Mar 07 '25 21:03 bretg

Hi @bretg, apologies for the delay. I tested without the id in the stored impression and am still getting the same problem - the additional bidders defined in the stored impression are not being added to the auction.

For completeness, here is the contents of the stored request:

{
    "ext": {
        "prebid": {
            "schains": [
                {
                    "bidders": [
                        "colossus",
                        "medianet"
                    ],
                    "schain": {
                        "complete": 1,
                        "nodes": [
                            {
                                "asi": "browsi.com",
                                "sid": "t-0000002",
                                "hp": 1
                            }
                        ]
                    }
                },
                {
                    "bidders": [
                        "amx",
                        "rise",
                        "insticator"
                    ],
                    "schain": {
                        "complete": 1,
                        "nodes": [
                            {
                                "asi": "teal.works",
                                "sid": "0000002",
                                "hp": 1
                            }
                        ]
                    }
                }
            ]
        }
    }
}

ollyburns avatar Mar 08 '25 08:03 ollyburns

@ollyburns - @AntoxaAntoxic found that there's a weird issue with the system values that are floats get converted to strings, so the system thinks there was a change.

In the request above there are two fields that are floats:

"reserve": 0.1
"floor": 0.1

So another workaround to try would be switching these to strings?

Anyhow, this is obviously something that needs to get fixed. He's looking into the options.

bretg avatar Mar 10 '25 17:03 bretg

@ollyburns - the fix was released in https://github.com/prebid/prebid-server-java/releases/tag/3.25.0 . Closing this issue for now. Feel free to reopen in case of further issues or raise a new one.

Net-burst avatar May 07 '25 14:05 Net-burst