TheMediaGrid
commented
2 years ago @bretg thank you for submitting this!
Closed bretg closed 2 years ago
TheMediaGrid
commented
2 years ago @bretg thank you for submitting this!
bsardo
commented
2 years ago Thanks for uncovering this and for providing example requests. @VeronikaSolovei9 and I did some investigating as this issue relates to PBS-Go.
Regarding the first issue where the FPD fields user.data, user.ext.data, site.content.data and site.ext.data are not present, it appears this only pertains to the resolved request at .ext.debug.resolvedrequest. The bid response sent to rubicon is correct with all of these pieces of information moved to the appropriate sections of the bid request IAW the referenced first party data spec. We compared the PBS-Go and PBS-Java rubicon bid requests stored in .ext.debug.httpcalls.rubicon to be sure.
Here is the PBS-Go rubicon bid request:
{
"id": "1001",
"imp": [{
"id": "some-impression-id",
"banner": {
"format": [{
"w": 468,
"h": 60
}, {
"w": 300,
"h": 600
}, {
"w": 300,
"h": 250
}],
"ext": {
"rp": {
"size_id": 15,
"alt_size_ids": [1, 10],
"mime": "text/html"
}
}
},
"secure": 1,
"ext": {
"rp": {
"zone_id": 535510,
"target": {
"customAdUnitAttr1": ["customAdUnitValue1"],
"customAdUnitAttr2": ["customAdUnitValue2"],
"customSiteAttr1": ["customSiteValue1"],
"customSiteAttr2": ["customSiteValue2"],
"page": ["www.rubiconproject.com"],
"pbadslot": "/test/adslot",
"search": ["site search terms"]
},
"track": {
"mint": "",
"mint_version": ""
}
},
"gpid": "/test/adslot"
}
}],
"site": {
"domain": "rubiconproject.com",
"page": "www.rubiconproject.com",
"search": "site search terms",
"publisher": {
"ext": {
"rp": {
"account_id": 1001
}
}
},
"content": {
"userrating": "4",
"data": [{
"name": "www.dataprovider1.com",
"segment": [{
"id": "687"
}, {
"id": "123"
}],
"ext": {
"segtax": 4,
"cids": ["test_cid"]
}
}]
},
"keywords": "sitekey1,sitekey2",
"ext": {
"rp": {
"site_id": 113932
}
}
},
"device": {
"ua": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36",
"ip": "174.57.47.200",
"ext": {
"rp": {
"pixelratio": 0
}
}
},
"user": {
"keywords": "userkey1,userkey2",
"data": [{
"name": "dataprovider.com",
"segment": [{
"id": "1"
}],
"ext": {
"segtax": 4
}
}],
"ext": {
"rp": {
"target": {
"customUserAttr1": ["customUserValue1"],
"customUserAttr2": ["customUserValue2"],
"iab": ["1"]
}
}
}
},
"test": 1,
"at": 1,
"source": {
"tid": "79347456-123e-415a-b525-d1ed24d73e88"
}
}Our team can look into how to make the resolved request accurately reflect what the request looks like before it is split into bid requests. The current behavior is not surprising since we are deliberately extracting all of the user, site and app FPD right after the request is merged with any stored requests and then adding the applicable pieces to each bid request downstream.
bsardo
commented
2 years ago As for the second issue, we believe we need a spec update. It appears both PBS-Go and PBS-Java were behaving according to the spec: In The PBS First Party Data spec, /openrtb2/auction endpoint section, requirement 6c states:
ext.prebid.bidderconfig.config.ortb2.user is merged into user.ATTR (where ATTR is in yob, gender, keywords) or user.ext.data.ATTR (for other values of ATTR).This should probably be changed to:
ext.prebid.bidderconfig.config.ortb2.user is merged into user.ATTR (where ATTR is in yob, gender, keywords, data) or user.ext.data.ATTR (for other values of ATTR).We'll make this change in PBS-Go so ext.prebid.bidderconfig.config.ortb2.user.data is mapped to user.data.
TheMediaGrid
commented
2 years ago @bsardo thanks a lot!
Re: 2nd issue- makes perfect sense. Re: 1st issue. Can you please provide more clarity on whether this is something that you can fix in PBS-Go to make sure there is no difference in the PBS config regardless of the hosting version to make the user.data piece work?
bsardo
commented
2 years ago Hi @TheMediaGrid. For the first issue we have a PR up now that’s in review that will update the resolved request so that it accurately reflects the result of the request merged with any referenced stored requests prior to the bid request generation phase. This means any first party data in the user.data field portion of the request or any referenced stored request will appear in the resolved request. The same applies to user.ext.data, site.content.data and site.ext.data.
VeronikaSolovei9
commented
2 years ago Debug log fix PR: https://github.com/prebid/prebid-server/pull/2196 User.data fix PR: https://github.com/prebid/prebid-server/pull/2197
bretg
commented
2 years ago Is this confirmed and closable?
SyntaxNode
commented
2 years ago The operational fix was released with PBS-Go 0.205.0. The fix to allow the resolved request to provide more information is still in progress.
TheMediaGrid
commented
2 years ago Is there a similar issue with ortb.site.content.data and ortb.app.content.data? Or is that only tied up to ortb.user.data? Just trying to make sure site/app.content.data are working in a unified way as well.
bsardo
commented
2 years ago Yes this same issue exists, at least in PBS-Go, for ortb2.site.content.data and ortb2.app.content.data.
bidderconfig.config.ortb2.site.content.data maps to site.ext.data.content.data.
bidderconfig.config.ortb2.app.content.data maps to app.ext.data.content.data.
@bretg, perhaps this also exists in PBS-Java?
Assuming we want bidderconfig.config.ortb2.{site/app}.content.data to merge into {site/app}.content.data, I think we will need another spec update, possibly to requirements 6a and 6b in Prebid Server and First Party Data, section /openrtb2/auction endpoint.
If we proceed with this change, do all or only a subset of the ORTB2 fields (including data) defined in bidderconfig.config.ortb2.{site/app}.content get merged into ortb2.{site/app}.content?
bretg
commented
2 years ago Good catch. Updated the FPD doc 6a and 6b to define that bidderconfig ortb2.site.content goes to site.content and ortb2.app.content goes to app.content.
If we proceed with this change, do all or only a subset of the ORTB2 fields (including data) defined in bidderconfig.config.ortb2.{site/app}.content get merged into ortb2.{site/app}.content?
Basically the intention was that EVERY ORTB field would get copied to the ORTB location. So yes, the whole content object should be copied.
VeronikaSolovei9
commented
2 years ago @bretg I just want to confirm one detail. We now have bidRequest.Site.Content.Data and bidRequest.App.Content.Data extracted from request. In further processing we set these values ({site or app}.Content.Data) to result site or app and then merge other fields.
With new update how this will work?
What value should take precedence in result site or app: bidderconfig.config.ortb2.{site/app}.content.data or bidRequest.{App/Site}.Content.Data ?
bretg
commented
2 years ago In further processing we set these values ({site or app}.Content.Data) to result site or app and then merge other fields. With new update how this will work?
Sorry, don't understand, @VeronikaSolovei9 - can you give an example?
What value should take precedence in result site or app: bidderconfig.config.ortb2.{site/app}.content.data or bidRequest.{App/Site}.Content.Data ?
This is defined in the doc as auction req 7:
7. If an attribute is defined in ext.prebid.bidderconfig.config.ortb2 and already exists in the OpenRTB object,
ext.prebid.bidderconfig.config.ortb2 should take precedence.This was discussed in last week's committee meeting:
Some OpenRTB fields are called out in the FPD specification such as yob, gender, and now data. Any others? We believe it makes sense to update the spec to use new wording to indicate all defined OpenRTB sub-fields are in scope, not just the specific ones listed in the spec today.
Ok, doc updated to remove the lists. This is more consistent with my response in #2234 -- let the publishers screw things up.
How do we merge arrays? This can get complicated and let’s try to keep it simple. Bret proposed to combine array elements together, not taking into account any specific item identifier such as “id” or “source”.
Isn't there a standard definition of a deep JSON merge? That's what we should do.
bsardo
commented
2 years ago This was discussed in last week's committee meeting:
Some OpenRTB fields are called out in the FPD specification such as yob, gender, and now data. Any others? We believe it makes sense to update the spec to use new wording to indicate all defined OpenRTB sub-fields are in scope, not just the specific ones listed in the spec today.
Ok, doc updated to remove the lists. This is more consistent with my response in #2234 -- let the publishers screw things up.
Great, the spec updates look good. All ORTB fields defined as FPD in ext.prebid.bidderconfig.config.ortb2.{app/site/user} will be merged into their corresponding root.{app/site/user} field for the applicable bidders.
bsardo
commented
2 years ago This was discussed in last week's committee meeting:
How do we merge arrays? This can get complicated and let’s try to keep it simple. Bret proposed to combine array elements together, not taking into account any specific item identifier such as “id” or “source”.
Isn't there a standard definition of a deep JSON merge? That's what we should do.
It appears a deep JSON merge overwrites arrays instead of concatenating them.
It's a bit of a challenge to find an explicit JSON deep merge definition, nor can I say with complete confidence which libraries in Go or Java are the de facto standard for performing this type of merge.
It should be noted though that PBS-Go uses the jsonpatch library MergePatch function quite a bit, including for merging the incoming request with any stored requests. And the MergePatch function is in accordance with the JSON Patch standard. This function does not concatenate arrays but rather overwrites arrays.
It appears a similar library, com.github.fge.jsonpatch.mergepatch.JsonMergePatch , is used quite a bit in PBS-Java. Given the names, I imagine this is also in accordance with the JSON Patch standard with the same behavior.
With that info in mind, coupled with the fact that merging all other types, both primitive and non-primitive, result in overwrites, our team's opinion is we should overwrite arrays instead of concatenating. The merge mechanics should then be clear to publishers since it will be the same for all field types in all scenarios (i.e. the mechanics of how we merge stored requests with the base request will be the same as how we merge FPD) and the development teams can use the same libraries throughout the PBS code bases to perform JSON merges.
On a similar note, I just want to point out that we have ORTB fields other than {app/site}.data and user.data in root.{app/site/user} that are arrays:
app.publisher.cat
{app/site}.cat
{app/site}.sectioncat
{app/site}.pagecat
{app/site}.content.producer.cat
{app/site}.content.catThese fields should also be merged in the same manner via overwriting instead of concatenating.
bsardo
commented
2 years ago The original issues described here have been resolved with PRs #2196 and #2197 so I'm closing this. I opened #2317 to track the broader enhancement described here that resulted in a spec change.
Grid reported an issue with user.data being transmitted to server-side adapters as user.ext.data.
The PBS First Party Data document does treat user.data in a special way:
So I tested this with PBS-Java -- works. However, it doesn't appear to work with PBS-Go. It's missing several FPD fields:
Here's the test request:
Here's the resolved request from PBS-Java:
And here's the resolved request from PBS-Go:
Possible bidderconfig issue
In a possibly related issue, bidderconfig is winding up in the wrong place in grid's outbound request. Not clear whether this is a problem in the grid adapter or PBS-core. Here's the request:
The outbound request to the grid endpoint contains this:
Note that user.ext.data.data is wrong in two ways: