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

Draft tmax proposal #4

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (cfg *Configuration) validate(v *viper.Viper) []error {

if cfg.TmaxAdjustments.Enabled {
glog.Warning(`cfg.TmaxAdjustments.Enabled will currently not do anything as tmax adjustment feature is still under development.`)
cfg.TmaxAdjustments.Enabled = false
//cfg.TmaxAdjustments.Enabled = false
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is uncommented for testing

}

if cfg.AccountDefaults.Privacy != nil {
Expand Down
8 changes: 7 additions & 1 deletion endpoints/openrtb2/amp_auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func NewAmpEndpoint(
bidderMap map[string]openrtb_ext.BidderName,
storedRespFetcher stored_requests.Fetcher,
hookExecutionPlanBuilder hooks.ExecutionPlanBuilder,
tmaxAdjustments *exchange.TmaxAdjustmentsPreprocessed,
) (httprouter.Handle, error) {

if ex == nil || validator == nil || requestsById == nil || accounts == nil || cfg == nil || metricsEngine == nil {
Expand Down Expand Up @@ -100,7 +101,8 @@ func NewAmpEndpoint(
nil,
ipValidator,
storedRespFetcher,
hookExecutionPlanBuilder}).AmpAuction), nil
hookExecutionPlanBuilder,
tmaxAdjustments}).AmpAuction), nil

}

Expand Down Expand Up @@ -248,6 +250,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
QueryParams: r.URL.Query(),
TCF2Config: tcf2Config,
Activities: activities,
TmaxAdjustments: deps.tmaxAdjustments,
}

auctionResponse, err := deps.ex.HoldAuction(ctx, auctionRequest, nil)
Expand Down Expand Up @@ -284,6 +287,9 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
}

labels, ao = sendAmpResponse(w, hookExecutor, auctionResponse, reqWrapper, account, labels, ao, errL)
if len(ao.Errors) == 0 && !auctionRequest.BidderResponseStartTime.IsZero() {
deps.metricsEngine.RecordOverheadTime(metrics.MakeAuctionResponse, time.Since(auctionRequest.BidderResponseStartTime))
}
}

func rejectAmpRequest(
Expand Down
9 changes: 8 additions & 1 deletion endpoints/openrtb2/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func NewEndpoint(
bidderMap map[string]openrtb_ext.BidderName,
storedRespFetcher stored_requests.Fetcher,
hookExecutionPlanBuilder hooks.ExecutionPlanBuilder,
tmaxAdjustments *exchange.TmaxAdjustmentsPreprocessed,
) (httprouter.Handle, error) {
if ex == nil || validator == nil || requestsById == nil || accounts == nil || cfg == nil || metricsEngine == nil {
return nil, errors.New("NewEndpoint requires non-nil arguments.")
Expand Down Expand Up @@ -122,7 +123,8 @@ func NewEndpoint(
nil,
ipValidator,
storedRespFetcher,
hookExecutionPlanBuilder}).Auction), nil
hookExecutionPlanBuilder,
tmaxAdjustments}).Auction), nil
}

type endpointDeps struct {
Expand All @@ -144,6 +146,7 @@ type endpointDeps struct {
privateNetworkIPValidator iputil.IPValidator
storedRespFetcher stored_requests.Fetcher
hookExecutionPlanBuilder hooks.ExecutionPlanBuilder
tmaxAdjustments *exchange.TmaxAdjustmentsPreprocessed
}

func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
Expand Down Expand Up @@ -246,6 +249,7 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http
HookExecutor: hookExecutor,
TCF2Config: tcf2Config,
Activities: activities,
TmaxAdjustments: deps.tmaxAdjustments,
}
auctionResponse, err := deps.ex.HoldAuction(ctx, auctionRequest, nil)
ao.RequestWrapper = req
Expand Down Expand Up @@ -279,6 +283,9 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http
glog.Errorf("Error setting seat non-bid: %v", err)
}
labels, ao = sendAuctionResponse(w, hookExecutor, response, req.BidRequest, account, labels, ao)
if len(ao.Errors) == 0 && !auctionRequest.BidderResponseStartTime.IsZero() {
deps.metricsEngine.RecordOverheadTime(metrics.MakeAuctionResponse, time.Since(auctionRequest.BidderResponseStartTime))
}
}

// setSeatNonBidRaw is transitional function for setting SeatNonBid inside bidResponse.Ext
Expand Down
3 changes: 2 additions & 1 deletion endpoints/openrtb2/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,7 @@ func buildTestEndpoint(test testCase, cfg *config.Configuration) (httprouter.Han
planBuilder = hooks.EmptyPlanBuilder{}
}

