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

Stored Request A/B testing support for PBS #1645

Closed
laurb9 opened this issue Jan 7, 2021 · 17 comments
Closed

Stored Request A/B testing support for PBS #1645

laurb9 opened this issue Jan 7, 2021 · 17 comments

Comments

@laurb9
Copy link
Contributor

laurb9 commented Jan 7, 2021

Description

We need a way for PBS-Go to randomly choose "experimental" values for any stored request, as a way to validate potential changes in a statistically relevant way and without impacting the bulk of the revenue. These experiments could be:

  • adding a new bidder
  • changing floors
  • ad sizes
  • etc

How it works

A list of experiments such as the one below is saved in the stored request under ext.prebid.storedrequest.ab_config. Each experiment has a code that identifies it for analytics purposes, the desired split ratio, the id of the stored request to use instead of the requested one and a map to replace eventual stored imp ids.

stored request

{
  "ext": {
    "prebid": {
      "storedrequest": {
        "ab_config": [
	  {
            "code": "add_new_bidder_test_1",
            "ratio": 10.0,
            "request_id": "<new-stored-req-id>",
            "imp_ids": {
              "imp-id-1": "<new-stored-imp-id-1>"
              ...
            }
	  },
	  ...
       ]

When PBS receives an incoming auction request that uses stored request stored-req-id-1 and stored imp stored-imp-id-1, for 10% of such requests it will use the data fromreplacement-stored-req-id and replacement-stored-req-id-1 instead. When the substitution happens, the field ext.prebid.storedrequest.ab_code will be set to add_new_bidder_test_1 for analytics modules. No changes are done to the control group (the one that uses the original ids).

If the replacement stored requests cannot be found, it is an error, just as if they had been specified in the original request.

Alternative implementation

The ab_config rules could be set in the Account object instead, as a map of stored-request-id to ab_config. This has the advantage that all ids can be remapped before fetching so only one fetch is required to fill them:

account "Publisher"

{
  "id": "Publisher",
  "disabled": false,
  "ab_config": {
    "stored-req-id-1": [ <config above> ],
    "other-stored-req-id": [ <other config> ],
    ...
  }

Was:

the requirements would be something like this (implementation details aside):

  • Customer can define additional secondary values for a given stored request/imp, possibly identified by a tag
  • Customer can define a distribution ratio of tags for a stored request/imp, adding up to 100%.
    For example: primary/tag1/tag2=80/15/5. By default the ratio is primary=100 and the untagged value is always chosen, as it works today.
  • PBS will randomly select the primary or one of the tags respecting the ratio, for each request.
  • The selection information (selected tags and distribution configuration) must be made available in the analytics object. This should not depend on the Customer adding extra fields in the stored data, but rather be a function of PBS itself.
  • Extra credit: synchronize the tag selection between the stored request and all stored imps. This does impose on the Customer to define the same tags for all stored items used in a request, and creates additional handling problem for PBS when they're not all present.
@laurb9 laurb9 changed the title A/B testing for PBS A/B testing support for PBS Jan 8, 2021
@SyntaxNode
Copy link
Contributor

I'm curious how you propose to express the tag within the request's JSON structure, or are you perhaps suggesting PBS choose between two stored requests?

@bretg
Copy link
Contributor

bretg commented Jan 8, 2021

I'd like to propose a 'module' approach, because it moves us towards the longer term vision we've discussed before -- evolving PBS to have hooks where an ecosystem of modules could plug in.

here are 3 example hooks:

  • request-enhancement: these modules get a chance to sniff and modify the OpenRTB
  • pre-auction: these modules run after request-enhancement and can alter which bidders/parameters are going to run in this auction. e.g. bidder optimization.
  • post-auction: these modules run after the responses have come back, and can sniff/modify the response. e.g. brand protection.

An A/B testing feature could plug into the request-enhancement hook, sniffing for certain stored-ids based on configuration, replace them with configured values.

For example, say the stored request ID is "1111-testapp-Footer_1-320x50". The "SRID Redirect" module configuration could be something like

modules:
    srid-redirect:
        enabled: true
        mappings:
            1111-testapp-Footer_1-320x50:
                1111-testapp-Footer_1-320x50: 95
                1111-testapp-Footer_1-320x50-test: 5   // replace 5% of the references with the "-test" version
            1111-testapp-Footer_2-320x50:
                1111-testapp-Footer_1-320x50: 100       // permanent redirect

PBS would need to have an infrastructure for providing modules with module-specific config.
modules. It could start as flat files, but modules should be able to get the data they need at runtime from a DB (whether mysql or mongo). If modules were responsible for their own queries at startup and refresh, then basic flat-file config is enough:

    srid-redirect:
        enabled: true
        config_query: select srid_config from module_config
        db_host: db.example.com
        config_refresh: 60
        ...

Obviously there are technical details to work out, and the implementation might look somewhat different between Go and Java, but the general idea is that it might be worth settling on a common set of hooks and interfaces so modules can be ported between the two code bases.

@laurb9
Copy link
Contributor Author

laurb9 commented Jan 13, 2021

I'm curious how you propose to express the tag within the request's JSON structure, or are you perhaps suggesting PBS choose between two stored requests?

Indeed, the intention is for PBS to transparently choose, because the client isn't able to do so in these cases.

I'd like to propose a 'module' approach, because it moves us towards the longer term vision we've discussed before -- evolving PBS to have hooks where an ecosystem of modules could plug in.

I like this idea. It might limit some of choices for implementing, but it's a good plan. This functionality may need to have hooks into different places for syncing the selection though.

PBS would need to have an infrastructure for providing modules with module-specific config.
modules. It could start as flat files, but modules should be able to get the data they need at runtime from a DB

Ideally this should not need a configuration update because that requires restarts, other than activating the functionality itself. The actual setup should come from the dynamic configuration so customers can do it and if it can use the existing stored request management UIs, all the better.

I've left out implementation ideas in the initial post because I wanted to see what kind of traction this can have.

@bszekely1
Copy link

@bretg I really like your design idea to have hooks that can be used to decorate or read each aspect of the auction.

@laurb9 I just did a deeper review of the PBS fields that have the potential to be modified for testing, but I am not seeing that many areas for optimization. Would you be able to describe what fields you may be focusing on so we can determine how this should be crafted?

@bretg
Copy link
Contributor

bretg commented Jan 14, 2021

Ideally this should not need a configuration update because that requires restarts, other than activating the functionality itself. The actual setup should come from the dynamic configuration

Makes sense.
a) each module is welcome to have its own configuration logic. Call for data from no-sql or mysql.
b) PBS-core could pass data from the existing account DB query. We have some ideas towards expanding the data that can be stored in the accounts table. More to come.

@laurb9
Copy link
Contributor Author

laurb9 commented Jan 19, 2021

I just did a deeper review of the PBS fields that have the potential to be modified for testing, but I am not seeing that many areas for optimization. Would you be able to describe what fields you may be focusing on so we can determine how this should be crafted?

@bszekely1 Any part of the request and/or imp can potentially be subject to A/B testing. It could be something obvious like the floor, sizes or keywords but it could be something else from the request. The idea is for PBS to transparently alternate between stored request values for a given stored request id (and stored imps).

@laurb9
Copy link
Contributor Author

laurb9 commented Jan 19, 2021

a) each module is welcome to have its own configuration logic. Call for data from no-sql or mysql.
b) PBS-core could pass data from the existing account DB query. We have some ideas towards expanding the data that can be stored in the accounts table. More to come.

Adding support for a new data type in PBS-Go at least is somewhat involved, so I'd like to see if it's possible to use existing configuration sources where it makes sense. Since accounts are generally not changing much, I expect hosts are caching them for longer.

I am thinking the (primary) stored requests could store the distribution information in an ext field which can cause PBS to replace it with one of the secondaries for some small percentage of time. This can be implemented easily but will have a small performance impact for reading the replacement stored request, which might be ok since they would all be cached most of the time. Or mitigate if we bundle it with the query for stored imps, which is done separately anyway.

@laurb9
Copy link
Contributor Author

laurb9 commented Jan 19, 2021

As a for instance, let's say incoming request looks like this

{
  "imp": [
    {
      "ext": {
        "prebid": {
          "storedrequest": {
            "id": "STORED_IMP_ID"
          }
        }
      }
    }
  ],
  "ext": {
    "prebid": {
      "storedrequest": {
        "id": "STORED_REQ_ID"
      }
    }
  }
}

The stored request identified by "STORED_REQ_ID" has something like this, where let's say we put the a/b testing setup in request.ext.prebid.ab_config

{
  "ext": {
    "prebid": {
      "storedrequest": {
          "ab_config": {
             "TEST1": 5.0,
             "TEST2": 15.0
          }
        }
      }
    }
  }
}

