prebid / prebid-server

Open-source solution for running real-time advertising auctions in the cloud.
https://prebid.org/product-suite/prebid-server/
Apache License 2.0
434 stars 744 forks source link

imp.ext.prebid.adunitcode is removed from imp.ext.prebid when rebuildImp is called #4060

Closed Abyfall closed 4 days ago

Abyfall commented 6 days ago

Hi guys, after upgrading to prebid server 3.1 we noticed that we couldn't retrieve the adUnitCode from imp.ext.prebid.adunitcode anymore.

It still present in the Prebid Server ORTB2 Extension Summary section of the doc so I assume it's a bug from prebid-server.

I tracked down the issue to the rebuildImpExt() & (e *ImpExt) marshal() https://github.com/prebid/prebid-server/blob/6e150f36c341d006d3426d813d7beaf420d9651d/openrtb_ext/request_wrapper.go#L1751-L1767

Those will recreate the imp.ext.prebid with the help of ExtImpPrebid struct. But as this struct doesn't contains the adunitcode, the value which was originally set is dropped.

This issue seems to exist since some time but we didn't face it before because we always had e.prebidDirty == false preventing thhe call to rebuildImpExt. This isn't the case anymore since the introduction of moveRewardedFromPrebidExtTo26 which always sets prebidDirty if prebid object was found in imp.ext.

https://github.com/prebid/prebid-server/blob/dd07431ef54c38a2617c04e76b3e7bda9c21f569/openrtb_ext/convert_up.go#L167-L175 _As a side note I think we should change moveRewardedFromPrebidExtTo26 https://github.com/prebid/prebid-server/blob/dd07431ef54c38a2617c04e76b3e7bda9c21f569/openrtb_ext/convert_up.go#L171 to also check if p.IsRewardedInventory is not nil before replacing the whole prebid field and flagging it as dirty_

bretg commented 5 days ago

Thanks for the bug report @Abyfall. A community PR would be welcomed here.

Abyfall commented 5 days ago

Thanks @bretg I just pushed a PR that solves this issue