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.

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

Option to do array merges or permissioning on user segments #4111

Open
patmmccann opened this issue Dec 16, 2024 · 3 comments
Open

Option to do array merges or permissioning on user segments #4111

patmmccann opened this issue Dec 16, 2024 · 3 comments

Comments

@patmmccann
Copy link
Contributor

Since PBS does not do server side array merges, https://github.com/prebid/Prebid.js/pull/12571/files , will allow user segment arrays to be repeated for each bidder in bidder config. This can result in a very large object.

In the case of eids, there are some optimizations using eidPermissions, but this is not possible in user data segments.

This could be solvede with either a 'doJsStyleMerge' argument or a user data permission type flag.

@bretg
Copy link
Contributor

bretg commented Dec 17, 2024

@dgirardi or @patmmccann - please define a "JsStyleMerge". I don't understand "pre-concatenate" at a deep enough level to write requirements. I strongly want to avoid a true deep merge of arrays with full merging of duplicates. A simple "concatenate-with-no-deduping" approach would be ok.

How are we going to know when to do a "traditionalMerge" vs a "concatenateArraysMerge"? I don't like the idea of embedding merge instruction metadata in every array. If we're talking about a limited number of ORTB locations that require this special behavior, then the thing to do is define server-side configuration for publishers/host companies to opt-in to the new behavior. Otherwise it's breaking.

We need to delineate all possible destinations in ORTB where merge behavior needs to be variable:

  • user.eids[] (merged from bidderconfig only? stored requests also?)
  • user.data[]
  • site.content.data[]
  • app.content.data[]
  • bcat?

Then we have server-side account-level configuration that defines the merge strategy. e.g.
settings:
merge:
concatenate:
- user.eids
- user.data
- site.content.data

And maybe we could support an "all" keyword so all array merges (even beyond the named locations?) are concatenated?


From the slack workspace:

Bret:
Prebid Server does a fair amount of merging: stored requests, module output, profiles. For 8 years the strategy has always been to replace arrays. It is surprising this is the first time it's caused an issue. I imagine the team would be willing to consider a simple alternate merge strategy (like concatenate with no de-duping and no trying to detect identical keys for a deep merge), but it's not clear to me how you would choose which merge strategy where.

Demetrio:
js only recently started using bidderconfig for eids, and that's likely much more noticeable than something like bcat being merged differently server side. previously we had a special eidpermissions setting for doing this, but the motivation to switch was to remove the distinction between passing EIDs using setBidderConfig and configuring a user ID module

the best outcome is that client and server use the same merging logic for bidder config. I'm not sure if it's feasible, if js decided to conform to server on the next major release, I'd expect that to be very painful for publishers

data segments would have the same problem, although maybe it's not as common to have global and bidder-specific ones there?

Here's an idea: js pre-concatenates arrays. If the client side config is {ortb2: {array: [1]}} and bidder config for someBidder is {ortb2: {array: [2]}} , the PBS adapters sends {bidderconfig: {bidders: ['someBidder'], config: {ortb2: {array: [1, 2]}}}}
1:13
is there something else being merged server side, or some other reason why this wouldn't work?

Bret:
My concern is more around arrays of objects in particular. e.g.
Merging

user.ext.eids:[{
          "source": "adserver.org",
          "uids": [
            {
              "id": "1111",
              "ext": {
                "rtiPartner": "TDID"
              }
            }
          ]
        },{
          "source": "pubcommon",
          "uids": [
            {
              "id": "2222"
            }
          ]
}}

with

user.ext.eids:[{
          "source": "pubcommon",
          "uids": [
            {
              "id": "3333"
            }
          ]
}}

Simple concatenation will leave this with two objects where source="pubcommon" and different IDs. This is going to confuse someone downstream.
In the simpler case of bcat, can we assume that downstream entities wouldn't choke on duplicate bcat entries. e.g. bcat: [ 1,2,3,1,3,5 ] would be entirely possible without de-duping.

Demetrio:
as I see it it's just a matter of having a clear and consistent merging strategy, we leave the semantics to the publisher. if you want to override a specific EID for a specific bidder, you'd set up to achieve that according to how merging works

the problem is the inconsistency between js and server. I'm hoping that pre-merging arrays client side makes it so that it looks as though server is doing the same merge as JS.

@dgirardi
Copy link
Contributor

We need to delineate all possible destinations in ORTB where merge behavior needs to be variable:
If we're talking about a limited number of ORTB locations that require this special behavior, then the thing to do is define server-side configuration for publishers/host companies to opt-in to the new behavior. Otherwise it's breaking.

This involves every array, but only when merging ext.prebid.bidderconfig. Since some versions of JS work around this now, we would need a request-level toggle to avoid doing it twice. It would also make it backwards compatible - even if something other than JS uses bidderconfig.

I don't see the advantage of doing this for specific parts of the request, it will quickly get out of date, and JS always uses the same logic to merge everything from bidder config.

please define a "JsStyleMerge".

Here's the implementation. Merging a JSON object B into a JSON object A (A' = merge(A, B)) means, for each key K:

  • If both A[K] and B[K] are JSON objects, A'[K] = merge(A[K], B[K]) recursively
  • If both A[K] and B[K] are JSON arrays, then A'[K] is the concatenation of A[K] with every item in B[K] that does not deep equal any item in A[K]
  • Otherwise, A'[K] = B[K]

@bretg
Copy link
Contributor

bretg commented Dec 18, 2024

Discussed in committee. The current proposal is:

  1. Limit the change in the array merge behavior to just the handling of ext.prebid.bidderconfig
  2. Only do this enhanced merge when the account.settings.bidderconfig.array_merge is "concat". (defaults to "replace" in 3.x).
  3. The "concat" style is defined just as concatenating the two arrays. No checks for duplication, no check for deep equality. This covers known use cases and will need to be well-documented. If this causes a problem downstream, we'll have to re-evaluate and perhaps the performance impact of true deep-merge scenarios.

Here's the use case:

bidderA and bidderB get special EID or special data segment that other bidders should not get. However, there are many other EIDs or segments that all bidders receive. e.g.

user.eids: [ 10 objects ],
user.data: [ 10 objects ],
site.content.data: [ 10 objects ],
ext.prebid.bidderconfig
   bidders: [bidderA, bidderB]
   config:
     ortb2: 
       user.eids: [... 1 extra ...]
       user.data: [... 1 extra ...]
       site.content.data: [... 1 extra ...]

What PBS will do in this case is overwrite existing "10 objects" with the "1 extra". This forces publishers to blow up their bidderconfig with a workaround:

user.eids: [ 10 objects ],
user.data: [ 10 objects ],
site.content.data: [ 10 objects ],
ext.prebid.bidderconfig
   bidders: [bidderA, bidderB]
   config:
     ortb2: 
       user.eids: [... repeat the 10 objects and add 1 extra ...]
       user.data: [... repeat the 10 objects and add 1 extra ...]
       site.content.data: [... repeat the 10 objects and add 1 extra ...]

So the goal here is remove the necessity of repeating the baseline data in the bidder-specific config scenario.

Note: ext.prebid.bidderconfig.ortb2 only supports three top-level objects: site, app, and user.

@bretg bretg moved this from Triage to Community Review in Prebid Server Prioritization Dec 18, 2024
@bretg bretg moved this from Community Review to Ready for Dev in Prebid Server Prioritization Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Dev
Development

No branches or pull requests

3 participants