Then, on seeing ab_config PBS makes a random number and picks one of the scenarios

  1. To use primary stored req, continue as if nothing happened, because it's already loaded

  2. Otherwise will query STORED_REQ_ID-TEST1, STORED_IMP_ID-TEST1, STORED_IMP_ID etc, and will proceed with those. Same if TEST2 came up when throwing the dice. We query both versions for the imps in case they don't have alternative versions for this particular test.

This is nice because synchronizes the imps and the request selection. It also makes it easy to validate or control externally by passing ab_config directly in the request, next to id.

@bretg
Copy link
Contributor

bretg commented Jan 20, 2021

Assuming that we're going to introduce hooks for enhanced functionality, I would like to avoid building any more functionality into the core PBS that's only going to be used by a handful of host companies.

The long term view is that features like PG, events, maybe even privacy rules could migrate to become optional modules.

However, when we've discussed modules in the past, I believe @SyntaxNode had some concern about how they would be implemented in PBS-Go. I'd like to get requirements from the Go team -- are there modularity requirements that would make the feature more or less appealing/implementable for PBS-Go?

@laurb9
Copy link
Contributor Author

laurb9 commented Jan 22, 2021

Once the hooks are implemented, the code could also be refactored as well to become a module or part of one. I would not want this feature to wait for PBS-Go to become modular though. I'm thinking to throw a proof of concept with minimal code footprint if we can agree the example above is reasonable, since established interfaces are harder to change than code.

