Skip to content
This repository has been archived by the owner on Dec 22, 2022. It is now read-only.

Commit

Permalink
Adding support for aliases (prebid#296)
Browse files Browse the repository at this point in the history
* Added alias support to the request validation and bid preprocessing. No new unit tests yet.

* Fixed some lint errors.

* Added aliases as a return value from cleanOpenRTBRequests.

* Added alias support to the core auction logic.

* Fixed a bug with the timeout computation.

* Made sure that the core biddername gets used for operational metrics.

* Fixed a unit test which was broken after the changes.

* Cleaner, simpler parse/validate method.

* Added some tests, and fixed some bugs.

* Added one more use case, and tightened the input validation a bit.

* Updated the endpoint docs.

* Better alias docs. Fixed the header mismatches.
  • Loading branch information
dbemiller authored Feb 7, 2018
1 parent ee8922d commit b0932cd
Show file tree
Hide file tree
Showing 9 changed files with 359 additions and 200 deletions.
79 changes: 50 additions & 29 deletions docs/endpoints/openrtb2/auction.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ The only exception here is the top-level `BidResponse`, because it's bidder-inde

#### Details

##### Targeting
#### Targeting

Targeting refers to strings which are sent to the adserver to
[make header bidding possible](http://prebid.org/overview/intro.html#how-does-prebid-work).
Expand Down Expand Up @@ -140,49 +140,70 @@ to set these params on the response at `response.seatbid[i].bid[j].ext.prebid.ta
The winning bid for each `request.imp[i]` will also contain `hb_bidder`, `hb_size`, and `hb_pb`
(with _no_ {bidderName} suffix).

#### Improving Performance
#### Bidder Aliases

`response.ext.responsetimemillis.{bidderName}` tells how long each bidder took to respond.
These can help quantify the performance impact of "the slowest bidder."

`response.ext.errors.{bidderName}` contains messages which describe why a request may be "suboptimal".
For example, suppose a `banner` and a `video` impression are offered to a bidder
which only supports `banner`.

In cases like these, the bidder can ignore the `video` impression and bid on the `banner` one.
However, the publisher can improve performance by only offering impressions which the bidder supports.
Requests can define Bidder aliases if they want to refer to a Bidder by a separate name.
This can be used to request bids from the same Bidder with different params. For example:

`response.ext.usersync.{bidderName}` contains user sync (aka cookie sync) status for this bidder/user.
```
{
"imp": [
{
"id": "some-impression-id",
"video": {
"mimes": ["video/mp4"]
},
"ext": {
"appnexus: {
"placementId": 123
},
"districtm": {
"placementId": 456
}
}
}
],
"ext": {
"prebid": {
"aliases": {
"districtm": "appnexus"
}
}
}
}
```

This includes:
For all intents and purposes, the alias will be treated as another Bidder. This new Bidder will behave exactly
like the original, except that the Response will contain seprate SeatBids, and any Targeting keys
will be formed using the alias' name.

1. Whether a user sync was present for this auction.
2. URL information to initiate a usersync.
If an alias overlaps with a core Bidder's name, then the alias will take precedence.
This prevents breaking API changes as new Bidders are added to the project.

Some sample response data:
For example, if the Request defines an alias like this:

```
{
"appnexus": {
"status": "one of ['none', 'expired', 'available']",
"syncs": [
"url": "sync.url.com",
"type": "one of ['iframe', 'redirect']"
]
},
"rubicon": {
"status": "available" // If a usersync is available, there are probably no syncs to run.
"aliases": {
"appnexus": "rubicon"
}
}
```

A `status` of `available` means that the user was synced with this bidder for this auction.
then any `imp.ext.appnexus` params will actually go to the **rubicon** adapter.
It will become impossible to fetch bids from Appnexus within that Request.

A `status` of `expired` means that the a user was synced, but it last happened over 7 days ago and may be stale.
#### Bidder Response Times

A `status` of `none` means that no user sync existed for this bidder.
`response.ext.responsetimemillis.{bidderName}` tells how long each bidder took to respond.
These can help quantify the performance impact of "the slowest bidder."

`response.ext.errors.{bidderName}` contains messages which describe why a request may be "suboptimal".
For example, suppose a `banner` and a `video` impression are offered to a bidder
which only supports `banner`.

PBS requests new syncs by returning the `response.ext.usersync.{bidderName}.syncs` array.
In cases like these, the bidder can ignore the `video` impression and bid on the `banner` one.
However, the publisher can improve performance by only offering impressions which the bidder supports.

#### Debugging

Expand Down
58 changes: 41 additions & 17 deletions endpoints/openrtb2/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,19 @@ func (deps *endpointDeps) validateRequest(req *openrtb.BidRequest) error {
return errors.New("request.imp must contain at least one element.")
}

var aliases map[string]string
if bidExt, err := deps.parseBidExt(req.Ext); err != nil {
return err
} else if bidExt != nil {
aliases = bidExt.Prebid.Aliases
}

if err := deps.validateAliases(aliases); err != nil {
return err
}

for index, imp := range req.Imp {
if err := deps.validateImp(&imp, index); err != nil {
if err := deps.validateImp(&imp, aliases, index); err != nil {
return err
}
}
Expand All @@ -209,14 +220,10 @@ func (deps *endpointDeps) validateRequest(req *openrtb.BidRequest) error {
return err
}

if err := deps.validateBidRequestExt(req.Ext); err != nil {
return err
}

return nil
}

func (deps *endpointDeps) validateImp(imp *openrtb.Imp, index int) error {
func (deps *endpointDeps) validateImp(imp *openrtb.Imp, aliases map[string]string, index int) error {
if imp.ID == "" {
return fmt.Errorf("request.imp[%d] missing required field: \"id\"", index)
}
Expand Down Expand Up @@ -255,7 +262,7 @@ func (deps *endpointDeps) validateImp(imp *openrtb.Imp, index int) error {
return err
}

if err := deps.validateImpExt(imp.Ext, index); err != nil {
if err := deps.validateImpExt(imp.Ext, aliases, index); err != nil {
return err
}

Expand Down Expand Up @@ -321,7 +328,7 @@ func validatePmp(pmp *openrtb.PMP, impIndex int) error {
return nil
}

func (deps *endpointDeps) validateImpExt(ext openrtb.RawJSON, impIndex int) error {
func (deps *endpointDeps) validateImpExt(ext openrtb.RawJSON, aliases map[string]string, impIndex int) error {
var bidderExts map[string]openrtb.RawJSON
if err := json.Unmarshal(ext, &bidderExts); err != nil {
return err
Expand All @@ -332,26 +339,43 @@ func (deps *endpointDeps) validateImpExt(ext openrtb.RawJSON, impIndex int) erro
}

for bidder, ext := range bidderExts {
bidderName, isValid := openrtb_ext.BidderMap[bidder]
if isValid {
if err := deps.paramsValidator.Validate(bidderName, ext); err != nil {
return fmt.Errorf("request.imp[%d].ext.%s failed validation.\n%v", impIndex, bidder, err)
if bidder != "prebid" {
coreBidder := bidder
if tmp, isAlias := aliases[bidder]; isAlias {
coreBidder = tmp
}
if bidderName, isValid := openrtb_ext.BidderMap[coreBidder]; isValid {
if err := deps.paramsValidator.Validate(bidderName, ext); err != nil {
return fmt.Errorf("request.imp[%d].ext.%s failed validation.\n%v", impIndex, coreBidder, err)
}
} else {
return fmt.Errorf("request.imp[%d].ext contains unknown bidder: %s. Did you forget an alias in request.ext.prebid.aliases?", impIndex, bidder)
}
} else if bidder != "prebid" {
return fmt.Errorf("request.imp[%d].ext contains unknown bidder: %s", impIndex, bidder)
}
}

return nil
}

func (deps *endpointDeps) validateBidRequestExt(ext openrtb.RawJSON) error {
func (deps *endpointDeps) parseBidExt(ext openrtb.RawJSON) (*openrtb_ext.ExtRequest, error) {
if len(ext) < 1 {
return nil
return nil, nil
}
var tmpExt openrtb_ext.ExtRequest
if err := json.Unmarshal(ext, &tmpExt); err != nil {
return fmt.Errorf("request.ext is invalid: %v", err)
return nil, fmt.Errorf("request.ext is invalid: %v", err)
}
return &tmpExt, nil
}

func (deps *endpointDeps) validateAliases(aliases map[string]string) error {
for thisAlias, coreBidder := range aliases {
if _, isCoreBidder := openrtb_ext.BidderMap[coreBidder]; !isCoreBidder {
return fmt.Errorf("request.ext.prebid.aliases.%s refers to unknown bidder: %s", thisAlias, coreBidder)
}
if thisAlias == coreBidder {
return fmt.Errorf("request.ext.prebid.aliases.%s defines a no-op alias. Choose a different alias, or remove this entry.", thisAlias)
}
}
return nil
}
Expand Down
88 changes: 80 additions & 8 deletions endpoints/openrtb2/auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/evanphx/json-patch"
"github.com/mxmCherry/openrtb"
"github.com/prebid/prebid-server/config"
Expand All @@ -13,12 +20,6 @@ import (
"github.com/prebid/prebid-server/pbsmetrics"
"github.com/prebid/prebid-server/stored_requests/backends/empty_fetcher"
"github.com/rcrowley/go-metrics"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
)

const maxSize = 1024 * 256
Expand All @@ -34,8 +35,7 @@ func TestGoodRequests(t *testing.T) {
endpoint(recorder, request, nil)

if recorder.Code != http.StatusOK {
t.Errorf("Expected status %d. Got %d. Request data was %s", http.StatusOK, recorder.Code, requestData)
//t.Errorf("Response body was: %s", recorder.Body)
t.Fatalf("Expected status %d. Got %d. Request data was %s\n\nResponse body was: %s", http.StatusOK, recorder.Code, requestData, recorder.Body.String())
}

var response openrtb.BidResponse
Expand Down Expand Up @@ -666,6 +666,30 @@ var validRequests = []string{
}
]
}`,
`{
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [
{
"id": "my-imp-id",
"video": {
"mimes":["video/mp4"]
},
"ext": {
"unknown": "good"
}
}
],
"ext": {
"prebid": {
"aliases": {
"unknown": "appnexus"
}
}
}
}`,
}

var invalidRequests = []string{
Expand Down Expand Up @@ -873,6 +897,54 @@ var invalidRequests = []string{
}
}
}`,
`{
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [
{
"id": "my-imp-id",
"video": {
"mimes":["video/mp4"]
},
"ext": {
"unknown": "good"
}
}
],
"ext": {
"prebid": {
"aliases": {
"unknown": "other-unknown"
}
}
}
}`,
`{
"id": "some-request-id",
"site": {
"page": "test.somepage.com"
},
"imp": [
{
"id": "my-imp-id",
"video": {
"mimes":["video/mp4"]
},
"ext": {
"appnexus": "good"
}
}
],
"ext": {
"prebid": {
"aliases": {
"appnexus": "appnexus"
}
}
}
}`,
}

// StoredRequest testing
Expand Down
Loading

0 comments on commit b0932cd

Please sign in to comment.