var endpointBuilder func(uuidutil.UUIDGenerator, exchange.Exchange, openrtb_ext.BidderParamValidator, stored_requests.Fetcher, stored_requests.AccountFetcher, *config.Configuration, metrics.MetricsEngine, analytics.PBSAnalyticsModule, map[string]string, []byte, map[string]openrtb_ext.BidderName, stored_requests.Fetcher, hooks.ExecutionPlanBuilder) (httprouter.Handle, error)
var endpointBuilder func(uuidutil.UUIDGenerator, exchange.Exchange, openrtb_ext.BidderParamValidator, stored_requests.Fetcher, stored_requests.AccountFetcher, *config.Configuration, metrics.MetricsEngine, analytics.PBSAnalyticsModule, map[string]string, []byte, map[string]openrtb_ext.BidderName, stored_requests.Fetcher, hooks.ExecutionPlanBuilder, *exchange.TmaxAdjustmentsPreprocessed) (httprouter.Handle, error)

switch test.endpointType {
case AMP_ENDPOINT:
Expand All @@ -1313,6 +1313,7 @@ func buildTestEndpoint(test testCase, cfg *config.Configuration) (httprouter.Han
bidderMap,
storedResponseFetcher,
planBuilder,
nil,
)

return endpoint, testExchange.(*exchangeTestWrapper), mockBidServersArray, mockCurrencyRatesServer, err
Expand Down
9 changes: 8 additions & 1 deletion endpoints/openrtb2/video_auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func NewVideoEndpoint(
defReqJSON []byte,
bidderMap map[string]openrtb_ext.BidderName,
cache prebid_cache_client.Client,
tmaxAdjustments *exchange.TmaxAdjustmentsPreprocessed,
) (httprouter.Handle, error) {

if ex == nil || validator == nil || requestsById == nil || accounts == nil || cfg == nil || met == nil {
Expand Down Expand Up @@ -89,7 +90,8 @@ func NewVideoEndpoint(
videoEndpointRegexp,
ipValidator,
empty_fetcher.EmptyFetcher{},
hooks.EmptyPlanBuilder{}}).VideoAuctionEndpoint), nil
hooks.EmptyPlanBuilder{},
tmaxAdjustments}).VideoAuctionEndpoint), nil
}

