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.

Sign up for GitHub

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

Debug disable feature implementation: #1677

Merged
merged 11 commits into from
Jan 28, 2021
Merged

Debug disable feature implementation: #1677

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a5886cd
Debug disable feature implementation:
Jan 22, 2021
2e05c58
Unit tests fix
Jan 22, 2021
4c7d53a
Possible cache fix
Jan 23, 2021
c7ca474
Added unit tests
Jan 25, 2021
f485731
Code review fixe: removed account level debug variable from context
Jan 26, 2021
9821fd3
Code review fix
Jan 26, 2021
5f1e090
Moved bidder level debug struct to adapters/info.go
Jan 26, 2021
fdbe72f
Code review fixes
Jan 26, 2021
6c64aa8
Cache fix
Jan 26, 2021
d40309a
Minor refactoring
Jan 27, 2021
e482baf
Code refactoring and bugfix
Jan 28, 2021
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
1 change: 1 addition & 0 deletions adapters/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ type BidderInfo struct {
Capabilities *CapabilitiesInfo `yaml:"capabilities" json:"capabilities"`
AliasOf string `json:"aliasOf,omitempty"`
ModifyingVastXmlAllowed bool `yaml:"modifyingVastXmlAllowed" json:"-" xml:"-"`
DebugInfo *config.DebugInfo `yaml:"debug,omitempty" json:"debug,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

The json must be json:"-" xml:"-" to play nicely with the /info/bidders endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, didn't know about this, fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Would calling it Debug instead of DebugInfo cause a naming conflict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed, no conflicts

}

type MaintainerInfo struct {
Expand Down
1 change: 1 addition & 0 deletions config/accounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type Account struct {
EventsEnabled bool `mapstructure:"events_enabled" json:"events_enabled"`
CCPA AccountCCPA `mapstructure:"ccpa" json:"ccpa"`
GDPR AccountGDPR `mapstructure:"gdpr" json:"gdpr"`
DebugAllowed bool `mapstructure:"debugallowed" json:"debugallowed"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call the JSON debug_allowed to match the other naming conventions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified!

}

// AccountCCPA represents account-specific CCPA configuration
Expand Down
5 changes: 5 additions & 0 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,10 @@ type Debug struct {
TimeoutNotification TimeoutNotification `mapstructure:"timeout_notification"`
}

type DebugInfo struct {
Allow bool `yaml:"allow" json:"allow"`
Copy link
Collaborator

@hhhjort hhhjort Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The default in Go for a boolean is false, but this is okay since it is inside of a pointer to an object that will be empty if not specified. This does result in this DebugInfo object in the Bidder infos to be optional, which does look a bit strange as server generated info output. Maybe we want to consider a DebugRestricted field instead of DebugInfo.Allow, or even just a pointer to a bool so we can detect the difference to a missing setting vs. and explicit false value.

Do we need to advertise this in the bidder info data? You can hide this in the bidder config, which seems a more logical place to house this data, and avoids the issue of the bidder infos being inconsistent on if they include an "optional" parameter. Also this is a setting that a PBS host would be expected to set, are there any other parameters in the bidder info that a PBS host might touch? That would argue that it goes in the bidder config if not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SyntaxNode please advice if we want to have DebugRestricted instead of DebugInfo.Allow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, moved to adapters/info.go, good catch!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you missed my point. I was thinking of moving this flag out of the BidderInfo struct, and thus out of the bidder/info endpoint, and into the actual server configuration in the Adapter struct, where we actually do sever level configuration of the adapter. I am not aware of anything in the /static/bidder-info/ directory that a PBS host should touch, all configuration should be driven by the config file and/or environment variables. Spreading configuration out across multiple files will just lead to confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm confused.
We want to have bidder level debug flag and store it in static/bidder-info/{bidder}.yaml files. The only place where we load it is in adapters/info.go -> func ParseBidderInfos. We Unmarshal file data to BidderInfo struct.
Unless I miss something...
Do you mean I need to move it to config/adapter.go -> Adapter struct after loading?
What is "the actual server configuration in the Adapter struct"?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SyntaxNode please advice if we want to have DebugRestricted instead of DebugInfo.Allow

I don't have a strong opinion. Bret introduced this config as debug allow and that makes sense to me, but yes - it does play against the boolean zero value. We'd need to ask in the GitHub issues to swap it to something like restricted.

I was thinking of moving this flag out of the BidderInfo struct, and thus out of the bidder/info endpoint, and into the actual server configuration in the Adapter struct,

I think both have valid arguments. I had spec'd it for the bidder yaml since I didn't expect a host to override this value. I thought if the bidder knows its url will include sensitive information, then it could specify such here for all hosts to follow.

We've also had discussions about in the future using bidder yaml as the base config for each adapter as is done in PBS-Java and then using the config system to override those values if needed. This would work well for reducing the code length of config.go, making it easier to port adapters to PBS-Java, and would provide a nice way to override yaml information for aliases.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, if PBS-Java is already using it for config, then it is not such a big change to for us to do the same. I have no opinion on if this struct should be in config/config.go or adapters/info.go

}

