Skip to content

Commit

Permalink
Log non bid reasons in bidder framework (#2891)
Browse files Browse the repository at this point in the history
Co-authored-by: Shriprasad Marathe <shriprasad.marathe@pubmatic.com>
Co-authored-by: ashish.shinde <ashish.shinde@pubmatic.com>
Co-authored-by: dhruv.sonone <dhruv.sonone@pubmatic.com>
  • Loading branch information
4 people committed Sep 19, 2024
1 parent 3c4527e commit 905b3a5
Show file tree
Hide file tree
Showing 11 changed files with 859 additions and 158 deletions.
2 changes: 1 addition & 1 deletion analytics/pubstack/pubstack_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func TestNewModuleSuccess(t *testing.T) {
{
ImpId: "123",
StatusCode: 34,
Ext: openrtb_ext.NonBidExt{Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{}}},
Ext: &openrtb_ext.NonBidExt{Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{}}},
},
},
},
Expand Down
7 changes: 7 additions & 0 deletions exchange/bidder.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,14 @@ type bidRequestOptions struct {

type extraBidderRespInfo struct {
respProcessingStartTime time.Time
seatNonBidBuilder SeatNonBidBuilder
}

type extraAuctionResponseInfo struct {
fledge *openrtb_ext.Fledge
bidsFound bool
bidderResponseStartTime time.Time
seatNonBidBuilder SeatNonBidBuilder
}

const ImpIdReqBody = "Stored bid response for impression id: "
Expand Down Expand Up @@ -135,6 +137,7 @@ type bidderAdapterConfig struct {
func (bidder *bidderAdapter) requestBid(ctx context.Context, bidderRequest BidderRequest, conversions currency.Conversions, reqInfo *adapters.ExtraRequestInfo, adsCertSigner adscert.Signer, bidRequestOptions bidRequestOptions, alternateBidderCodes openrtb_ext.ExtAlternateBidderCodes, hookExecutor hookexecution.StageExecutor, ruleToAdjustments openrtb_ext.AdjustmentsByDealID) ([]*entities.PbsOrtbSeatBid, extraBidderRespInfo, []error) {
request := openrtb_ext.RequestWrapper{BidRequest: bidderRequest.BidRequest}
reject := hookExecutor.ExecuteBidderRequestStage(&request, string(bidderRequest.BidderName))
seatNonBidBuilder := SeatNonBidBuilder{}
if reject != nil {
return nil, extraBidderRespInfo{}, []error{reject}
}
Expand Down Expand Up @@ -398,13 +401,17 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, bidderRequest Bidde
}
} else {
errs = append(errs, httpInfo.err)
nonBidReason := httpInfoToNonBidReason(httpInfo)
seatNonBidBuilder.rejectImps(httpInfo.request.ImpIDs, nonBidReason, string(bidderRequest.BidderName))
}
}

seatBids := make([]*entities.PbsOrtbSeatBid, 0, len(seatBidMap))
for _, seatBid := range seatBidMap {
seatBids = append(seatBids, seatBid)
}

extraRespInfo.seatNonBidBuilder = seatNonBidBuilder
return seatBids, extraRespInfo, errs
}

Expand Down
146 changes: 146 additions & 0 deletions exchange/bidder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@ import (
"errors"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"net/http/httptrace"
"net/url"
"os"
"sort"
"strings"
"syscall"
"testing"
"time"

Expand Down Expand Up @@ -3096,6 +3100,148 @@ func TestGetBidType(t *testing.T) {
}
}