/*
Expand Down Expand Up @@ -307,6 +309,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
GlobalPrivacyControlHeader: secGPC,
PubID: labels.PubID,
HookExecutor: hookexecution.EmptyHookExecutor{},
TmaxAdjustments: deps.tmaxAdjustments,
}

auctionResponse, err := deps.ex.HoldAuction(ctx, auctionRequest, &debugLog)
Expand Down Expand Up @@ -363,6 +366,10 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
return
}

if len(vo.Errors) == 0 && !auctionRequest.BidderResponseStartTime.IsZero() {
deps.metricsEngine.RecordOverheadTime(metrics.MakeAuctionResponse, time.Since(auctionRequest.BidderResponseStartTime))
}

w.Header().Set("Content-Type", "application/json")
w.Write(resp)

Expand Down
6 changes: 6 additions & 0 deletions exchange/bidder.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type bidRequestOptions struct {
headerDebugAllowed bool
addCallSignHeader bool
bidAdjustments map[string]float64
tmaxAdjustments *TmaxAdjustmentsPreprocessed
}

type extraBidderRespInfo struct {
Expand Down Expand Up @@ -143,6 +144,11 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, bidderRequest Bidde
//check if real request exists for this bidder or it only has stored responses
dataLen := 0
if len(bidderRequest.BidRequest.Imp) > 0 {
// Reducing the amount of time bidders have to compensate for the processing time used by PBS to fetch a stored request (if needed), validate the OpenRTB request and split it into multiple requests sanitized for each bidder
// As well as for the time needed by PBS to prepare the auction response
if bidRequestOptions.tmaxAdjustments != nil && bidRequestOptions.tmaxAdjustments.BidderResponseDurationMin != 0 {
bidderRequest.BidRequest.TMax = getBidderTmax(&bidderTmaxCtx{ctx}, bidderRequest.BidRequest.TMax, bidRequestOptions.tmaxAdjustments)
}
reqData, errs = bidder.Bidder.MakeRequests(bidderRequest.BidRequest, reqInfo)

if len(reqData) == 0 {
Expand Down
145 changes: 145 additions & 0 deletions exchange/bidder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3063,3 +3063,148 @@ func TestGetBidType(t *testing.T) {
})
}
}

type mockBidderTmaxCtx struct {
startTime, deadline time.Time
ok bool
}

func (m *mockBidderTmaxCtx) Deadline() (deadline time.Time, _ bool) {
return m.deadline, m.ok
}
func (m *mockBidderTmaxCtx) RemainingDurationMS(deadline time.Time) int64 {
return deadline.Sub(m.startTime).Milliseconds()
}

func TestGetBidderTmax(t *testing.T) {
var (
requestTmaxMS int64 = 700
bidderNetworkLatencyBuffer uint = 50
responsePreparationDuration uint = 60
)
requestTmaxNS := requestTmaxMS * int64(time.Millisecond)
startTime := time.Date(2023, 5, 30, 1, 0, 0, 0, time.UTC)
deadline := time.Date(2023, 5, 30, 1, 0, 0, int(requestTmaxNS), time.UTC)
ctx := &mockBidderTmaxCtx{startTime: startTime, deadline: deadline, ok: true}
tests := []struct {
description string
ctx bidderTmaxContext
requestTmax int64
expectedTmax int64
tmaxAdjustments config.TmaxAdjustments
}{
{
description: "returns-requestTmax-when-tmax-adjustment-is-not-enabled",
ctx: ctx,
requestTmax: requestTmaxMS,
tmaxAdjustments: config.TmaxAdjustments{Enabled: false},
expectedTmax: requestTmaxMS,
},
{
description: "returns-requestTmax-when-BidderResponseDurationMin-is-not-set",
ctx: ctx,
requestTmax: requestTmaxMS,
tmaxAdjustments: config.TmaxAdjustments{Enabled: true, BidderResponseDurationMin: 0},
expectedTmax: requestTmaxMS,
},
{
description: "returns-requestTmax-when-BidderNetworkLatencyBuffer-and-PBSResponsePreparationDuration-is-not-set",
ctx: ctx,
requestTmax: requestTmaxMS,
tmaxAdjustments: config.TmaxAdjustments{Enabled: true, BidderResponseDurationMin: 100, BidderNetworkLatencyBuffer: 0, PBSResponsePreparationDuration: 0},
expectedTmax: requestTmaxMS,
},
{
description: "returns-requestTmax-when-context-deadline-is-not-set",
ctx: &mockBidderTmaxCtx{ok: false},
requestTmax: requestTmaxMS,
tmaxAdjustments: config.TmaxAdjustments{Enabled: true, BidderResponseDurationMin: 100, BidderNetworkLatencyBuffer: 50, PBSResponsePreparationDuration: 60},
expectedTmax: requestTmaxMS,
},
{
description: "returns-remaing-duration-by-subtracting-BidderNetworkLatencyBuffer-and-PBSResponsePreparationDuration",
ctx: ctx,
requestTmax: requestTmaxMS,
tmaxAdjustments: config.TmaxAdjustments{Enabled: true, BidderResponseDurationMin: 100, BidderNetworkLatencyBuffer: bidderNetworkLatencyBuffer, PBSResponsePreparationDuration: responsePreparationDuration},
expectedTmax: ctx.RemainingDurationMS(deadline) - int64(bidderNetworkLatencyBuffer) - int64(responsePreparationDuration),
},
}
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
assert.Equal(t, test.expectedTmax, getBidderTmax(test.ctx, test.requestTmax, test.tmaxAdjustments))

})
}
}

func TestUpdateBidderTmax(t *testing.T) {
respStatus := 200
respBody := "{\"bid\":false}"
server := httptest.NewServer(mockHandler(respStatus, "getBody", respBody))
defer server.Close()

requestHeaders := http.Header{}
requestHeaders.Add("Content-Type", "application/json")

currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))
var requestTmax int64 = 700

bidderReq := BidderRequest{
BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}, TMax: requestTmax},
BidderName: "test",
}
extraInfo := &adapters.ExtraRequestInfo{}

tests := []struct {
description string
requestTmax int64
tmaxAdjustments *config.TmaxAdjustments
assertFn func(actualTmax int64) bool
}{
{
description: "tmax-is-not-enabled",
requestTmax: requestTmax,
tmaxAdjustments: &config.TmaxAdjustments{Enabled: false},
assertFn: func(actualTmax int64) bool {
return requestTmax == actualTmax
},
},
{
description: "bidder-response-duration-not-set",
requestTmax: 700,
tmaxAdjustments: &config.TmaxAdjustments{Enabled: true, BidderResponseDurationMin: 0},
assertFn: func(actualTmax int64) bool {
return requestTmax == actualTmax
},
},
{
description: "updates-bidder-tmax",
requestTmax: requestTmax,
tmaxAdjustments: &config.TmaxAdjustments{Enabled: true, BidderResponseDurationMin: 100, BidderNetworkLatencyBuffer: 50, PBSResponsePreparationDuration: 50},
assertFn: func(actualTmax int64) bool {
return requestTmax > actualTmax
},
},
}
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
bidderImpl := &goodSingleBidder{
httpRequest: &adapters.RequestData{
Method: "POST",
Uri: server.URL,
Body: []byte("{\"key\":\"val\"}"),
Headers: http.Header{},
},
bidResponse: &adapters.BidderResponse{},
}

ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(50*time.Millisecond))
defer cancel()
bidReqOptions := bidRequestOptions{tmaxAdjustments: test.tmaxAdjustments}
bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, &config.DebugInfo{Allow: false}, "")
_, _, errs := bidder.requestBid(ctx, bidderReq, currencyConverter.Rates(), extraInfo, &adscert.NilSigner{}, bidReqOptions, openrtb_ext.ExtAlternateBidderCodes{}, &hookexecution.EmptyHookExecutor{}, nil)
assert.Empty(t, errs)
assert.True(t, test.assertFn(bidderImpl.bidRequest.TMax))
})
}
}
19 changes: 12 additions & 7 deletions exchange/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,13 @@ type AuctionRequest struct {
// map of imp id to stored response
StoredAuctionResponses stored_responses.ImpsWithBidResponses
// map of imp id to bidder to stored response
StoredBidResponses stored_responses.ImpBidderStoredResp
BidderImpReplaceImpID stored_responses.BidderImpReplaceImpID
PubID string
HookExecutor hookexecution.StageExecutor
QueryParams url.Values
StoredBidResponses stored_responses.ImpBidderStoredResp
BidderImpReplaceImpID stored_responses.BidderImpReplaceImpID
PubID string
HookExecutor hookexecution.StageExecutor
QueryParams url.Values
BidderResponseStartTime time.Time
TmaxAdjustments *TmaxAdjustmentsPreprocessed
}

// BidderRequest holds the bidder specific request and all other
Expand Down Expand Up @@ -367,9 +369,10 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog
alternateBidderCodes = *r.Account.AlternateBidderCodes
}
var extraRespInfo extraAuctionResponseInfo
adapterBids, adapterExtra, extraRespInfo = e.getAllBids(auctionCtx, bidderRequests, bidAdjustmentFactors, conversions, accountDebugAllow, r.GlobalPrivacyControlHeader, debugLog.DebugOverride, alternateBidderCodes, requestExtLegacy.Prebid.Experiment, r.HookExecutor, r.StartTime, bidAdjustmentRules)
adapterBids, adapterExtra, extraRespInfo = e.getAllBids(auctionCtx, bidderRequests, bidAdjustmentFactors, conversions, accountDebugAllow, r.GlobalPrivacyControlHeader, debugLog.DebugOverride, alternateBidderCodes, requestExtLegacy.Prebid.Experiment, r.HookExecutor, r.StartTime, bidAdjustmentRules, r.TmaxAdjustments)
fledge = extraRespInfo.fledge
anyBidsReturned = extraRespInfo.bidsFound
r.BidderResponseStartTime = extraRespInfo.bidderResponseStartTime
}

var (
Expand Down Expand Up @@ -658,7 +661,8 @@ func (e *exchange) getAllBids(
experiment *openrtb_ext.Experiment,
hookExecutor hookexecution.StageExecutor,
pbsRequestStartTime time.Time,
bidAdjustmentRules map[string][]openrtb_ext.Adjustment) (
bidAdjustmentRules map[string][]openrtb_ext.Adjustment,
tmaxAdjustments *TmaxAdjustmentsPreprocessed) (
map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid,
map[openrtb_ext.BidderName]*seatResponseExtra,
extraAuctionResponseInfo) {
Expand Down Expand Up @@ -697,6 +701,7 @@ func (e *exchange) getAllBids(
headerDebugAllowed: headerDebugAllowed,
addCallSignHeader: isAdsCertEnabled(experiment, e.bidderInfo[string(bidderRequest.BidderName)]),
bidAdjustments: bidAdjustments,
tmaxAdjustments: tmaxAdjustments,
}
seatBids, extraBidderRespInfo, err := e.adapterMap[bidderRequest.BidderCoreName].requestBid(ctx, bidderRequest, conversions, &reqInfo, e.adsCertSigner, bidReqOptions, alternateBidderCodes, hookExecutor, bidAdjustmentRules)
brw.bidderResponseStartTime = extraBidderRespInfo.respProcessingStartTime
Expand Down
Loading