func (cfg *Debug) validate(errs []error) []error {
return cfg.TimeoutNotification.validate(errs)
}
Expand Down Expand Up @@ -937,6 +941,7 @@ func SetupViper(v *viper.Viper, filename string) {
v.SetDefault("blacklisted_accts", []string{""})
v.SetDefault("account_required", false)
v.SetDefault("account_defaults.disabled", false)
v.SetDefault("account_defaults.debugallowed", true)
v.SetDefault("certificates_file", "")
v.SetDefault("auto_gen_source_tid", true)

Expand Down
11 changes: 11 additions & 0 deletions endpoints/openrtb2/video_auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,17 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
return
}

ctx = context.WithValue(ctx, exchange.DebugAllowedContextKey, account.DebugAllowed)
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't like the context. Is there a way we can do without this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored. We have this variable in account. To have this in bidder -> requestBid I passed it through function param from HoldAuction


/*if debugLog.Enabled && account.DebugAllowed {
debugLog.Data.Request = string(requestJson)
if headerBytes, err := json.Marshal(r.Header); err == nil {
debugLog.Data.Headers = string(headerBytes)
} else {
debugLog.Data.Headers = fmt.Sprintf("Unable to marshal headers data: %s", err.Error())
}
}*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed? Are the request and header fields added elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this commented block of code is moved to line 160. It's not removed.


auctionRequest := exchange.AuctionRequest{
BidRequest: bidReq,
Account: *account,
Expand Down
2 changes: 1 addition & 1 deletion exchange/adapter_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func buildExchangeBidders(cfg *config.Configuration, infos adapters.BidderInfos,

exchangeBidders := make(map[openrtb_ext.BidderName]adaptedBidder, len(bidders))
for bidderName, bidder := range bidders {
exchangeBidders[bidderName] = adaptBidder(bidder, client, cfg, me, bidderName)
exchangeBidders[bidderName] = adaptBidder(bidder, client, cfg, me, bidderName, infos[bidderName.String()].DebugInfo)
}

return exchangeBidders, nil
Expand Down
6 changes: 3 additions & 3 deletions exchange/adapter_util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestBuildAdaptersSuccess(t *testing.T) {

appnexusBidder, _ := appnexus.Builder(openrtb_ext.BidderAppnexus, config.Adapter{})
appnexusBidderWithInfo := adapters.EnforceBidderInfo(appnexusBidder, infoActive)
appnexusBidderAdapted := adaptBidder(appnexusBidderWithInfo, client, &config.Configuration{}, metricEngine, openrtb_ext.BidderAppnexus)
appnexusBidderAdapted := adaptBidder(appnexusBidderWithInfo, client, &config.Configuration{}, metricEngine, openrtb_ext.BidderAppnexus, nil)
appnexusBidderValidated := addValidatedBidderMiddleware(appnexusBidderAdapted)

idLegacyAdapted := &adaptedAdapter{lifestreet.NewLifestreetLegacyAdapter(adapters.DefaultHTTPAdapterConfig, "anyEndpoint")}
Expand Down Expand Up @@ -78,11 +78,11 @@ func TestBuildExchangeBidders(t *testing.T) {

appnexusBidder, _ := appnexus.Builder(openrtb_ext.BidderAppnexus, config.Adapter{})
appnexusBidderWithInfo := adapters.EnforceBidderInfo(appnexusBidder, infoActive)
appnexusBidderAdapted := adaptBidder(appnexusBidderWithInfo, client, &config.Configuration{}, metricEngine, openrtb_ext.BidderAppnexus)
appnexusBidderAdapted := adaptBidder(appnexusBidderWithInfo, client, &config.Configuration{}, metricEngine, openrtb_ext.BidderAppnexus, nil)

rubiconBidder, _ := rubicon.Builder(openrtb_ext.BidderRubicon, config.Adapter{})
rubiconBidderWithInfo := adapters.EnforceBidderInfo(rubiconBidder, infoActive)
rubiconBidderAdapted := adaptBidder(rubiconBidderWithInfo, client, &config.Configuration{}, metricEngine, openrtb_ext.BidderRubicon)
rubiconBidderAdapted := adaptBidder(rubiconBidderWithInfo, client, &config.Configuration{}, metricEngine, openrtb_ext.BidderRubicon, nil)

testCases := []struct {
description string
Expand Down
25 changes: 23 additions & 2 deletions exchange/bidder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ type pbsOrtbSeatBid struct {
//
// The name refers to the "Adapter" architecture pattern, and should not be confused with a Prebid "Adapter"
// (which is being phased out and replaced by Bidder for OpenRTB auctions)
func adaptBidder(bidder adapters.Bidder, client *http.Client, cfg *config.Configuration, me metrics.MetricsEngine, name openrtb_ext.BidderName) adaptedBidder {
func adaptBidder(bidder adapters.Bidder, client *http.Client, cfg *config.Configuration, me metrics.MetricsEngine, name openrtb_ext.BidderName, debugInfo *config.DebugInfo) adaptedBidder {
return &bidderAdapter{
Bidder: bidder,
BidderName: name,
Expand All @@ -102,10 +102,22 @@ func adaptBidder(bidder adapters.Bidder, client *http.Client, cfg *config.Config
config: bidderAdapterConfig{
Debug: cfg.Debug,
DisableConnMetrics: cfg.Metrics.Disabled.AdapterConnectionMetrics,
DebugInfo: parseDebugInfo(debugInfo),
},
}
}

func parseDebugInfo(info *config.DebugInfo) config.DebugInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor point, but perhaps we just want to return the final boolean value here, rather than a full config.DebugInfo. Is there any benefit in keeping it packed inside of the the struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fixed

var confDebugInfo config.DebugInfo

if info == nil {
confDebugInfo.Allow = true
} else {
confDebugInfo.Allow = info.Allow
}
return confDebugInfo
}

type bidderAdapter struct {
Bidder adapters.Bidder
BidderName openrtb_ext.BidderName
Expand All @@ -117,6 +129,7 @@ type bidderAdapter struct {
type bidderAdapterConfig struct {
Debug config.Debug
DisableConnMetrics bool
DebugInfo config.DebugInfo
}

func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb.BidRequest, name openrtb_ext.BidderName, bidAdjustment float64, conversions currency.Conversions, reqInfo *adapters.ExtraRequestInfo) (*pbsOrtbSeatBid, []error) {
Expand Down Expand Up @@ -155,8 +168,16 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb.Bi
for i := 0; i < len(reqData); i++ {
httpInfo := <-responseChannel
// If this is a test bid, capture debugging info from the requests.
// Write debug data to ext in case if:
// - debugContextKey (url param) in true
// - account debug is allowed
// - bidder debug is allowed
if debugInfo := ctx.Value(DebugContextKey); debugInfo != nil && debugInfo.(bool) {
seatBid.httpCalls = append(seatBid.httpCalls, makeExt(httpInfo))
if debugAllowed := ctx.Value(DebugAllowedContextKey); debugAllowed != nil && debugAllowed.(bool) {
if bidder.config.DebugInfo.Allow {
seatBid.httpCalls = append(seatBid.httpCalls, makeExt(httpInfo))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you combine the if clauses to improve readability?

if accountDebugAllowed && bidder.config.DebugInfo.Allow {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, was going to to this too, done! :)

}

if httpInfo.err == nil {
Expand Down
168 changes: 103 additions & 65 deletions exchange/bidder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ import (
// 1. The Bidder implementation is called with the arguments we expect.
// 2. The returned values are correct for a non-test bid.
func TestSingleBidder(t *testing.T) {
type aTest struct {
debugInfo *config.DebugInfo
httpCallsLen int
}

testCases := []*aTest{
{&config.DebugInfo{Allow: false}, 0},
{&config.DebugInfo{Allow: true}, 1},
}

respStatus := 200
respBody := "{\"bid\":false}"
server := httptest.NewServer(mockHandler(respStatus, "getBody", respBody))
Expand All @@ -46,24 +56,6 @@ func TestSingleBidder(t *testing.T) {
bidAdjustment := 2.0
firstInitialPrice := 3.0
secondInitialPrice := 4.0
mockBidderResponse := &adapters.BidderResponse{
Bids: []*adapters.TypedBid{
{
Bid: &openrtb.Bid{
Price: firstInitialPrice,
},
BidType: openrtb_ext.BidTypeBanner,
DealPriority: 4,
},
{
Bid: &openrtb.Bid{
Price: secondInitialPrice,
},
BidType: openrtb_ext.BidTypeVideo,
DealPriority: 5,
},
},
}

bidderImpl := &goodSingleBidder{
httpRequest: &adapters.RequestData{
Expand All @@ -72,53 +64,82 @@ func TestSingleBidder(t *testing.T) {
Body: []byte("{\"key\":\"val\"}"),
Headers: http.Header{},
},
bidResponse: mockBidderResponse,
bidResponse: nil,
}
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))
seatBid, errs := bidder.requestBid(context.Background(), &openrtb.BidRequest{}, "test", bidAdjustment, currencyConverter.Rates(), &adapters.ExtraRequestInfo{})

// Make sure the goodSingleBidder was called with the expected arguments.
if bidderImpl.httpResponse == nil {
t.Errorf("The Bidder should be called with the server's response.")
}
if bidderImpl.httpResponse.StatusCode != respStatus {
t.Errorf("Bad response status. Expected %d, got %d", respStatus, bidderImpl.httpResponse.StatusCode)
}
if string(bidderImpl.httpResponse.Body) != respBody {
t.Errorf("Bad response body. Expected %s, got %s", respBody, string(bidderImpl.httpResponse.Body))
}
ctx := context.Background()
ctx = context.WithValue(ctx, DebugAllowedContextKey, true)
ctx = context.WithValue(ctx, DebugContextKey, true)

// Make sure the returned values are what we expect
if len(errs) != 0 {
t.Errorf("bidder.Bid returned %d errors. Expected 0", len(errs))
}
if len(seatBid.bids) != len(mockBidderResponse.Bids) {
t.Fatalf("Expected %d bids. Got %d", len(mockBidderResponse.Bids), len(seatBid.bids))
}
for index, typedBid := range mockBidderResponse.Bids {
if typedBid.Bid != seatBid.bids[index].bid {
t.Errorf("Bid %d did not point to the same bid returned by the Bidder.", index)
for _, test := range testCases {

mockBidderResponse := &adapters.BidderResponse{
Bids: []*adapters.TypedBid{
{
Bid: &openrtb.Bid{
Price: firstInitialPrice,
},
BidType: openrtb_ext.BidTypeBanner,
DealPriority: 4,
},
{
Bid: &openrtb.Bid{
Price: secondInitialPrice,
},
BidType: openrtb_ext.BidTypeVideo,
DealPriority: 5,
},
},
}
if typedBid.BidType != seatBid.bids[index].bidType {
t.Errorf("Bid %d did not have the right type. Expected %s, got %s", index, typedBid.BidType, seatBid.bids[index].bidType)
bidderImpl.bidResponse = mockBidderResponse

bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, test.debugInfo)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))

seatBid, errs := bidder.requestBid(ctx, &openrtb.BidRequest{}, "test", bidAdjustment, currencyConverter.Rates(), &adapters.ExtraRequestInfo{})

// Make sure the goodSingleBidder was called with the expected arguments.
if bidderImpl.httpResponse == nil {
t.Errorf("The Bidder should be called with the server's response.")
}
if typedBid.DealPriority != seatBid.bids[index].dealPriority {
t.Errorf("Bid %d did not have the right deal priority. Expected %s, got %s", index, typedBid.BidType, seatBid.bids[index].bidType)
if bidderImpl.httpResponse.StatusCode != respStatus {
t.Errorf("Bad response status. Expected %d, got %d", respStatus, bidderImpl.httpResponse.StatusCode)
}
if string(bidderImpl.httpResponse.Body) != respBody {
t.Errorf("Bad response body. Expected %s, got %s", respBody, string(bidderImpl.httpResponse.Body))
}
}
if mockBidderResponse.Bids[0].Bid.Price != bidAdjustment*firstInitialPrice {
t.Errorf("Bid[0].Price was not adjusted properly. Expected %f, got %f", bidAdjustment*firstInitialPrice, mockBidderResponse.Bids[0].Bid.Price)
}
if mockBidderResponse.Bids[1].Bid.Price != bidAdjustment*secondInitialPrice {
t.Errorf("Bid[1].Price was not adjusted properly. Expected %f, got %f", bidAdjustment*secondInitialPrice, mockBidderResponse.Bids[1].Bid.Price)
}
if len(seatBid.httpCalls) != 0 {
t.Errorf("The bidder shouldn't log HttpCalls when request.test == 0. Found %d", len(seatBid.httpCalls))
}

if len(seatBid.ext) != 0 {
t.Errorf("The bidder shouldn't define any seatBid.ext. Got %s", string(seatBid.ext))
// Make sure the returned values are what we expect
if len(errs) != 0 {
t.Errorf("bidder.Bid returned %d errors. Expected 0", len(errs))
}
if len(seatBid.bids) != len(mockBidderResponse.Bids) {
t.Fatalf("Expected %d bids. Got %d", len(mockBidderResponse.Bids), len(seatBid.bids))
}
for index, typedBid := range mockBidderResponse.Bids {
if typedBid.Bid != seatBid.bids[index].bid {
t.Errorf("Bid %d did not point to the same bid returned by the Bidder.", index)
}
if typedBid.BidType != seatBid.bids[index].bidType {
t.Errorf("Bid %d did not have the right type. Expected %s, got %s", index, typedBid.BidType, seatBid.bids[index].bidType)
}
if typedBid.DealPriority != seatBid.bids[index].dealPriority {
t.Errorf("Bid %d did not have the right deal priority. Expected %s, got %s", index, typedBid.BidType, seatBid.bids[index].bidType)
}
}
if mockBidderResponse.Bids[0].Bid.Price != bidAdjustment*firstInitialPrice {
t.Errorf("Bid[0].Price was not adjusted properly. Expected %f, got %f", bidAdjustment*firstInitialPrice, mockBidderResponse.Bids[0].Bid.Price)
}
if mockBidderResponse.Bids[1].Bid.Price != bidAdjustment*secondInitialPrice {
t.Errorf("Bid[1].Price was not adjusted properly. Expected %f, got %f", bidAdjustment*secondInitialPrice, mockBidderResponse.Bids[1].Bid.Price)
}
if len(seatBid.httpCalls) != test.httpCallsLen {
t.Errorf("The bidder shouldn't log HttpCalls when request.test == 0. Found %d", len(seatBid.httpCalls))
}

if len(seatBid.ext) != 0 {
t.Errorf("The bidder shouldn't define any seatBid.ext. Got %s", string(seatBid.ext))
}
}
}

Expand Down Expand Up @@ -162,7 +183,7 @@ func TestMultiBidder(t *testing.T) {
}},
bidResponse: mockBidderResponse,
}
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))
seatBid, errs := bidder.requestBid(context.Background(), &openrtb.BidRequest{}, "test", 1.0, currencyConverter.Rates(), &adapters.ExtraRequestInfo{})

Expand Down Expand Up @@ -524,7 +545,7 @@ func TestMultiCurrencies(t *testing.T) {
)

// Execute:
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(
&http.Client{},
mockedHTTPServer.URL,
Expand Down Expand Up @@ -675,7 +696,7 @@ func TestMultiCurrencies_RateConverterNotSet(t *testing.T) {
}

// Execute:
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))
seatBid, errs := bidder.requestBid(
context.Background(),
Expand Down Expand Up @@ -841,7 +862,7 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) {
}

// Execute:
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(
&http.Client{},
mockedHTTPServer.URL,
Expand Down Expand Up @@ -1020,7 +1041,7 @@ func TestMobileNativeTypes(t *testing.T) {
},
bidResponse: tc.mockBidderResponse,
}
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))

seatBids, _ := bidder.requestBid(
Expand All @@ -1041,7 +1062,7 @@ func TestMobileNativeTypes(t *testing.T) {
}

func TestErrorReporting(t *testing.T) {
bidder := adaptBidder(&bidRejector{}, nil, &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(&bidRejector{}, nil, &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))
bids, errs := bidder.requestBid(context.Background(), &openrtb.BidRequest{}, "test", 1.0, currencyConverter.Rates(), &adapters.ExtraRequestInfo{})
if bids != nil {
Expand Down Expand Up @@ -1224,7 +1245,7 @@ func TestCallRecordAdapterConnections(t *testing.T) {
metrics.On("RecordAdapterConnections", expectedAdapterName, false, mock.MatchedBy(compareConnWaitTime)).Once()

// Run requestBid using an http.Client with a mock handler
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, metrics, openrtb_ext.BidderAppnexus)
bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, metrics, openrtb_ext.BidderAppnexus, nil)
currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0))
_, errs := bidder.requestBid(context.Background(), &openrtb.BidRequest{}, "test", bidAdjustment, currencyConverter.Rates(), &adapters.ExtraRequestInfo{})

Expand Down Expand Up @@ -1401,6 +1422,23 @@ func TestTimeoutNotificationOn(t *testing.T) {
assert.EqualValues(t, logExpected, logActual)
}

func TestParseDebugInfoTrue(t *testing.T) {
debugInfo := &config.DebugInfo{Allow: true}
resDebugInfo := parseDebugInfo(debugInfo)
assert.True(t, resDebugInfo.Allow, "Debug Allow value should be false")
Copy link
Contributor

Choose a reason for hiding this comment

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

The failure message here should mention true instead of false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thank you!

}

func TestParseDebugInfoFalse(t *testing.T) {
debugInfo := &config.DebugInfo{Allow: false}
resDebugInfo := parseDebugInfo(debugInfo)
assert.False(t, resDebugInfo.Allow, "Debug Allow value should be false")
}

func TestParseDebugInfoIsNil(t *testing.T) {
resDebugInfo := parseDebugInfo(nil)
assert.True(t, resDebugInfo.Allow, "Debug Allow value should be false")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, just need to change failure message to true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

}

func wrapWithBidderInfo(bidder adapters.Bidder) adapters.Bidder {
bidderInfo := adapters.BidderInfo{
Status: adapters.StatusActive,
Expand Down
Loading