@SyntaxNode
Copy link
Contributor

SyntaxNode commented Jan 22, 2021

I believe @SyntaxNode had some concern about how they would be implemented in PBS-Go.

Go is statically linked, so a true module concept isn't really possible. There is a plugin concept, but it comes with enough caveats that we haven't explored it further.

Our stance is to logically separate the code, such as with adapters and analyzers, and to allow hosts to only enable what they'd like. The compiled size of PBS-Go is relatively small, so I don't think folks will complain if we ship a little extra code they don't use. If hosts want to use their own private modules, then they can fork and we'll hopefully provide clean enough code to isolate their addition enough to avoid endless rebasing.

I like the idea of using the config system to enable and configure modules.

In this case, I imagine we could build in a hook at the start of the HoldAuction call after the endpoints have composed their OpenRTB request and before the request is split for each bidder. The hook would be intercepted by a new type of "module". I do like @bretg's proposal of the mapping concept existing outside of the OpenRTB json model.

@laurb9
I think your proposal would be complicated to implement since the A/B scenario model isn't compatible with the OpenrRTB model due to the "TEST1" / "TEST2" choices.
Sorry. I misunderstood the proposal. You had intended TEST1 and TEST2 to be other stored request ids.

@bretg
Copy link
Contributor

bretg commented Jan 22, 2021

Discussed in PBS committee. Additional detail is needed:

  • how will analytics work -- which field(s) will carry the test chosen?
  • is there a way to make sure this can be done with one stored request DB fetch?
  • If not, can we eliminate the possibility of having more than one extra DB fetch?