func TestSeatNonBid(t *testing.T) {
type args struct {
BidRequest *openrtb2.BidRequest
Seat string
SeatRequests []*adapters.RequestData
BidderResponse func() (*http.Response, error)
client *http.Client
}
type expect struct {
seatBids []*entities.PbsOrtbSeatBid
seatNonBids SeatNonBidBuilder
errors []error
}
testCases := []struct {
name string
args args
expect expect
}{
{
name: "NBR_101_timeout_for_context_deadline_exceeded",
args: args{
Seat: "pubmatic",
BidRequest: &openrtb2.BidRequest{
Imp: []openrtb2.Imp{{ID: "1234"}},
},
SeatRequests: []*adapters.RequestData{{ImpIDs: []string{"1234"}}},
BidderResponse: func() (*http.Response, error) { return nil, context.DeadlineExceeded },
client: &http.Client{Timeout: time.Nanosecond}, // for timeout
},
expect: expect{
seatNonBids: SeatNonBidBuilder{
"pubmatic": {{
ImpId: "1234",
StatusCode: int(ErrorTimeout),
}},
},
errors: []error{&errortypes.Timeout{Message: context.DeadlineExceeded.Error()}},
seatBids: []*entities.PbsOrtbSeatBid{{Bids: []*entities.PbsOrtbBid{}, Currency: "USD", Seat: "pubmatic", HttpCalls: []*openrtb_ext.ExtHttpCall{}}},
},
}, {
name: "NBR_103_Bidder_Unreachable_Connection_Refused",
args: args{
Seat: "appnexus",
SeatRequests: []*adapters.RequestData{{ImpIDs: []string{"1234", "4567"}}},
BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "1234"}, {ID: "4567"}}},
BidderResponse: func() (*http.Response, error) {
return nil, &net.OpError{Err: os.NewSyscallError(syscall.ECONNREFUSED.Error(), syscall.ECONNREFUSED)}
},
},
expect: expect{
seatNonBids: SeatNonBidBuilder{
"appnexus": {
{ImpId: "1234", StatusCode: int(ErrorBidderUnreachable)},
{ImpId: "4567", StatusCode: int(ErrorBidderUnreachable)},
},
},
seatBids: []*entities.PbsOrtbSeatBid{{Bids: []*entities.PbsOrtbBid{}, Currency: "USD", Seat: "appnexus", HttpCalls: []*openrtb_ext.ExtHttpCall{}}},
errors: []error{&url.Error{Op: "Get", URL: "", Err: &net.OpError{Err: os.NewSyscallError(syscall.ECONNREFUSED.Error(), syscall.ECONNREFUSED)}}},
},
}, {
name: "no_impids_populated_in_request_data",
args: args{
SeatRequests: []*adapters.RequestData{{
ImpIDs: nil, // no imp ids
}},
BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "1234"}}},
BidderResponse: func() (*http.Response, error) {
return nil, errors.New("some_error")
},
},
expect: expect{
seatNonBids: SeatNonBidBuilder{},
seatBids: []*entities.PbsOrtbSeatBid{{Bids: []*entities.PbsOrtbBid{}, Currency: "USD", HttpCalls: []*openrtb_ext.ExtHttpCall{}}},
errors: []error{&url.Error{Op: "Get", URL: "", Err: errors.New("some_error")}},
},
},
}
for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
mockBidder := &mockBidder{}
mockBidder.On("MakeRequests", mock.Anything, mock.Anything).Return(test.args.SeatRequests, []error(nil))
mockMetricsEngine := &metrics.MetricsEngineMock{}
mockMetricsEngine.On("RecordOverheadTime", mock.Anything, mock.Anything).Return(nil)
mockMetricsEngine.On("RecordBidderServerResponseTime", mock.Anything).Return(nil)
roundTrip := &mockRoundTripper{}
roundTrip.On("RoundTrip", mock.Anything).Return(test.args.BidderResponse())
client := &http.Client{
Transport: roundTrip,
Timeout: 0,
}
if test.args.client != nil {
client.Timeout = test.args.client.Timeout
}
bidder := AdaptBidder(mockBidder, client, &config.Configuration{}, mockMetricsEngine, openrtb_ext.BidderAppnexus, &config.DebugInfo{}, test.args.Seat)

ctx := context.Background()
if client.Timeout > 0 {
ctxTimeout, cancel := context.WithTimeout(ctx, client.Timeout)
ctx = ctxTimeout
defer cancel()
}
seatBids, responseExtra, errors := bidder.requestBid(ctx, BidderRequest{
BidRequest: test.args.BidRequest,
BidderName: openrtb_ext.BidderName(test.args.Seat),
}, nil, &adapters.ExtraRequestInfo{}, &MockSigner{}, bidRequestOptions{}, openrtb_ext.ExtAlternateBidderCodes{}, hookexecution.EmptyHookExecutor{}, nil)
assert.Equal(t, test.expect.seatBids, seatBids)
assert.Equal(t, test.expect.seatNonBids, responseExtra.seatNonBidBuilder)
assert.Equal(t, test.expect.errors, errors)
for _, nonBids := range responseExtra.seatNonBidBuilder {
for _, nonBid := range nonBids {
for _, seatBid := range seatBids {
for _, bid := range seatBid.Bids {
// ensure non bids are not present in seat bids
if nonBid.ImpId == bid.Bid.ImpID {
assert.Fail(t, "imp id [%s] present in both seat bid and non seat bid", nonBid.ImpId)
}
}
}
}
}
})
}
}

type mockRoundTripper struct {
mock.Mock
}

func (rt *mockRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
args := rt.Called(request)
var response *http.Response
if args.Get(0) != nil {
response = args.Get(0).(*http.Response)
}
var err error
if args.Get(1) != nil {
err = args.Get(1).(error)
}

return response, err
}

type mockBidderTmaxCtx struct {
startTime, deadline, now time.Time
ok bool
Expand Down
Loading

0 comments on commit 905b3a5

Please sign in to comment.