@hhhjort
Copy link
Collaborator

hhhjort commented Jan 22, 2021

To properly get value out of this framework, it seems that it will be necessary to tie detailed analytics data with outside sources of data (to get final win results and such). Have people already tied together these multiple sources of data so that auction specific data (bid request ID?) can be related to final wins and/or other such performance data, or is this another part of the puzzle that needs to be solved? I would hate to go through the work of implementing this, for it then to be discovered that tying together the data sources is going to be too difficult to be worthwhile. If similar merging of data sources has already been done, then it is safe to assume it is a solved problem. But if this is uncharted territory, it would be nice to know that the data merge has at least been spec'ed out and looks reasonable.

@laurb9
Copy link
Contributor Author

laurb9 commented Jan 23, 2021

@hhhjort You are correct, we need to know which bids actually won over the adserver in order to be able to get any useful insights about spend. The events framework described in #1015 was added recently to PBS-Go and will allow us to link impression and win events to PBS auctions through the bid id.

Briefly, the change involves PBS injecting event URLs to the bids and VAST returned, which include the bid-id, bidder and auction timestamp, and which are then hit by PUC or SDK when rendering the creative.

I can also think of some cases where we can work without actual wins. Whether PBS won or not, we have the bids meaning we know the minimum price that adserver would have to beat, which bidders bid, how long auction took and so on. It's only speculation at this point, but I believe that can be used to optimize the PBS side of the equation.

@bretg bretg changed the title A/B testing support for PBS Stored Request A/B testing support for PBS Feb 17, 2021
@SyntaxNode
Copy link
Contributor

I like the simplicity of choosing a stored request. I'm good with this being a PBS-Go feature, with the following conditions:

  • Hosts have the ability to enable or disable this feature using the existing configuration system. The default is to disable.
  • This feature is documented in the prebid.github.io repo, to be published to https://docs.prebid.org. The number of possible database calls should be explicitly mentioned, such that hosts who are not using a memory cache are warned of performance impacts.
  • Only the top level ab_config will be resolved. Nested ab tests will not be supported.
  • The implementation will be isolated as much as possible from the PBS-Core logic, I recommend a separate package along the lines of hooks/abtest, with the understanding this will be migrated to the formal modular approach when available. This change is expected to be a breaking change in terms of configuration for hosts who enable this feature, but perhaps we can build in an auto-upgrade path.
  • You address the open concern regarding analytics. We previously discussed in the committee including the decision of the a/b test logic in the response so publishers can understand which data correlates to which a/b config.

Seem reasonable? If so, please update the top post of this thread to be the complete implementation spec, with configuration examples. It's hard to read through the entire conversation to figure out the latest state of things.

@laurb9
Copy link
Contributor Author

laurb9 commented Mar 19, 2021

Thank you. This sounds like the right way to move forward.

We've discussed the possible approaches internally and I opened a proof of concept draft in #1766 to illustrate a possible implementation that satisfies the modularity requirement. It is just a fetcher that can be composed or not with the others so it would be easy to enable or disable.

This POC uses the stored request because it is easier to test with. The same configuration can be easily supplied in the account object instead (as a map of stored request id -> ab_config), which would allow us to perform the stored request fetch in a single step again. I have not implemented it here because the fetchers do not yet take an account parameter. #1024 is another ticket that wants fetchers to be account-aware, so adding account is not outside the realm of possibility.

One of the drawbacks of the format proposed originally is that it prescribes an ID format for stored requests, and I think that restricts its use because some hosts may use uids or integers or other formats. So the format we are proposing is more verbose but at the same time more precise.

@laurb9
Copy link
Contributor Author

laurb9 commented Apr 2, 2021

Going to put the brakes on this one for now.
I closed #1766 - the POC there is definitely usable but managing the stored request configuration is a relatively complex task and prone to error.

We are exploring other approaches. It is ok to close this ticket (or stalebot will anyway :) ).

@bretg bretg closed this as completed Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants