From 00f48e3e15c84546a2557137b098049660274148 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Wed, 5 Jun 2019 14:14:45 -0400 Subject: [PATCH 01/17] going out real quick, brb --- endpoints/openrtb2/amp_auction.go | 4 ++ endpoints/openrtb2/auction.go | 4 ++ endpoints/openrtb2/video_auction.go | 4 ++ exchange/exchange.go | 62 +++++++++++++++++++++++++++++ pbsmetrics/config/metrics_test.go | 19 +++++++++ pbsmetrics/go_metrics.go | 18 +++++++++ pbsmetrics/metrics.go | 17 +++++++- 7 files changed, 127 insertions(+), 1 deletion(-) diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 0d617071f19..4de1b5c55f0 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -91,6 +91,10 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeAMP, + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 772acbeba1f..6f01fee91bc 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -89,6 +89,10 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeORTB2Web, + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index acb28307534..7c52b82514d 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -74,6 +74,10 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeVideo, + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, diff --git a/exchange/exchange.go b/exchange/exchange.go index 5fd1bb6a29f..797b6fb3ac8 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -85,6 +85,68 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque } } + // Before the `cleanOpenRTBRequests(..)` function runs, let's set the labels.RType value here because this is a place common to all endpoints. + /* labels.RType must be extracted from bidRequest.Imp object + type Imp struct { + . + . + Banner *Banner `json:"banner,omitempty"` + + // Attribute: + // video + // Type: + // object + // Description: + // A Video object (Section 3.2.7); required if this impression is + // offered as a video ad opportunity. + Video *Video `json:"video,omitempty"` + + // Attribute: + // audio + // Type: + // object + // Description: + // An Audio object (Section 3.2.8); required if this impression is + // offered as an audio ad opportunity. + Audio *Audio `json:"audio,omitempty"` + + // Attribute: + // native + // Type: + // object + // Description: + // A Native object (Section 3.2.9); required if this impression is + // offered as a native ad opportunity. + Native *Native `json:"native,omitempty"` + . + . + } + Since: + "Hans Hjort [2:58 PM] + More complicated than that, as the type can have multiple simultaneous values. + Might be best to have 4 flags, `isNative`, `isBanner`, `isVideo`, and `isAudio`." + Then: + We can just turn on flags here if the Imp has some other type in there + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, + */ + for _, impInRequest := range bidRequest.Imp { + if impInRequest.Banner != nil { + labels.IsImpBanner = true + } + if impInRequest.Video != nil { + labels.IsImpVideo = true + } + if impInRequest.Audio != nil { + labels.IsImpAudio = true + } + if impInRequest.Native != nil { + labels.IsImpNative = true + } + } + // Slice of BidRequests, each a copy of the original cleaned to only contain bidder data for the named bidder blabels := make(map[openrtb_ext.BidderName]*pbsmetrics.AdapterLabels) cleanRequests, aliases, errs := cleanOpenRTBRequests(ctx, bidRequest, usersyncs, blabels, labels, e.gDPR, e.UsersyncIfAmbiguous) diff --git a/pbsmetrics/config/metrics_test.go b/pbsmetrics/config/metrics_test.go index 58c0ab45ec2..a1043d13633 100644 --- a/pbsmetrics/config/metrics_test.go +++ b/pbsmetrics/config/metrics_test.go @@ -46,6 +46,10 @@ func TestMultiMetricsEngine(t *testing.T) { labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, PubID: "test1", Browser: pbsmetrics.BrowserSafari, CookieFlag: pbsmetrics.CookieFlagYes, @@ -54,6 +58,10 @@ func TestMultiMetricsEngine(t *testing.T) { apnLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, Adapter: openrtb_ext.BidderAppnexus, PubID: "test1", Browser: pbsmetrics.BrowserSafari, @@ -63,6 +71,10 @@ func TestMultiMetricsEngine(t *testing.T) { pubLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, + IsImpBanner: false, + IsImpVideo: false, + IsImpAudio: false, + IsImpNative: false, Adapter: openrtb_ext.BidderPubmatic, PubID: "test1", Browser: pbsmetrics.BrowserSafari, @@ -79,6 +91,7 @@ func TestMultiMetricsEngine(t *testing.T) { metricsEngine.RecordAdapterBidReceived(pubLabels, openrtb_ext.BidTypeBanner, true) metricsEngine.RecordAdapterTime(pubLabels, time.Millisecond*20) } + //Make the metrics engine, instantiated here with goEngine, fill its RequestStatuses[RequestType][pbsmetrics.RequestStatusXX] with the new boolean values added to pbsmetrics.Labels VerifyMetrics(t, "RequestStatuses.OpenRTB2.OK", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) VerifyMetrics(t, "RequestStatuses.Legacy.OK", goEngine.RequestStatuses[pbsmetrics.ReqTypeLegacy][pbsmetrics.RequestStatusOK].Count(), 0) VerifyMetrics(t, "RequestStatuses.AMP.OK", goEngine.RequestStatuses[pbsmetrics.ReqTypeAMP][pbsmetrics.RequestStatusOK].Count(), 0) @@ -87,6 +100,12 @@ func TestMultiMetricsEngine(t *testing.T) { VerifyMetrics(t, "RequestStatuses.Video.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeVideo][pbsmetrics.RequestStatusBadInput].Count(), 0) VerifyMetrics(t, "RequestStatuses.OpenRTB2.Error", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusErr].Count(), 0) VerifyMetrics(t, "RequestStatuses.OpenRTB2.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusBadInput].Count(), 0) + //PBS-108 modify these accordingly so from the second comma on, they make sense. Modifications should probably come from the metrics.go file in here and should have a test case of their on + VerifyMetrics(t, "ImpMeter.isBanner", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) + VerifyMetrics(t, "ImpMeter.isVideo", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) + VerifyMetrics(t, "ImpMeter.isAudio", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) + VerifyMetrics(t, "ImpMeter.isNative", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) + VerifyMetrics(t, "Request", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) VerifyMetrics(t, "ImpMeter", goEngine.ImpMeter.Count(), 10) VerifyMetrics(t, "NoCookieMeter", goEngine.NoCookieMeter.Count(), 0) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index e569542de89..b537f728b26 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -37,6 +37,12 @@ type Metrics struct { userSyncSet map[openrtb_ext.BidderName]metrics.Meter userSyncGDPRPrevent map[openrtb_ext.BidderName]metrics.Meter + // Media types found in the "imp" JSON object + ImpBanner metrics.Meter + ImpVideo metrics.Meter + ImpAudio metrics.Meter + ImpNative metrics.Meter + AdapterMetrics map[openrtb_ext.BidderName]*AdapterMetrics // Don't export accountMetrics because we need helper functions here to insure its properly populated dynamically accountMetrics map[string]*accountMetrics @@ -107,6 +113,11 @@ func NewBlankMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderNa userSyncSet: make(map[openrtb_ext.BidderName]metrics.Meter), userSyncGDPRPrevent: make(map[openrtb_ext.BidderName]metrics.Meter), + ImpBanner: blankMeter, + ImpVideo: blankMeter, + ImpAudio: blankMeter, + ImpNative: blankMeter, + AdapterMetrics: make(map[openrtb_ext.BidderName]*AdapterMetrics, len(exchanges)), accountMetrics: make(map[string]*accountMetrics), @@ -137,6 +148,13 @@ func NewMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderName) * newMetrics.ConnectionAcceptErrorMeter = metrics.GetOrRegisterMeter("connection_accept_errors", registry) newMetrics.ConnectionCloseErrorMeter = metrics.GetOrRegisterMeter("connection_close_errors", registry) newMetrics.ImpMeter = metrics.GetOrRegisterMeter("imps_requested", registry) + + // Find out how to GetOrRegisterMeter + newMetrics.ImpBanner = metrics.GetOrRegisterMeter("imp_banner", registry) + newMetrics.ImpVideo = metrics.GetOrRegisterMeter("imp_video", registry) + newMetrics.ImpAudio = metrics.GetOrRegisterMeter("imp_audio", registry) + newMetrics.ImpNative = metrics.GetOrRegisterMeter("imp_native", registry) + newMetrics.SafariRequestMeter = metrics.GetOrRegisterMeter("safari_requests", registry) newMetrics.NoCookieMeter = metrics.GetOrRegisterMeter("no_cookie_requests", registry) newMetrics.AppRequestMeter = metrics.GetOrRegisterMeter("app_requests", registry) diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index db24ea90dfb..78dadafb231 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -10,6 +10,10 @@ import ( type Labels struct { Source DemandSource RType RequestType + IsImpBanner bool + IsImpVideo bool + IsImpAudio bool + IsImpNative bool PubID string // exchange specific ID, so we cannot compile in values Browser Browser CookieFlag CookieFlag @@ -19,7 +23,7 @@ type Labels struct { // AdapterLabels defines the labels that can be attached to the adapter metrics. type AdapterLabels struct { Source DemandSource - RType RequestType + RType RequestType //change?? Adapter openrtb_ext.BidderName PubID string // exchange specific ID, so we cannot compile in values Browser Browser @@ -33,6 +37,9 @@ type AdapterLabels struct { // DemandSource : Demand source enumeration type DemandSource string +// ImpMediaType : Media type described in the "imp" JSON object +type ImpMediaType string + // RequestType : Request type enumeration type RequestType string @@ -78,6 +85,14 @@ const ( ReqTypeVideo RequestType = "video" ) +// The media types described in the "imp json objects +const ( + ImpTypeBanner ImpMediaType = "banner" + ImpTypeVideo ImpMediaType = "video" + ImpTypeAudio ImpMediaType = "audio" + ImpTypeNative ImpMediaType = "native" +) + func RequestTypes() []RequestType { return []RequestType{ ReqTypeLegacy, From 6d8bf610572f518a1666920e51b016da02c021f0 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Wed, 5 Jun 2019 16:46:00 -0400 Subject: [PATCH 02/17] brb to attend 395 --- pbsmetrics/config/metrics_test.go | 14 +++++++------- pbsmetrics/go_metrics.go | 8 ++++---- pbsmetrics/metrics.go | 4 ++++ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/pbsmetrics/config/metrics_test.go b/pbsmetrics/config/metrics_test.go index a1043d13633..ee5051e9088 100644 --- a/pbsmetrics/config/metrics_test.go +++ b/pbsmetrics/config/metrics_test.go @@ -46,7 +46,7 @@ func TestMultiMetricsEngine(t *testing.T) { labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: false, + IsImpBanner: true, IsImpVideo: false, IsImpAudio: false, IsImpNative: false, @@ -58,7 +58,7 @@ func TestMultiMetricsEngine(t *testing.T) { apnLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: false, + IsImpBanner: true, IsImpVideo: false, IsImpAudio: false, IsImpNative: false, @@ -71,7 +71,7 @@ func TestMultiMetricsEngine(t *testing.T) { pubLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: false, + IsImpBanner: true, IsImpVideo: false, IsImpAudio: false, IsImpNative: false, @@ -101,10 +101,10 @@ func TestMultiMetricsEngine(t *testing.T) { VerifyMetrics(t, "RequestStatuses.OpenRTB2.Error", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusErr].Count(), 0) VerifyMetrics(t, "RequestStatuses.OpenRTB2.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusBadInput].Count(), 0) //PBS-108 modify these accordingly so from the second comma on, they make sense. Modifications should probably come from the metrics.go file in here and should have a test case of their on - VerifyMetrics(t, "ImpMeter.isBanner", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) - VerifyMetrics(t, "ImpMeter.isVideo", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) - VerifyMetrics(t, "ImpMeter.isAudio", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) - VerifyMetrics(t, "ImpMeter.isNative", goEngine.RequestStatuses[pbsmetrics.ReqTypeNative][pbsmetrics.RequestStatusOK].Count(), 0) + VerifyMetrics(t, "ImpTypeBanner", goEngine.ImpTypeBanner.Count(), 3) + VerifyMetrics(t, "ImpTypeVideo", goEngine.ImpTypeVideo.Count(), 0) + VerifyMetrics(t, "ImpTypeAudio", goEngine.ImpTypeAudio.Count(), 0) + VerifyMetrics(t, "ImpTypeNative", goEngine.ImpTypeNative.Count(), 0) VerifyMetrics(t, "Request", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) VerifyMetrics(t, "ImpMeter", goEngine.ImpMeter.Count(), 10) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index b537f728b26..655949b6bcb 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -38,10 +38,10 @@ type Metrics struct { userSyncGDPRPrevent map[openrtb_ext.BidderName]metrics.Meter // Media types found in the "imp" JSON object - ImpBanner metrics.Meter - ImpVideo metrics.Meter - ImpAudio metrics.Meter - ImpNative metrics.Meter + ImpTypeBanner metrics.Meter + ImpTypeVideo metrics.Meter + ImpTypeAudio metrics.Meter + ImpTypeNative metrics.Meter AdapterMetrics map[openrtb_ext.BidderName]*AdapterMetrics // Don't export accountMetrics because we need helper functions here to insure its properly populated dynamically diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 78dadafb231..58a86f9d70e 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -24,6 +24,10 @@ type Labels struct { type AdapterLabels struct { Source DemandSource RType RequestType //change?? + IsImpBanner bool + IsImpVideo bool + IsImpAudio bool + IsImpNative bool Adapter openrtb_ext.BidderName PubID string // exchange specific ID, so we cannot compile in values Browser Browser From 654fb2c0a9c36600fc035bc09ca7b6d4c6f2e65f Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Fri, 7 Jun 2019 15:28:05 -0400 Subject: [PATCH 03/17] Test cases still not ready, but touching base to see if we are on the right track here --- pbsmetrics/go_metrics.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index 655949b6bcb..1639bcf20be 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -113,10 +113,10 @@ func NewBlankMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderNa userSyncSet: make(map[openrtb_ext.BidderName]metrics.Meter), userSyncGDPRPrevent: make(map[openrtb_ext.BidderName]metrics.Meter), - ImpBanner: blankMeter, - ImpVideo: blankMeter, - ImpAudio: blankMeter, - ImpNative: blankMeter, + ImpTypeBanner: blankMeter, + ImpTypeVideo: blankMeter, + ImpTypeAudio: blankMeter, + ImpTypeNative: blankMeter, AdapterMetrics: make(map[openrtb_ext.BidderName]*AdapterMetrics, len(exchanges)), accountMetrics: make(map[string]*accountMetrics), @@ -150,10 +150,10 @@ func NewMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderName) * newMetrics.ImpMeter = metrics.GetOrRegisterMeter("imps_requested", registry) // Find out how to GetOrRegisterMeter - newMetrics.ImpBanner = metrics.GetOrRegisterMeter("imp_banner", registry) - newMetrics.ImpVideo = metrics.GetOrRegisterMeter("imp_video", registry) - newMetrics.ImpAudio = metrics.GetOrRegisterMeter("imp_audio", registry) - newMetrics.ImpNative = metrics.GetOrRegisterMeter("imp_native", registry) + newMetrics.ImpTypeBanner = metrics.GetOrRegisterMeter("imp_banner", registry) + newMetrics.ImpTypeVideo = metrics.GetOrRegisterMeter("imp_video", registry) + newMetrics.ImpTypeAudio = metrics.GetOrRegisterMeter("imp_audio", registry) + newMetrics.ImpTypeNative = metrics.GetOrRegisterMeter("imp_native", registry) newMetrics.SafariRequestMeter = metrics.GetOrRegisterMeter("safari_requests", registry) newMetrics.NoCookieMeter = metrics.GetOrRegisterMeter("no_cookie_requests", registry) From aaa9a00fd30e6ca827e9cf83e56c7cefb45c57a0 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Mon, 10 Jun 2019 12:54:50 -0400 Subject: [PATCH 04/17] removed some commented lines --- exchange/exchange.go | 47 ------------------------------- pbsmetrics/config/metrics_test.go | 2 +- pbsmetrics/metrics.go | 2 +- 3 files changed, 2 insertions(+), 49 deletions(-) diff --git a/exchange/exchange.go b/exchange/exchange.go index 797b6fb3ac8..91e523143d3 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -85,53 +85,6 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque } } - // Before the `cleanOpenRTBRequests(..)` function runs, let's set the labels.RType value here because this is a place common to all endpoints. - /* labels.RType must be extracted from bidRequest.Imp object - type Imp struct { - . - . - Banner *Banner `json:"banner,omitempty"` - - // Attribute: - // video - // Type: - // object - // Description: - // A Video object (Section 3.2.7); required if this impression is - // offered as a video ad opportunity. - Video *Video `json:"video,omitempty"` - - // Attribute: - // audio - // Type: - // object - // Description: - // An Audio object (Section 3.2.8); required if this impression is - // offered as an audio ad opportunity. - Audio *Audio `json:"audio,omitempty"` - - // Attribute: - // native - // Type: - // object - // Description: - // A Native object (Section 3.2.9); required if this impression is - // offered as a native ad opportunity. - Native *Native `json:"native,omitempty"` - . - . - } - Since: - "Hans Hjort [2:58 PM] - More complicated than that, as the type can have multiple simultaneous values. - Might be best to have 4 flags, `isNative`, `isBanner`, `isVideo`, and `isAudio`." - Then: - We can just turn on flags here if the Imp has some other type in there - IsImpBanner: false, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, - */ for _, impInRequest := range bidRequest.Imp { if impInRequest.Banner != nil { labels.IsImpBanner = true diff --git a/pbsmetrics/config/metrics_test.go b/pbsmetrics/config/metrics_test.go index ee5051e9088..ce091886363 100644 --- a/pbsmetrics/config/metrics_test.go +++ b/pbsmetrics/config/metrics_test.go @@ -100,7 +100,7 @@ func TestMultiMetricsEngine(t *testing.T) { VerifyMetrics(t, "RequestStatuses.Video.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeVideo][pbsmetrics.RequestStatusBadInput].Count(), 0) VerifyMetrics(t, "RequestStatuses.OpenRTB2.Error", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusErr].Count(), 0) VerifyMetrics(t, "RequestStatuses.OpenRTB2.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusBadInput].Count(), 0) - //PBS-108 modify these accordingly so from the second comma on, they make sense. Modifications should probably come from the metrics.go file in here and should have a test case of their on + VerifyMetrics(t, "ImpTypeBanner", goEngine.ImpTypeBanner.Count(), 3) VerifyMetrics(t, "ImpTypeVideo", goEngine.ImpTypeVideo.Count(), 0) VerifyMetrics(t, "ImpTypeAudio", goEngine.ImpTypeAudio.Count(), 0) diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 58a86f9d70e..0c6f3c9fff6 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -23,7 +23,7 @@ type Labels struct { // AdapterLabels defines the labels that can be attached to the adapter metrics. type AdapterLabels struct { Source DemandSource - RType RequestType //change?? + RType RequestType IsImpBanner bool IsImpVideo bool IsImpAudio bool From 5d7fa8fcbf7cc28e7c1fc952e599ffea099b99e8 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Fri, 14 Jun 2019 15:22:27 -0400 Subject: [PATCH 05/17] Counting Imp types inside go_metrics.RecordImps function --- endpoints/openrtb2/amp_auction.go | 8 ++++---- endpoints/openrtb2/auction.go | 8 ++++---- endpoints/openrtb2/video_auction.go | 8 ++++---- exchange/exchange.go | 8 ++++---- pbsmetrics/config/metrics_test.go | 32 ++++++++++++++--------------- pbsmetrics/go_metrics.go | 28 ++++++++++++++----------- pbsmetrics/metrics.go | 20 +++++++++--------- 7 files changed, 58 insertions(+), 54 deletions(-) diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 4de1b5c55f0..9449d9eba47 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -91,10 +91,10 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeAMP, - IsImpBanner: false, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, + BannerImps: 0, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 6f01fee91bc..311bf7481c1 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -89,10 +89,10 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: false, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, + BannerImps: 0, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index 7c52b82514d..a229bba0ab7 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -74,10 +74,10 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeVideo, - IsImpBanner: false, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, + BannerImps: 0, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, diff --git a/exchange/exchange.go b/exchange/exchange.go index 91e523143d3..bc16df79cea 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -87,16 +87,16 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque for _, impInRequest := range bidRequest.Imp { if impInRequest.Banner != nil { - labels.IsImpBanner = true + labels.BannerImps += 1 } if impInRequest.Video != nil { - labels.IsImpVideo = true + labels.VideoImps += 1 } if impInRequest.Audio != nil { - labels.IsImpAudio = true + labels.AudioImps += 1 } if impInRequest.Native != nil { - labels.IsImpNative = true + labels.NativeImps += 1 } } diff --git a/pbsmetrics/config/metrics_test.go b/pbsmetrics/config/metrics_test.go index ce091886363..8b6829a2cfd 100644 --- a/pbsmetrics/config/metrics_test.go +++ b/pbsmetrics/config/metrics_test.go @@ -46,10 +46,10 @@ func TestMultiMetricsEngine(t *testing.T) { labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: true, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, + BannerImps: 1, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, PubID: "test1", Browser: pbsmetrics.BrowserSafari, CookieFlag: pbsmetrics.CookieFlagYes, @@ -58,10 +58,10 @@ func TestMultiMetricsEngine(t *testing.T) { apnLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: true, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, + BannerImps: 1, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, Adapter: openrtb_ext.BidderAppnexus, PubID: "test1", Browser: pbsmetrics.BrowserSafari, @@ -71,10 +71,10 @@ func TestMultiMetricsEngine(t *testing.T) { pubLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - IsImpBanner: true, - IsImpVideo: false, - IsImpAudio: false, - IsImpNative: false, + BannerImps: 1, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, Adapter: openrtb_ext.BidderPubmatic, PubID: "test1", Browser: pbsmetrics.BrowserSafari, @@ -101,10 +101,10 @@ func TestMultiMetricsEngine(t *testing.T) { VerifyMetrics(t, "RequestStatuses.OpenRTB2.Error", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusErr].Count(), 0) VerifyMetrics(t, "RequestStatuses.OpenRTB2.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusBadInput].Count(), 0) - VerifyMetrics(t, "ImpTypeBanner", goEngine.ImpTypeBanner.Count(), 3) - VerifyMetrics(t, "ImpTypeVideo", goEngine.ImpTypeVideo.Count(), 0) - VerifyMetrics(t, "ImpTypeAudio", goEngine.ImpTypeAudio.Count(), 0) - VerifyMetrics(t, "ImpTypeNative", goEngine.ImpTypeNative.Count(), 0) + VerifyMetrics(t, "ImpsTypeBanner", goEngine.ImpsTypeBanner.Count(), 5) + VerifyMetrics(t, "ImpsTypeVideo", goEngine.ImpsTypeVideo.Count(), 0) + VerifyMetrics(t, "ImpsTypeAudio", goEngine.ImpsTypeAudio.Count(), 0) + VerifyMetrics(t, "ImpsTypeNative", goEngine.ImpsTypeNative.Count(), 0) VerifyMetrics(t, "Request", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) VerifyMetrics(t, "ImpMeter", goEngine.ImpMeter.Count(), 10) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index 1639bcf20be..d870777c48b 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -38,10 +38,10 @@ type Metrics struct { userSyncGDPRPrevent map[openrtb_ext.BidderName]metrics.Meter // Media types found in the "imp" JSON object - ImpTypeBanner metrics.Meter - ImpTypeVideo metrics.Meter - ImpTypeAudio metrics.Meter - ImpTypeNative metrics.Meter + ImpsTypeBanner metrics.Meter + ImpsTypeVideo metrics.Meter + ImpsTypeAudio metrics.Meter + ImpsTypeNative metrics.Meter AdapterMetrics map[openrtb_ext.BidderName]*AdapterMetrics // Don't export accountMetrics because we need helper functions here to insure its properly populated dynamically @@ -113,10 +113,10 @@ func NewBlankMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderNa userSyncSet: make(map[openrtb_ext.BidderName]metrics.Meter), userSyncGDPRPrevent: make(map[openrtb_ext.BidderName]metrics.Meter), - ImpTypeBanner: blankMeter, - ImpTypeVideo: blankMeter, - ImpTypeAudio: blankMeter, - ImpTypeNative: blankMeter, + ImpsTypeBanner: blankMeter, + ImpsTypeVideo: blankMeter, + ImpsTypeAudio: blankMeter, + ImpsTypeNative: blankMeter, AdapterMetrics: make(map[openrtb_ext.BidderName]*AdapterMetrics, len(exchanges)), accountMetrics: make(map[string]*accountMetrics), @@ -150,10 +150,10 @@ func NewMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderName) * newMetrics.ImpMeter = metrics.GetOrRegisterMeter("imps_requested", registry) // Find out how to GetOrRegisterMeter - newMetrics.ImpTypeBanner = metrics.GetOrRegisterMeter("imp_banner", registry) - newMetrics.ImpTypeVideo = metrics.GetOrRegisterMeter("imp_video", registry) - newMetrics.ImpTypeAudio = metrics.GetOrRegisterMeter("imp_audio", registry) - newMetrics.ImpTypeNative = metrics.GetOrRegisterMeter("imp_native", registry) + newMetrics.ImpsTypeBanner = metrics.GetOrRegisterMeter("imp_banner", registry) //TODO: where is the "imp_banner" label or value? + newMetrics.ImpsTypeVideo = metrics.GetOrRegisterMeter("imp_video", registry) + newMetrics.ImpsTypeAudio = metrics.GetOrRegisterMeter("imp_audio", registry) + newMetrics.ImpsTypeNative = metrics.GetOrRegisterMeter("imp_native", registry) newMetrics.SafariRequestMeter = metrics.GetOrRegisterMeter("safari_requests", registry) newMetrics.NoCookieMeter = metrics.GetOrRegisterMeter("no_cookie_requests", registry) @@ -317,6 +317,10 @@ func (me *Metrics) RecordRequest(labels Labels) { func (me *Metrics) RecordImps(labels Labels, numImps int) { me.ImpMeter.Mark(int64(numImps)) + me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) + me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) + me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) + me.ImpsTypeNative.Mark(int64(labels.NativeImps)) } func (me *Metrics) RecordConnectionAccept(success bool) { diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 0c6f3c9fff6..6a209f5e20f 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -10,10 +10,10 @@ import ( type Labels struct { Source DemandSource RType RequestType - IsImpBanner bool - IsImpVideo bool - IsImpAudio bool - IsImpNative bool + BannerImps int + VideoImps int + AudioImps int + NativeImps int PubID string // exchange specific ID, so we cannot compile in values Browser Browser CookieFlag CookieFlag @@ -24,10 +24,10 @@ type Labels struct { type AdapterLabels struct { Source DemandSource RType RequestType - IsImpBanner bool - IsImpVideo bool - IsImpAudio bool - IsImpNative bool + BannerImps int + VideoImps int + AudioImps int + NativeImps int Adapter openrtb_ext.BidderName PubID string // exchange specific ID, so we cannot compile in values Browser Browser @@ -41,7 +41,7 @@ type AdapterLabels struct { // DemandSource : Demand source enumeration type DemandSource string -// ImpMediaType : Media type described in the "imp" JSON object +// ImpMediaType : Media type described in the "imp" JSON object TODO is this still needed? type ImpMediaType string // RequestType : Request type enumeration @@ -89,7 +89,7 @@ const ( ReqTypeVideo RequestType = "video" ) -// The media types described in the "imp json objects +// The media types described in the "imp" json objects TODO is this still needed? const ( ImpTypeBanner ImpMediaType = "banner" ImpTypeVideo ImpMediaType = "video" From bcd4b12c8b87f8f8b5cd23833494d066668dbca8 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Fri, 14 Jun 2019 16:22:49 -0400 Subject: [PATCH 06/17] Added >0 check in go_metrics.RecordImps --- pbsmetrics/go_metrics.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index d870777c48b..8b6867ca915 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -317,10 +317,18 @@ func (me *Metrics) RecordRequest(labels Labels) { func (me *Metrics) RecordImps(labels Labels, numImps int) { me.ImpMeter.Mark(int64(numImps)) - me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) - me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) - me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) - me.ImpsTypeNative.Mark(int64(labels.NativeImps)) + if labels.BannerImps > 0 { + me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) + } + if labels.VideoImps > 0 { + me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) + } + if labels.AudioImps > 0 { + me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) + } + if labels.NativeImps > 0 { + me.ImpsTypeNative.Mark(int64(labels.NativeImps)) + } } func (me *Metrics) RecordConnectionAccept(success bool) { From d4849385f3a27c8418a0da4eb0f76a7033fac9f3 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Mon, 17 Jun 2019 15:59:53 -0400 Subject: [PATCH 07/17] working on the prometheus part --- pbsmetrics/prometheus/prometheus.go | 38 +++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 3d09f10a4d8..faaa1ba0925 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -16,6 +16,10 @@ type Metrics struct { connCounter prometheus.Gauge connError *prometheus.CounterVec imps *prometheus.CounterVec + bannerImps *prometheus.CounterVec + videoImps *prometheus.CounterVec + audioImps *prometheus.CounterVec + nativeImps *prometheus.CounterVec requests *prometheus.CounterVec reqTimer *prometheus.HistogramVec adaptRequests *prometheus.CounterVec @@ -58,6 +62,28 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { standardLabelNames, ) metrics.Registry.MustRegister(metrics.imps) + + metrics.bannerImps = newCounter(cfg, "banner_imps_requested_total", + "Total number of impressions of type 'banner' requested through PBS.", + standardLabelNames, + ) + metrics.videoImps = newCounter(cfg, "video_imps_requested_total", + "Total number of impressions of type 'video' requested through PBS.", + standardLabelNames, + ) + metrics.videoImps = newCounter(cfg, "audio_imps_requested_total", + "Total number of impressions of type 'audio' requested through PBS.", + standardLabelNames, + ) + metrics.nativeImps = newCounter(cfg, "native_imps_requested_total", + "Total number of impressions of type 'native' requested through PBS.", + standardLabelNames, + ) + metrics.Registry.MustRegister(metrics.imps) + metrics.Registry.MustRegister(metrics.imps) + metrics.Registry.MustRegister(metrics.imps) + metrics.Registry.MustRegister(metrics.imps) + metrics.requests = newCounter(cfg, "requests_total", "Total number of requests made to PBS.", standardLabelNames, @@ -190,6 +216,18 @@ func (me *Metrics) RecordRequest(labels pbsmetrics.Labels) { func (me *Metrics) RecordImps(labels pbsmetrics.Labels, numImps int) { me.imps.With(resolveLabels(labels)).Add(float64(numImps)) + if labels.BannerImps > 0 { + me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) + } + if labels.VideoImps > 0 { + me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) + } + if labels.AudioImps > 0 { + me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) + } + if labels.NativeImps > 0 { + me.ImpsTypeNative.Mark(int64(labels.NativeImps)) + } } func (me *Metrics) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { From d5a6afedd73a0f2e0fd5b6a65e47a7463056fb1d Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 18 Jun 2019 13:36:36 -0400 Subject: [PATCH 08/17] going out real quick --- pbsmetrics/prometheus/prometheus.go | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index faaa1ba0925..f4b14cca039 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -48,6 +48,8 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { bidLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_bid", "adapter", "bidtype", "markup_type"} errorLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_error", "adapter"} + impLabelNames := []string{"banner", "video", "audio", "native"} + metrics := Metrics{} metrics.Registry = prometheus.NewRegistry() metrics.connCounter = newConnCounter(cfg) @@ -59,31 +61,9 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { metrics.Registry.MustRegister(metrics.connError) metrics.imps = newCounter(cfg, "imps_requested_total", "Total number of impressions requested through PBS.", - standardLabelNames, - ) - metrics.Registry.MustRegister(metrics.imps) - - metrics.bannerImps = newCounter(cfg, "banner_imps_requested_total", - "Total number of impressions of type 'banner' requested through PBS.", - standardLabelNames, - ) - metrics.videoImps = newCounter(cfg, "video_imps_requested_total", - "Total number of impressions of type 'video' requested through PBS.", - standardLabelNames, - ) - metrics.videoImps = newCounter(cfg, "audio_imps_requested_total", - "Total number of impressions of type 'audio' requested through PBS.", - standardLabelNames, - ) - metrics.nativeImps = newCounter(cfg, "native_imps_requested_total", - "Total number of impressions of type 'native' requested through PBS.", - standardLabelNames, + impLabelNames, ) metrics.Registry.MustRegister(metrics.imps) - metrics.Registry.MustRegister(metrics.imps) - metrics.Registry.MustRegister(metrics.imps) - metrics.Registry.MustRegister(metrics.imps) - metrics.requests = newCounter(cfg, "requests_total", "Total number of requests made to PBS.", standardLabelNames, @@ -216,6 +196,8 @@ func (me *Metrics) RecordRequest(labels pbsmetrics.Labels) { func (me *Metrics) RecordImps(labels pbsmetrics.Labels, numImps int) { me.imps.With(resolveLabels(labels)).Add(float64(numImps)) + // TODO. Read https://prometheus.io/docs/prometheus/latest/getting_started/ + // and make changes here if labels.BannerImps > 0 { me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) } From d9190028418abc5556441e24c5935591225856f3 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Thu, 20 Jun 2019 11:05:56 -0400 Subject: [PATCH 09/17] Finally working as a counterVector for banner. Replicate for the others --- pbsmetrics/prometheus/prometheus.go | 49 +++++++++++++------ pbsmetrics/prometheus/prometheus_test.go | 61 ++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index f4b14cca039..4e6335634f0 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -1,6 +1,7 @@ package prometheusmetrics import ( + "strconv" "time" "github.com/prebid/prebid-server/config" @@ -16,6 +17,7 @@ type Metrics struct { connCounter prometheus.Gauge connError *prometheus.CounterVec imps *prometheus.CounterVec + impTypes *prometheus.CounterVec bannerImps *prometheus.CounterVec videoImps *prometheus.CounterVec audioImps *prometheus.CounterVec @@ -48,7 +50,7 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { bidLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_bid", "adapter", "bidtype", "markup_type"} errorLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_error", "adapter"} - impLabelNames := []string{"banner", "video", "audio", "native"} + impLabelNames := []string{"banner_imps", "video_imps", "audio_imps", "native_imps"} metrics := Metrics{} metrics.Registry = prometheus.NewRegistry() @@ -61,9 +63,17 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { metrics.Registry.MustRegister(metrics.connError) metrics.imps = newCounter(cfg, "imps_requested_total", "Total number of impressions requested through PBS.", - impLabelNames, + standardLabelNames, ) metrics.Registry.MustRegister(metrics.imps) + + // NEW imp types + metrics.impTypes = newCounter(cfg, "imps_types_total", + "Total number of impression types requested through PBS.", + impLabelNames, + ) + metrics.Registry.MustRegister(metrics.impTypes) + metrics.requests = newCounter(cfg, "requests_total", "Total number of requests made to PBS.", standardLabelNames, @@ -198,18 +208,20 @@ func (me *Metrics) RecordImps(labels pbsmetrics.Labels, numImps int) { me.imps.With(resolveLabels(labels)).Add(float64(numImps)) // TODO. Read https://prometheus.io/docs/prometheus/latest/getting_started/ // and make changes here - if labels.BannerImps > 0 { - me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) - } - if labels.VideoImps > 0 { - me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) - } - if labels.AudioImps > 0 { - me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) - } - if labels.NativeImps > 0 { - me.ImpsTypeNative.Mark(int64(labels.NativeImps)) - } + /* + if labels.BannerImps > 0 { + me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) + } + if labels.VideoImps > 0 { + me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) + } + if labels.AudioImps > 0 { + me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) + } + if labels.NativeImps > 0 { + me.ImpsTypeNative.Mark(int64(labels.NativeImps)) + } + */ } func (me *Metrics) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { @@ -339,6 +351,15 @@ func resolveUserSyncLabels(userLabels pbsmetrics.UserLabels) prometheus.Labels { } } +func resolveImpTypeLabels(labels pbsmetrics.Labels) prometheus.Labels { + return prometheus.Labels{ + "banner_imps": strconv.Itoa(labels.BannerImps), + "video_imps": strconv.Itoa(labels.VideoImps), + "audio_imps": strconv.Itoa(labels.AudioImps), + "native_imps": strconv.Itoa(labels.NativeImps), + } +} + // initializeTimeSeries precreates all possible metric label values, so there is no locking needed at run time creating new instances func initializeTimeSeries(m *Metrics) { // Connection errors diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index 75119e7e1b0..b9d4bc03d89 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -1,6 +1,7 @@ package prometheusmetrics import ( + //"fmt" "regexp" "strconv" "testing" @@ -78,6 +79,7 @@ func TestImpMetrics(t *testing.T) { metrics3 := dto.Metric{} metrics4 := dto.Metric{} + //proMetrics.imps.With(resolveLabels(labels[4)).Add(float64(5)) proMetrics.RecordImps(labels[0], 1) proMetrics.RecordImps(labels[1], 5) proMetrics.RecordImps(labels[0], 2) @@ -99,6 +101,35 @@ func TestImpMetrics(t *testing.T) { assertCounterValue(t, "imps_requested[4]", &metrics4, 1) } +func TestImpTypeMetrics(t *testing.T) { + proMetrics := newTestMetricsEngine() + + metricsBanner := dto.Metric{} + //metricsAudio := dto.Metric{} + //metricsVideo := dto.Metric{} + //metricsNative := dto.Metric{} + + proMetrics.impTypes.With(prometheus.Labels{"banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0"}).Add(float64(impTypeLabels[0].BannerImps)) + //a_banner_imp_counter := proMetrics.impTypes.With(prometheus.Labels{"banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0"}) + //a_banner_imp_counter.Add(1) + + //prometheus.Labels{ "banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0" } + //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[0])).Add(float64(impTypeLabels[0].BannerImps)) + //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[1])).Add(float64(impTypeLabels[1].VideoImps)) + //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Add(float64(impTypeLabels[2].AudioImps)) + //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Add(float64(impTypeLabels[3].NativeImps)) + + proMetrics.impTypes.With(prometheus.Labels{"banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0"}).Write(&metricsBanner) + //proMetrics.impTypes.With(resolveLabels(impTypeLabels[1])).Write(&metricsAudio) + //proMetrics.impTypes.With(resolveLabels(impTypeLabels[2])).Write(&metricsVideo) + //proMetrics.impTypes.With(resolveLabels(impTypeLabels[3])).Write(&metricsNative) + + assertCounterValue(t, "imps_types_0", &metricsBanner, 1) + //assertCounterValue(t, "imps_types_1", &metricsAudio, 1) + //assertCounterValue(t, "imps_types_2", &metricsVideo, 1) + //assertCounterValue(t, "imps_types_3", &metricsNative, 1) +} + func TestTimerMetrics(t *testing.T) { proMetrics := newTestMetricsEngine() @@ -424,6 +455,36 @@ var labels = []pbsmetrics.Labels{ RequestStatus: pbsmetrics.RequestStatusOK, }, } +var impTypeLabels = []pbsmetrics.Labels{ + { + //impType: "banner", + BannerImps: 1, + VideoImps: 0, + AudioImps: 0, + NativeImps: 0, + }, + { + //impType: "audio", + BannerImps: 0, + VideoImps: 1, + AudioImps: 0, + NativeImps: 0, + }, + { + //impType: "video", + BannerImps: 0, + VideoImps: 0, + AudioImps: 1, + NativeImps: 0, + }, + { + //impType: "native", + BannerImps: 0, + VideoImps: 0, + AudioImps: 0, + NativeImps: 1, + }, +} var s struct{} From 5d76416dfbb069aab59a4a6d45b69c0f841c14e9 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Thu, 20 Jun 2019 12:11:35 -0400 Subject: [PATCH 10/17] Prometheus implemented --- pbsmetrics/prometheus/prometheus.go | 33 ++++++++----------- pbsmetrics/prometheus/prometheus_test.go | 41 +++++++++++------------- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 4e6335634f0..cb9096677c4 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -18,10 +18,6 @@ type Metrics struct { connError *prometheus.CounterVec imps *prometheus.CounterVec impTypes *prometheus.CounterVec - bannerImps *prometheus.CounterVec - videoImps *prometheus.CounterVec - audioImps *prometheus.CounterVec - nativeImps *prometheus.CounterVec requests *prometheus.CounterVec reqTimer *prometheus.HistogramVec adaptRequests *prometheus.CounterVec @@ -206,22 +202,19 @@ func (me *Metrics) RecordRequest(labels pbsmetrics.Labels) { func (me *Metrics) RecordImps(labels pbsmetrics.Labels, numImps int) { me.imps.With(resolveLabels(labels)).Add(float64(numImps)) - // TODO. Read https://prometheus.io/docs/prometheus/latest/getting_started/ - // and make changes here - /* - if labels.BannerImps > 0 { - me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) - } - if labels.VideoImps > 0 { - me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) - } - if labels.AudioImps > 0 { - me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) - } - if labels.NativeImps > 0 { - me.ImpsTypeNative.Mark(int64(labels.NativeImps)) - } - */ + var impLabels prometheus.Labels = resolveImpTypeLabels(labels) + if labels.BannerImps > 0 { + me.impTypes.With(impLabels).Add(float64(labels.BannerImps)) + } + if labels.VideoImps > 0 { + me.impTypes.With(impLabels).Add(float64(labels.VideoImps)) + } + if labels.AudioImps > 0 { + me.impTypes.With(impLabels).Add(float64(labels.AudioImps)) + } + if labels.NativeImps > 0 { + me.impTypes.With(impLabels).Add(float64(labels.NativeImps)) + } } func (me *Metrics) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index b9d4bc03d89..1087a80afc4 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -105,29 +105,24 @@ func TestImpTypeMetrics(t *testing.T) { proMetrics := newTestMetricsEngine() metricsBanner := dto.Metric{} - //metricsAudio := dto.Metric{} - //metricsVideo := dto.Metric{} - //metricsNative := dto.Metric{} - - proMetrics.impTypes.With(prometheus.Labels{"banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0"}).Add(float64(impTypeLabels[0].BannerImps)) - //a_banner_imp_counter := proMetrics.impTypes.With(prometheus.Labels{"banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0"}) - //a_banner_imp_counter.Add(1) - - //prometheus.Labels{ "banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0" } - //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[0])).Add(float64(impTypeLabels[0].BannerImps)) - //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[1])).Add(float64(impTypeLabels[1].VideoImps)) - //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Add(float64(impTypeLabels[2].AudioImps)) - //proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Add(float64(impTypeLabels[3].NativeImps)) - - proMetrics.impTypes.With(prometheus.Labels{"banner_imps": "1", "video_imps": "0", "audio_imps": "0", "native_imps": "0"}).Write(&metricsBanner) - //proMetrics.impTypes.With(resolveLabels(impTypeLabels[1])).Write(&metricsAudio) - //proMetrics.impTypes.With(resolveLabels(impTypeLabels[2])).Write(&metricsVideo) - //proMetrics.impTypes.With(resolveLabels(impTypeLabels[3])).Write(&metricsNative) - - assertCounterValue(t, "imps_types_0", &metricsBanner, 1) - //assertCounterValue(t, "imps_types_1", &metricsAudio, 1) - //assertCounterValue(t, "imps_types_2", &metricsVideo, 1) - //assertCounterValue(t, "imps_types_3", &metricsNative, 1) + metricsAudio := dto.Metric{} + metricsVideo := dto.Metric{} + metricsNative := dto.Metric{} + + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[0])).Add(float64(impTypeLabels[0].BannerImps)) + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[1])).Add(float64(impTypeLabels[1].VideoImps)) + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Add(float64(impTypeLabels[2].AudioImps)) + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Add(float64(impTypeLabels[3].NativeImps)) + + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[0])).Write(&metricsBanner) + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[1])).Write(&metricsAudio) + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Write(&metricsVideo) + proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Write(&metricsNative) + + assertCounterValue(t, "imps_types_banner_count", &metricsBanner, 1) + assertCounterValue(t, "imps_types_banner_count", &metricsAudio, 1) + assertCounterValue(t, "imps_types_banner_count", &metricsVideo, 1) + assertCounterValue(t, "imps_types_banner_count", &metricsNative, 1) } func TestTimerMetrics(t *testing.T) { From 76619938315a71ababe38c98bdbbd86de30b5777 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Thu, 20 Jun 2019 14:08:22 -0400 Subject: [PATCH 11/17] Test case for RecordImps() function --- pbsmetrics/prometheus/prometheus.go | 3 -- pbsmetrics/prometheus/prometheus_test.go | 56 ++++++++++++++++++++---- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index cb9096677c4..c9e90c586bd 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -62,14 +62,11 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { standardLabelNames, ) metrics.Registry.MustRegister(metrics.imps) - - // NEW imp types metrics.impTypes = newCounter(cfg, "imps_types_total", "Total number of impression types requested through PBS.", impLabelNames, ) metrics.Registry.MustRegister(metrics.impTypes) - metrics.requests = newCounter(cfg, "requests_total", "Total number of requests made to PBS.", standardLabelNames, diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index 1087a80afc4..8d4c72906d9 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -1,7 +1,7 @@ package prometheusmetrics import ( - //"fmt" + "fmt" "regexp" "strconv" "testing" @@ -12,6 +12,7 @@ import ( "github.com/prebid/prebid-server/pbsmetrics" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/assert" ) var gaugeValueRegexp = regexp.MustCompile("gauge:") @@ -119,10 +120,47 @@ func TestImpTypeMetrics(t *testing.T) { proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Write(&metricsVideo) proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Write(&metricsNative) - assertCounterValue(t, "imps_types_banner_count", &metricsBanner, 1) - assertCounterValue(t, "imps_types_banner_count", &metricsAudio, 1) - assertCounterValue(t, "imps_types_banner_count", &metricsVideo, 1) - assertCounterValue(t, "imps_types_banner_count", &metricsNative, 1) + assertCounterValue(t, "imps_types_banner_count", &metricsBanner, 5) + assertCounterValue(t, "imps_types_banner_count", &metricsAudio, 9) + assertCounterValue(t, "imps_types_banner_count", &metricsVideo, 8) + assertCounterValue(t, "imps_types_banner_count", &metricsNative, 3) +} + +func TestRecordImps(t *testing.T) { + var sampleEngine *Metrics = newTestMetricsEngine() + var sampleMetrics dto.Metric = dto.Metric{} + var samplePBSLabels pbsmetrics.Labels = pbsmetrics.Labels{ + BannerImps: 5, + VideoImps: 4, + AudioImps: 3, + NativeImps: 2, + } + var samplePrometheusMetrics prometheus.Labels = resolveImpTypeLabels(samplePBSLabels) + + sampleEngine.RecordImps(samplePBSLabels, 14) + sampleEngine.impTypes.With(samplePrometheusMetrics).Write(&sampleMetrics) + + for _, metricLabel := range sampleMetrics.Label { + var impType string = *metricLabel.Name + switch impType { + case "banner_imps": + assert.Equal(t, *metricLabel.Value, "5", "'banner_imps' should equal '3'") + case "video_imps": + assert.Equal(t, *metricLabel.Value, "4", "'banner_imps' should equal '3'") + case "audio_imps": + assert.Equal(t, *metricLabel.Value, "3", "'banner_imps' should equal '3'") + case "native_imps": + assert.Equal(t, *metricLabel.Value, "2", "'banner_imps' should equal '3'") + default: + fmt.Printf("'%s' is not recognized as a label \n", impType) + } + } + + //fmt.Printf("%s \n", sampleMetrics.String()) + //assertCounterValue(t, "imps_types_banner_count", &sampleMetrics, 5) + //assertCounterValue(t, "imps_types_banner_count", &metricsAudio, 9) + //assertCounterValue(t, "imps_types_banner_count", &metricsVideo, 8) + //assertCounterValue(t, "imps_types_banner_count", &metricsNative, 3) } func TestTimerMetrics(t *testing.T) { @@ -453,7 +491,7 @@ var labels = []pbsmetrics.Labels{ var impTypeLabels = []pbsmetrics.Labels{ { //impType: "banner", - BannerImps: 1, + BannerImps: 5, VideoImps: 0, AudioImps: 0, NativeImps: 0, @@ -461,7 +499,7 @@ var impTypeLabels = []pbsmetrics.Labels{ { //impType: "audio", BannerImps: 0, - VideoImps: 1, + VideoImps: 9, AudioImps: 0, NativeImps: 0, }, @@ -469,7 +507,7 @@ var impTypeLabels = []pbsmetrics.Labels{ //impType: "video", BannerImps: 0, VideoImps: 0, - AudioImps: 1, + AudioImps: 8, NativeImps: 0, }, { @@ -477,7 +515,7 @@ var impTypeLabels = []pbsmetrics.Labels{ BannerImps: 0, VideoImps: 0, AudioImps: 0, - NativeImps: 1, + NativeImps: 3, }, } From 73e7676a19753229f60fa955b667e1c25a962ada Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Wed, 26 Jun 2019 18:45:13 -0400 Subject: [PATCH 12/17] refactored imp types to be boolean values inside a new ImpLabels type --- endpoints/auction.go | 10 +- endpoints/openrtb2/amp_auction.go | 12 ++- endpoints/openrtb2/auction.go | 14 +-- endpoints/openrtb2/video_auction.go | 15 ++- exchange/exchange.go | 17 ++- pbsmetrics/config/metrics.go | 6 +- pbsmetrics/config/metrics_test.go | 35 ++++--- pbsmetrics/go_metrics.go | 23 ++-- pbsmetrics/metrics.go | 27 +++-- pbsmetrics/metrics_mock.go | 4 +- pbsmetrics/prometheus/prometheus.go | 93 +++++++++++------ pbsmetrics/prometheus/prometheus_test.go | 127 +++++++++-------------- 12 files changed, 195 insertions(+), 188 deletions(-) diff --git a/endpoints/auction.go b/endpoints/auction.go index 091c659e6f2..b7973b18977 100644 --- a/endpoints/auction.go +++ b/endpoints/auction.go @@ -93,6 +93,12 @@ func (a *auction) auction(w http.ResponseWriter, r *http.Request, _ httprouter.P CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } + impLabels := pbsmetrics.ImpLabels{ + BannerImps: false, + VideoImps: false, + AudioImps: false, + NativeImps: false, + } if ua := user_agent.New(r.Header.Get("User-Agent")); ua != nil { name, _ := ua.Browser() if name == "Safari" { @@ -104,11 +110,11 @@ func (a *auction) auction(w http.ResponseWriter, r *http.Request, _ httprouter.P defer func() { if req == nil { a.metricsEngine.RecordRequest(labels) - a.metricsEngine.RecordImps(labels, 0) + a.metricsEngine.RecordImps(impLabels) } else { // handles the case that ParsePBSRequest returns an error, so req.Start is not defined a.metricsEngine.RecordRequest(labels) - a.metricsEngine.RecordImps(labels, len(req.AdUnits)) + a.metricsEngine.RecordImps(impLabels) a.metricsEngine.RecordRequestTime(labels, time.Since(req.Start)) } }() diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 9449d9eba47..5f4d18625f8 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -91,18 +91,20 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeAMP, - BannerImps: 0, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } + impLabels := pbsmetrics.ImpLabels{ + BannerImps: false, + VideoImps: false, + AudioImps: false, + NativeImps: false, + } defer func() { deps.metricsEngine.RecordRequest(labels) - deps.metricsEngine.RecordImps(labels, 1) + deps.metricsEngine.RecordImps(impLabels) deps.metricsEngine.RecordRequestTime(labels, time.Since(start)) deps.analytics.LogAmpObject(&ao) }() diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 311bf7481c1..6f8cc9d702f 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -89,19 +89,20 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeORTB2Web, - BannerImps: 0, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } - numImps := 0 + impLabels := pbsmetrics.ImpLabels{ + BannerImps: false, + VideoImps: false, + AudioImps: false, + NativeImps: false, + } defer func() { deps.metricsEngine.RecordRequest(labels) - deps.metricsEngine.RecordImps(labels, numImps) + deps.metricsEngine.RecordImps(impLabels) deps.metricsEngine.RecordRequestTime(labels, time.Since(start)) deps.analytics.LogAuctionObject(&ao) }() @@ -148,7 +149,6 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http } } - numImps = len(req.Imp) response, err := deps.ex.HoldAuction(ctx, req, usersyncs, labels, &deps.categories) ao.Request = req ao.Response = response diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index a229bba0ab7..30d8d99fecc 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -74,19 +74,20 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandUnknown, RType: pbsmetrics.ReqTypeVideo, - BannerImps: 0, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, PubID: "", Browser: pbsmetrics.BrowserOther, CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } - numImps := 0 + impLabels := pbsmetrics.ImpLabels{ + BannerImps: false, + VideoImps: false, + AudioImps: false, + NativeImps: false, + } defer func() { deps.metricsEngine.RecordRequest(labels) - deps.metricsEngine.RecordImps(labels, numImps) + deps.metricsEngine.RecordImps(impLabels) deps.metricsEngine.RecordRequestTime(labels, time.Since(start)) deps.analytics.LogAuctionObject(&ao) }() @@ -203,8 +204,6 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re } } - numImps = len(bidReq.Imp) - //execute auction logic response, err := deps.ex.HoldAuction(ctx, bidReq, usersyncs, labels, &deps.categories) ao.Request = bidReq diff --git a/exchange/exchange.go b/exchange/exchange.go index bc16df79cea..fe9c5bef4c0 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -86,18 +86,13 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque } for _, impInRequest := range bidRequest.Imp { - if impInRequest.Banner != nil { - labels.BannerImps += 1 - } - if impInRequest.Video != nil { - labels.VideoImps += 1 - } - if impInRequest.Audio != nil { - labels.AudioImps += 1 - } - if impInRequest.Native != nil { - labels.NativeImps += 1 + var impLabels pbsmetrics.ImpLabels = pbsmetrics.ImpLabels{ + BannerImps: impInRequest.Banner != nil, + VideoImps: impInRequest.Video != nil, + AudioImps: impInRequest.Audio != nil, + NativeImps: impInRequest.Native != nil, } + e.me.RecordImps(impLabels) } // Slice of BidRequests, each a copy of the original cleaned to only contain bidder data for the named bidder diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go index 2c494bd07ee..83283fdb185 100644 --- a/pbsmetrics/config/metrics.go +++ b/pbsmetrics/config/metrics.go @@ -84,9 +84,9 @@ func (me *MultiMetricsEngine) RecordConnectionClose(success bool) { } // RecordImps across all engines -func (me *MultiMetricsEngine) RecordImps(labels pbsmetrics.Labels, numImps int) { +func (me *MultiMetricsEngine) RecordImps(implabels pbsmetrics.ImpLabels) { for _, thisME := range *me { - thisME.RecordImps(labels, numImps) + thisME.RecordImps(implabels) } } @@ -186,7 +186,7 @@ func (me *DummyMetricsEngine) RecordConnectionClose(success bool) { } // RecordImps as a noop -func (me *DummyMetricsEngine) RecordImps(labels pbsmetrics.Labels, numImps int) { +func (me *DummyMetricsEngine) RecordImps(implabels pbsmetrics.ImpLabels) { return } diff --git a/pbsmetrics/config/metrics_test.go b/pbsmetrics/config/metrics_test.go index 8b6829a2cfd..1df035ccac4 100644 --- a/pbsmetrics/config/metrics_test.go +++ b/pbsmetrics/config/metrics_test.go @@ -46,10 +46,6 @@ func TestMultiMetricsEngine(t *testing.T) { labels := pbsmetrics.Labels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - BannerImps: 1, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, PubID: "test1", Browser: pbsmetrics.BrowserSafari, CookieFlag: pbsmetrics.CookieFlagYes, @@ -58,10 +54,6 @@ func TestMultiMetricsEngine(t *testing.T) { apnLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - BannerImps: 1, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, Adapter: openrtb_ext.BidderAppnexus, PubID: "test1", Browser: pbsmetrics.BrowserSafari, @@ -71,19 +63,21 @@ func TestMultiMetricsEngine(t *testing.T) { pubLabels := pbsmetrics.AdapterLabels{ Source: pbsmetrics.DemandWeb, RType: pbsmetrics.ReqTypeORTB2Web, - BannerImps: 1, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, Adapter: openrtb_ext.BidderPubmatic, PubID: "test1", Browser: pbsmetrics.BrowserSafari, CookieFlag: pbsmetrics.CookieFlagYes, AdapterBids: pbsmetrics.AdapterBidPresent, } + impTypeLabels := pbsmetrics.ImpLabels{ + BannerImps: true, + VideoImps: false, + AudioImps: true, + NativeImps: true, + } for i := 0; i < 5; i++ { metricsEngine.RecordRequest(labels) - metricsEngine.RecordImps(labels, 2) + metricsEngine.RecordImps(impTypeLabels) metricsEngine.RecordRequestTime(labels, time.Millisecond*20) metricsEngine.RecordAdapterRequest(pubLabels) metricsEngine.RecordAdapterRequest(apnLabels) @@ -91,6 +85,13 @@ func TestMultiMetricsEngine(t *testing.T) { metricsEngine.RecordAdapterBidReceived(pubLabels, openrtb_ext.BidTypeBanner, true) metricsEngine.RecordAdapterTime(pubLabels, time.Millisecond*20) } + impTypeLabels.BannerImps = false + impTypeLabels.VideoImps = true + impTypeLabels.AudioImps = false + impTypeLabels.NativeImps = false + for i := 0; i < 3; i++ { + metricsEngine.RecordImps(impTypeLabels) + } //Make the metrics engine, instantiated here with goEngine, fill its RequestStatuses[RequestType][pbsmetrics.RequestStatusXX] with the new boolean values added to pbsmetrics.Labels VerifyMetrics(t, "RequestStatuses.OpenRTB2.OK", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) VerifyMetrics(t, "RequestStatuses.Legacy.OK", goEngine.RequestStatuses[pbsmetrics.ReqTypeLegacy][pbsmetrics.RequestStatusOK].Count(), 0) @@ -102,12 +103,12 @@ func TestMultiMetricsEngine(t *testing.T) { VerifyMetrics(t, "RequestStatuses.OpenRTB2.BadInput", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusBadInput].Count(), 0) VerifyMetrics(t, "ImpsTypeBanner", goEngine.ImpsTypeBanner.Count(), 5) - VerifyMetrics(t, "ImpsTypeVideo", goEngine.ImpsTypeVideo.Count(), 0) - VerifyMetrics(t, "ImpsTypeAudio", goEngine.ImpsTypeAudio.Count(), 0) - VerifyMetrics(t, "ImpsTypeNative", goEngine.ImpsTypeNative.Count(), 0) + VerifyMetrics(t, "ImpsTypeVideo", goEngine.ImpsTypeVideo.Count(), 3) + VerifyMetrics(t, "ImpsTypeAudio", goEngine.ImpsTypeAudio.Count(), 5) + VerifyMetrics(t, "ImpsTypeNative", goEngine.ImpsTypeNative.Count(), 5) VerifyMetrics(t, "Request", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) - VerifyMetrics(t, "ImpMeter", goEngine.ImpMeter.Count(), 10) + VerifyMetrics(t, "ImpMeter", goEngine.ImpMeter.Count(), 8) VerifyMetrics(t, "NoCookieMeter", goEngine.NoCookieMeter.Count(), 0) VerifyMetrics(t, "SafariRequestMeter", goEngine.SafariRequestMeter.Count(), 5) VerifyMetrics(t, "SafariNoCookieMeter", goEngine.SafariNoCookieMeter.Count(), 0) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index 8b6867ca915..37746867151 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -149,8 +149,7 @@ func NewMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderName) * newMetrics.ConnectionCloseErrorMeter = metrics.GetOrRegisterMeter("connection_close_errors", registry) newMetrics.ImpMeter = metrics.GetOrRegisterMeter("imps_requested", registry) - // Find out how to GetOrRegisterMeter - newMetrics.ImpsTypeBanner = metrics.GetOrRegisterMeter("imp_banner", registry) //TODO: where is the "imp_banner" label or value? + newMetrics.ImpsTypeBanner = metrics.GetOrRegisterMeter("imp_banner", registry) newMetrics.ImpsTypeVideo = metrics.GetOrRegisterMeter("imp_video", registry) newMetrics.ImpsTypeAudio = metrics.GetOrRegisterMeter("imp_audio", registry) newMetrics.ImpsTypeNative = metrics.GetOrRegisterMeter("imp_native", registry) @@ -315,19 +314,19 @@ func (me *Metrics) RecordRequest(labels Labels) { am.requestMeter.Mark(1) } -func (me *Metrics) RecordImps(labels Labels, numImps int) { - me.ImpMeter.Mark(int64(numImps)) - if labels.BannerImps > 0 { - me.ImpsTypeBanner.Mark(int64(labels.BannerImps)) +func (me *Metrics) RecordImps(labels ImpLabels) { + me.ImpMeter.Mark(int64(1)) + if labels.BannerImps { + me.ImpsTypeBanner.Mark(int64(1)) } - if labels.VideoImps > 0 { - me.ImpsTypeVideo.Mark(int64(labels.VideoImps)) + if labels.VideoImps { + me.ImpsTypeVideo.Mark(int64(1)) } - if labels.AudioImps > 0 { - me.ImpsTypeAudio.Mark(int64(labels.AudioImps)) + if labels.AudioImps { + me.ImpsTypeAudio.Mark(int64(1)) } - if labels.NativeImps > 0 { - me.ImpsTypeNative.Mark(int64(labels.NativeImps)) + if labels.NativeImps { + me.ImpsTypeNative.Mark(int64(1)) } } diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 6a209f5e20f..3ffdcb19379 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -10,10 +10,6 @@ import ( type Labels struct { Source DemandSource RType RequestType - BannerImps int - VideoImps int - AudioImps int - NativeImps int PubID string // exchange specific ID, so we cannot compile in values Browser Browser CookieFlag CookieFlag @@ -24,10 +20,6 @@ type Labels struct { type AdapterLabels struct { Source DemandSource RType RequestType - BannerImps int - VideoImps int - AudioImps int - NativeImps int Adapter openrtb_ext.BidderName PubID string // exchange specific ID, so we cannot compile in values Browser Browser @@ -36,6 +28,14 @@ type AdapterLabels struct { AdapterErrors map[AdapterError]struct{} } +// ImpLabels +type ImpLabels struct { + BannerImps bool + VideoImps bool + AudioImps bool + NativeImps bool +} + // Label typecasting. Se below the type definitions for possible values // DemandSource : Demand source enumeration @@ -107,6 +107,15 @@ func RequestTypes() []RequestType { } } +func ImpTypes() []ImpMediaType { + return []ImpMediaType{ + ImpTypeBanner, + ImpTypeVideo, + ImpTypeAudio, + ImpTypeNative, + } +} + // Browser flag; at this point we only care about identifying Safari const ( BrowserSafari Browser = "safari" @@ -227,7 +236,7 @@ type MetricsEngine interface { RecordConnectionAccept(success bool) RecordConnectionClose(success bool) RecordRequest(labels Labels) // ignores adapter. only statusOk and statusErr fom status - RecordImps(labels Labels, numImps int) // ignores adapter. only statusOk and statusErr fom status + RecordImps(labels ImpLabels) // ignores adapter. only statusOk and statusErr fom status RecordRequestTime(labels Labels, length time.Duration) // ignores adapter. only statusOk and statusErr fom status RecordAdapterRequest(labels AdapterLabels) RecordAdapterPanic(labels AdapterLabels) diff --git a/pbsmetrics/metrics_mock.go b/pbsmetrics/metrics_mock.go index b25b48ed7e8..9663abeec51 100644 --- a/pbsmetrics/metrics_mock.go +++ b/pbsmetrics/metrics_mock.go @@ -31,8 +31,8 @@ func (me *MetricsEngineMock) RecordConnectionClose(success bool) { } // RecordImps mock -func (me *MetricsEngineMock) RecordImps(labels Labels, numImps int) { - me.Called(labels, numImps) +func (me *MetricsEngineMock) RecordImps(labels ImpLabels) { + me.Called(labels) return } diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index c9e90c586bd..1075285a6b7 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -1,7 +1,8 @@ package prometheusmetrics import ( - "strconv" + //"fmt" + //"strconv" "time" "github.com/prebid/prebid-server/config" @@ -17,7 +18,6 @@ type Metrics struct { connCounter prometheus.Gauge connError *prometheus.CounterVec imps *prometheus.CounterVec - impTypes *prometheus.CounterVec requests *prometheus.CounterVec reqTimer *prometheus.HistogramVec adaptRequests *prometheus.CounterVec @@ -45,7 +45,6 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { adapterLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_bid", "adapter"} bidLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_bid", "adapter", "bidtype", "markup_type"} errorLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_error", "adapter"} - impLabelNames := []string{"banner_imps", "video_imps", "audio_imps", "native_imps"} metrics := Metrics{} @@ -57,16 +56,12 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { []string{"ErrorType"}, ) metrics.Registry.MustRegister(metrics.connError) - metrics.imps = newCounter(cfg, "imps_requested_total", - "Total number of impressions requested through PBS.", - standardLabelNames, - ) - metrics.Registry.MustRegister(metrics.imps) - metrics.impTypes = newCounter(cfg, "imps_types_total", - "Total number of impression types requested through PBS.", + metrics.imps = newCounter(cfg, "imps_requested_total_by_type", + "Count of Impressions by type and in total requested through PBS.", + //standardLabelNames, impLabelNames, ) - metrics.Registry.MustRegister(metrics.impTypes) + metrics.Registry.MustRegister(metrics.imps) metrics.requests = newCounter(cfg, "requests_total", "Total number of requests made to PBS.", standardLabelNames, @@ -197,21 +192,21 @@ func (me *Metrics) RecordRequest(labels pbsmetrics.Labels) { me.requests.With(resolveLabels(labels)).Inc() } -func (me *Metrics) RecordImps(labels pbsmetrics.Labels, numImps int) { - me.imps.With(resolveLabels(labels)).Add(float64(numImps)) - var impLabels prometheus.Labels = resolveImpTypeLabels(labels) - if labels.BannerImps > 0 { - me.impTypes.With(impLabels).Add(float64(labels.BannerImps)) - } - if labels.VideoImps > 0 { - me.impTypes.With(impLabels).Add(float64(labels.VideoImps)) - } - if labels.AudioImps > 0 { - me.impTypes.With(impLabels).Add(float64(labels.AudioImps)) - } - if labels.NativeImps > 0 { - me.impTypes.With(impLabels).Add(float64(labels.NativeImps)) - } +func (me *Metrics) RecordImps(implabels pbsmetrics.ImpLabels) { + me.imps.With(resolveImpLabels(implabels)).Inc() + //var impLabels prometheus.Labels = resolveImpTypeLabels(labels) + //if labels.BannerImps { + // me.imps.With(promLabels).Inc() + //} + //if labels.VideoImps { + // me.imps.With(promLabels).Inc() + //} + //if labels.AudioImps { + // me.imps.With(promLabels).Inc() + //} + //if labels.NativeImps { + // me.imps.With(promLabels).Inc() + //} } func (me *Metrics) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { @@ -341,13 +336,26 @@ func resolveUserSyncLabels(userLabels pbsmetrics.UserLabels) prometheus.Labels { } } -func resolveImpTypeLabels(labels pbsmetrics.Labels) prometheus.Labels { - return prometheus.Labels{ - "banner_imps": strconv.Itoa(labels.BannerImps), - "video_imps": strconv.Itoa(labels.VideoImps), - "audio_imps": strconv.Itoa(labels.AudioImps), - "native_imps": strconv.Itoa(labels.NativeImps), +func resolveImpLabels(labels pbsmetrics.ImpLabels) prometheus.Labels { + var impLabels prometheus.Labels = prometheus.Labels{ + "banner_imps": "no", + "video_imps": "no", + "audio_imps": "no", + "native_imps": "no", + } + if labels.BannerImps { + impLabels["banner_imps"] = "yes" + } + if labels.VideoImps { + impLabels["video_imps"] = "yes" + } + if labels.AudioImps { + impLabels["audio_imps"] = "yes" } + if labels.NativeImps { + impLabels["native_imps"] = "yes" + } + return impLabels } // initializeTimeSeries precreates all possible metric label values, so there is no locking needed at run time creating new instances @@ -366,9 +374,10 @@ func initializeTimeSeries(m *Metrics) { adapterLabels := labels // save regenerating these dimensions for adapter status labels = addDimension(labels, "response_status", requestStatusesAsString()) for _, l := range labels { - _ = m.imps.With(l) + //a := m.imps.With(l) _ = m.requests.With(l) _ = m.reqTimer.With(l) + //fmt.Printf("%v \n", a) } // Adapter labels @@ -401,6 +410,15 @@ func initializeTimeSeries(m *Metrics) { _ = m.storedImpCacheResult.With(l) _ = m.storedReqCacheResult.With(l) } + + // ImpType labels + impTypeLabels := addDimension([]prometheus.Labels{}, "banner_imps", []string{"yes", "no"}) + impTypeLabels = addDimension(impTypeLabels, "video_imps", []string{"yes", "no"}) + impTypeLabels = addDimension(impTypeLabels, "audio_imps", []string{"yes", "no"}) + impTypeLabels = addDimension(impTypeLabels, "native_imps", []string{"yes", "no"}) + for _, l := range impTypeLabels { + _ = m.imps.With(l) + } } // addDimesion will expand a slice of labels to add the dimension of a new set of values for a new label name @@ -454,6 +472,15 @@ func requestTypesAsString() []string { return output } +func impTypesAsString() []string { + list := pbsmetrics.ImpTypes() + output := make([]string, len(list)) + for i, s := range list { + output[i] = string(s) + } + return output +} + func browserTypesAsString() []string { list := pbsmetrics.BrowserTypes() output := make([]string, len(list)) diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index 8d4c72906d9..e8c5078056b 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -78,89 +78,57 @@ func TestImpMetrics(t *testing.T) { metrics1 := dto.Metric{} metrics2 := dto.Metric{} metrics3 := dto.Metric{} - metrics4 := dto.Metric{} - - //proMetrics.imps.With(resolveLabels(labels[4)).Add(float64(5)) - proMetrics.RecordImps(labels[0], 1) - proMetrics.RecordImps(labels[1], 5) - proMetrics.RecordImps(labels[0], 2) - proMetrics.RecordImps(labels[2], 2) - proMetrics.RecordImps(labels[0], 7) - proMetrics.RecordImps(labels[2], 1) - proMetrics.RecordImps(labels[4], 1) - - proMetrics.imps.With(resolveLabels(labels[0])).Write(&metrics0) - proMetrics.imps.With(resolveLabels(labels[1])).Write(&metrics1) - proMetrics.imps.With(resolveLabels(labels[2])).Write(&metrics2) - proMetrics.imps.With(resolveLabels(labels[3])).Write(&metrics3) - proMetrics.imps.With(resolveLabels(labels[4])).Write(&metrics4) - - assertCounterValue(t, "imps_requested[0]", &metrics0, 10) - assertCounterValue(t, "imps_requested[1]", &metrics1, 5) - assertCounterValue(t, "imps_requested[2]", &metrics2, 3) - assertCounterValue(t, "imps_requested[3]", &metrics3, 0) - assertCounterValue(t, "imps_requested[4]", &metrics4, 1) -} -func TestImpTypeMetrics(t *testing.T) { - proMetrics := newTestMetricsEngine() - - metricsBanner := dto.Metric{} - metricsAudio := dto.Metric{} - metricsVideo := dto.Metric{} - metricsNative := dto.Metric{} - - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[0])).Add(float64(impTypeLabels[0].BannerImps)) - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[1])).Add(float64(impTypeLabels[1].VideoImps)) - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Add(float64(impTypeLabels[2].AudioImps)) - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Add(float64(impTypeLabels[3].NativeImps)) - - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[0])).Write(&metricsBanner) - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[1])).Write(&metricsAudio) - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[2])).Write(&metricsVideo) - proMetrics.impTypes.With(resolveImpTypeLabels(impTypeLabels[3])).Write(&metricsNative) - - assertCounterValue(t, "imps_types_banner_count", &metricsBanner, 5) - assertCounterValue(t, "imps_types_banner_count", &metricsAudio, 9) - assertCounterValue(t, "imps_types_banner_count", &metricsVideo, 8) - assertCounterValue(t, "imps_types_banner_count", &metricsNative, 3) + proMetrics.RecordImps(impTypeLabels[0]) + proMetrics.RecordImps(impTypeLabels[1]) + proMetrics.RecordImps(impTypeLabels[0]) + proMetrics.RecordImps(impTypeLabels[2]) + proMetrics.RecordImps(impTypeLabels[0]) + proMetrics.RecordImps(impTypeLabels[2]) + proMetrics.RecordImps(impTypeLabels[3]) + + proMetrics.imps.With(resolveImpLabels(impTypeLabels[0])).Write(&metrics0) + proMetrics.imps.With(resolveImpLabels(impTypeLabels[1])).Write(&metrics1) + proMetrics.imps.With(resolveImpLabels(impTypeLabels[2])).Write(&metrics2) + proMetrics.imps.With(resolveImpLabels(impTypeLabels[3])).Write(&metrics3) + + assertCounterValue(t, "imp_metrics[0]", &metrics0, 3) + assertCounterValue(t, "imp_metrics[1]", &metrics1, 1) + assertCounterValue(t, "imp_metrics[2]", &metrics2, 2) + assertCounterValue(t, "imp_metrics[3]", &metrics3, 1) } func TestRecordImps(t *testing.T) { var sampleEngine *Metrics = newTestMetricsEngine() var sampleMetrics dto.Metric = dto.Metric{} - var samplePBSLabels pbsmetrics.Labels = pbsmetrics.Labels{ - BannerImps: 5, - VideoImps: 4, - AudioImps: 3, - NativeImps: 2, + var samplePBSImpLabels pbsmetrics.ImpLabels = pbsmetrics.ImpLabels{ + BannerImps: false, + VideoImps: false, + AudioImps: false, + NativeImps: true, } - var samplePrometheusMetrics prometheus.Labels = resolveImpTypeLabels(samplePBSLabels) + var samplePrometheusMetrics prometheus.Labels = resolveImpLabels(samplePBSImpLabels) - sampleEngine.RecordImps(samplePBSLabels, 14) - sampleEngine.impTypes.With(samplePrometheusMetrics).Write(&sampleMetrics) + sampleEngine.RecordImps(samplePBSImpLabels) + sampleEngine.imps.With(samplePrometheusMetrics).Write(&sampleMetrics) for _, metricLabel := range sampleMetrics.Label { var impType string = *metricLabel.Name switch impType { case "banner_imps": - assert.Equal(t, *metricLabel.Value, "5", "'banner_imps' should equal '3'") + assert.Equal(t, *metricLabel.Value, "no", "'banner_imps' should equal 'no'") case "video_imps": - assert.Equal(t, *metricLabel.Value, "4", "'banner_imps' should equal '3'") + assert.Equal(t, *metricLabel.Value, "no", "'banner_imps' should equal 'no'") case "audio_imps": - assert.Equal(t, *metricLabel.Value, "3", "'banner_imps' should equal '3'") + assert.Equal(t, *metricLabel.Value, "no", "'banner_imps' should equal 'no'") case "native_imps": - assert.Equal(t, *metricLabel.Value, "2", "'banner_imps' should equal '3'") + assert.Equal(t, *metricLabel.Value, "yes", "'banner_imps' should equal 'yes'") default: fmt.Printf("'%s' is not recognized as a label \n", impType) } } - //fmt.Printf("%s \n", sampleMetrics.String()) - //assertCounterValue(t, "imps_types_banner_count", &sampleMetrics, 5) - //assertCounterValue(t, "imps_types_banner_count", &metricsAudio, 9) - //assertCounterValue(t, "imps_types_banner_count", &metricsVideo, 8) - //assertCounterValue(t, "imps_types_banner_count", &metricsNative, 3) + assertCounterValue(t, "imps_types_banner_count", &sampleMetrics, 1) } func TestTimerMetrics(t *testing.T) { @@ -488,34 +456,35 @@ var labels = []pbsmetrics.Labels{ RequestStatus: pbsmetrics.RequestStatusOK, }, } -var impTypeLabels = []pbsmetrics.Labels{ + +var impTypeLabels = []pbsmetrics.ImpLabels{ { //impType: "banner", - BannerImps: 5, - VideoImps: 0, - AudioImps: 0, - NativeImps: 0, + BannerImps: true, + VideoImps: false, + AudioImps: false, + NativeImps: false, }, { //impType: "audio", - BannerImps: 0, - VideoImps: 9, - AudioImps: 0, - NativeImps: 0, + BannerImps: false, + VideoImps: true, + AudioImps: false, + NativeImps: false, }, { //impType: "video", - BannerImps: 0, - VideoImps: 0, - AudioImps: 8, - NativeImps: 0, + BannerImps: false, + VideoImps: false, + AudioImps: true, + NativeImps: false, }, { //impType: "native", - BannerImps: 0, - VideoImps: 0, - AudioImps: 0, - NativeImps: 3, + BannerImps: false, + VideoImps: false, + AudioImps: false, + NativeImps: true, }, } From 569e6495369dfbaf4d896ee85896701e3e065fed Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 9 Jul 2019 11:27:19 -0400 Subject: [PATCH 13/17] refactoring prometheus.go --- pbsmetrics/prometheus/prometheus.go | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 1075285a6b7..62222347a6f 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -1,8 +1,6 @@ package prometheusmetrics import ( - //"fmt" - //"strconv" "time" "github.com/prebid/prebid-server/config" @@ -58,7 +56,6 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { metrics.Registry.MustRegister(metrics.connError) metrics.imps = newCounter(cfg, "imps_requested_total_by_type", "Count of Impressions by type and in total requested through PBS.", - //standardLabelNames, impLabelNames, ) metrics.Registry.MustRegister(metrics.imps) @@ -194,19 +191,6 @@ func (me *Metrics) RecordRequest(labels pbsmetrics.Labels) { func (me *Metrics) RecordImps(implabels pbsmetrics.ImpLabels) { me.imps.With(resolveImpLabels(implabels)).Inc() - //var impLabels prometheus.Labels = resolveImpTypeLabels(labels) - //if labels.BannerImps { - // me.imps.With(promLabels).Inc() - //} - //if labels.VideoImps { - // me.imps.With(promLabels).Inc() - //} - //if labels.AudioImps { - // me.imps.With(promLabels).Inc() - //} - //if labels.NativeImps { - // me.imps.With(promLabels).Inc() - //} } func (me *Metrics) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { @@ -374,10 +358,9 @@ func initializeTimeSeries(m *Metrics) { adapterLabels := labels // save regenerating these dimensions for adapter status labels = addDimension(labels, "response_status", requestStatusesAsString()) for _, l := range labels { - //a := m.imps.With(l) + _ = m.imps.With(l) _ = m.requests.With(l) _ = m.reqTimer.With(l) - //fmt.Printf("%v \n", a) } // Adapter labels From 44096ba0b745f5dbd63f875f76605d50e4b443f0 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 9 Jul 2019 18:20:30 -0400 Subject: [PATCH 14/17] Hans' refactoring and RecordLegacyImps() implemented --- endpoints/auction.go | 10 +- endpoints/openrtb2/amp_auction.go | 7 - endpoints/openrtb2/auction.go | 7 - endpoints/openrtb2/video_auction.go | 7 - pbsmetrics/config/metrics.go | 14 +- pbsmetrics/config/metrics_test.go | 2 + pbsmetrics/go_metrics.go | 7 + pbsmetrics/metrics.go | 5 +- pbsmetrics/metrics_mock.go | 6 + pbsmetrics/prometheus/prometheus.go | 156 ++++++++++++++--------- pbsmetrics/prometheus/prometheus_test.go | 46 +++++-- 11 files changed, 164 insertions(+), 103 deletions(-) diff --git a/endpoints/auction.go b/endpoints/auction.go index b7973b18977..98901d4368b 100644 --- a/endpoints/auction.go +++ b/endpoints/auction.go @@ -93,12 +93,6 @@ func (a *auction) auction(w http.ResponseWriter, r *http.Request, _ httprouter.P CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } - impLabels := pbsmetrics.ImpLabels{ - BannerImps: false, - VideoImps: false, - AudioImps: false, - NativeImps: false, - } if ua := user_agent.New(r.Header.Get("User-Agent")); ua != nil { name, _ := ua.Browser() if name == "Safari" { @@ -110,11 +104,11 @@ func (a *auction) auction(w http.ResponseWriter, r *http.Request, _ httprouter.P defer func() { if req == nil { a.metricsEngine.RecordRequest(labels) - a.metricsEngine.RecordImps(impLabels) + a.metricsEngine.RecordLegacyImps(labels, 0) } else { // handles the case that ParsePBSRequest returns an error, so req.Start is not defined a.metricsEngine.RecordRequest(labels) - a.metricsEngine.RecordImps(impLabels) + a.metricsEngine.RecordLegacyImps(labels, len(req.AdUnits)) a.metricsEngine.RecordRequestTime(labels, time.Since(req.Start)) } }() diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 5f4d18625f8..b3e19ae870a 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/endpoints/openrtb2/amp_auction.go @@ -96,15 +96,8 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } - impLabels := pbsmetrics.ImpLabels{ - BannerImps: false, - VideoImps: false, - AudioImps: false, - NativeImps: false, - } defer func() { deps.metricsEngine.RecordRequest(labels) - deps.metricsEngine.RecordImps(impLabels) deps.metricsEngine.RecordRequestTime(labels, time.Since(start)) deps.analytics.LogAmpObject(&ao) }() diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 6f8cc9d702f..83d0d9c6c54 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -94,15 +94,8 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } - impLabels := pbsmetrics.ImpLabels{ - BannerImps: false, - VideoImps: false, - AudioImps: false, - NativeImps: false, - } defer func() { deps.metricsEngine.RecordRequest(labels) - deps.metricsEngine.RecordImps(impLabels) deps.metricsEngine.RecordRequestTime(labels, time.Since(start)) deps.analytics.LogAuctionObject(&ao) }() diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index 30d8d99fecc..9b9179bd8ed 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -79,15 +79,8 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re CookieFlag: pbsmetrics.CookieFlagUnknown, RequestStatus: pbsmetrics.RequestStatusOK, } - impLabels := pbsmetrics.ImpLabels{ - BannerImps: false, - VideoImps: false, - AudioImps: false, - NativeImps: false, - } defer func() { deps.metricsEngine.RecordRequest(labels) - deps.metricsEngine.RecordImps(impLabels) deps.metricsEngine.RecordRequestTime(labels, time.Since(start)) deps.analytics.LogAuctionObject(&ao) }() diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go index 83283fdb185..d0d1349978b 100644 --- a/pbsmetrics/config/metrics.go +++ b/pbsmetrics/config/metrics.go @@ -83,13 +83,20 @@ func (me *MultiMetricsEngine) RecordConnectionClose(success bool) { } } -// RecordImps across all engines +// RecordImps across openRTB2 engines that support the 'Native' Imp type func (me *MultiMetricsEngine) RecordImps(implabels pbsmetrics.ImpLabels) { for _, thisME := range *me { thisME.RecordImps(implabels) } } +// RecordImps for the legacy endpoint +func (me *MultiMetricsEngine) RecordLegacyImps(labels pbsmetrics.Labels, numImps int) { + for _, thisME := range *me { + thisME.RecordLegacyImps(labels, numImps) + } +} + // RecordRequestTime across all engines func (me *MultiMetricsEngine) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { for _, thisME := range *me { @@ -190,6 +197,11 @@ func (me *DummyMetricsEngine) RecordImps(implabels pbsmetrics.ImpLabels) { return } +// RecordLegacyImps as a noop +func (me *DummyMetricsEngine) RecordLegacyImps(labels pbsmetrics.Labels, numImps int) { + return +} + // RecordRequestTime as a noop func (me *DummyMetricsEngine) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { return diff --git a/pbsmetrics/config/metrics_test.go b/pbsmetrics/config/metrics_test.go index 1df035ccac4..7ad60ca2085 100644 --- a/pbsmetrics/config/metrics_test.go +++ b/pbsmetrics/config/metrics_test.go @@ -78,6 +78,7 @@ func TestMultiMetricsEngine(t *testing.T) { for i := 0; i < 5; i++ { metricsEngine.RecordRequest(labels) metricsEngine.RecordImps(impTypeLabels) + metricsEngine.RecordLegacyImps(labels, 2) metricsEngine.RecordRequestTime(labels, time.Millisecond*20) metricsEngine.RecordAdapterRequest(pubLabels) metricsEngine.RecordAdapterRequest(apnLabels) @@ -109,6 +110,7 @@ func TestMultiMetricsEngine(t *testing.T) { VerifyMetrics(t, "Request", goEngine.RequestStatuses[pbsmetrics.ReqTypeORTB2Web][pbsmetrics.RequestStatusOK].Count(), 5) VerifyMetrics(t, "ImpMeter", goEngine.ImpMeter.Count(), 8) + VerifyMetrics(t, "LegacyImpMeter", goEngine.LegacyImpMeter.Count(), 10) VerifyMetrics(t, "NoCookieMeter", goEngine.NoCookieMeter.Count(), 0) VerifyMetrics(t, "SafariRequestMeter", goEngine.SafariRequestMeter.Count(), 5) VerifyMetrics(t, "SafariNoCookieMeter", goEngine.SafariNoCookieMeter.Count(), 0) diff --git a/pbsmetrics/go_metrics.go b/pbsmetrics/go_metrics.go index 37746867151..29e5f933a5a 100644 --- a/pbsmetrics/go_metrics.go +++ b/pbsmetrics/go_metrics.go @@ -17,6 +17,7 @@ type Metrics struct { ConnectionAcceptErrorMeter metrics.Meter ConnectionCloseErrorMeter metrics.Meter ImpMeter metrics.Meter + LegacyImpMeter metrics.Meter AppRequestMeter metrics.Meter NoCookieMeter metrics.Meter SafariRequestMeter metrics.Meter @@ -97,6 +98,7 @@ func NewBlankMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderNa ConnectionAcceptErrorMeter: blankMeter, ConnectionCloseErrorMeter: blankMeter, ImpMeter: blankMeter, + LegacyImpMeter: blankMeter, AppRequestMeter: blankMeter, NoCookieMeter: blankMeter, SafariRequestMeter: blankMeter, @@ -148,6 +150,7 @@ func NewMetrics(registry metrics.Registry, exchanges []openrtb_ext.BidderName) * newMetrics.ConnectionAcceptErrorMeter = metrics.GetOrRegisterMeter("connection_accept_errors", registry) newMetrics.ConnectionCloseErrorMeter = metrics.GetOrRegisterMeter("connection_close_errors", registry) newMetrics.ImpMeter = metrics.GetOrRegisterMeter("imps_requested", registry) + newMetrics.LegacyImpMeter = metrics.GetOrRegisterMeter("legacy_imps_requested", registry) newMetrics.ImpsTypeBanner = metrics.GetOrRegisterMeter("imp_banner", registry) newMetrics.ImpsTypeVideo = metrics.GetOrRegisterMeter("imp_video", registry) @@ -330,6 +333,10 @@ func (me *Metrics) RecordImps(labels ImpLabels) { } } +func (me *Metrics) RecordLegacyImps(labels Labels, numImps int) { + me.LegacyImpMeter.Mark(int64(numImps)) +} + func (me *Metrics) RecordConnectionAccept(success bool) { if success { me.ConnectionCounter.Inc(1) diff --git a/pbsmetrics/metrics.go b/pbsmetrics/metrics.go index 3ffdcb19379..fe79473f117 100644 --- a/pbsmetrics/metrics.go +++ b/pbsmetrics/metrics.go @@ -89,7 +89,7 @@ const ( ReqTypeVideo RequestType = "video" ) -// The media types described in the "imp" json objects TODO is this still needed? +// The media types described in the "imp" json objects const ( ImpTypeBanner ImpMediaType = "banner" ImpTypeVideo ImpMediaType = "video" @@ -236,7 +236,8 @@ type MetricsEngine interface { RecordConnectionAccept(success bool) RecordConnectionClose(success bool) RecordRequest(labels Labels) // ignores adapter. only statusOk and statusErr fom status - RecordImps(labels ImpLabels) // ignores adapter. only statusOk and statusErr fom status + RecordImps(labels ImpLabels) // RecordImps across openRTB2 engines that support the 'Native' Imp Type + RecordLegacyImps(labels Labels, numImps int) // RecordImps for the legacy engine RecordRequestTime(labels Labels, length time.Duration) // ignores adapter. only statusOk and statusErr fom status RecordAdapterRequest(labels AdapterLabels) RecordAdapterPanic(labels AdapterLabels) diff --git a/pbsmetrics/metrics_mock.go b/pbsmetrics/metrics_mock.go index 9663abeec51..d40637ef397 100644 --- a/pbsmetrics/metrics_mock.go +++ b/pbsmetrics/metrics_mock.go @@ -36,6 +36,12 @@ func (me *MetricsEngineMock) RecordImps(labels ImpLabels) { return } +// RecordLegacyImps mock +func (me *MetricsEngineMock) RecordLegacyImps(labels Labels, numImps int) { + me.Called(labels, numImps) + return +} + // RecordRequestTime mock func (me *MetricsEngineMock) RecordRequestTime(labels Labels, length time.Duration) { me.Called(labels, length) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 62222347a6f..c5c51b0757c 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -16,6 +16,7 @@ type Metrics struct { connCounter prometheus.Gauge connError *prometheus.CounterVec imps *prometheus.CounterVec + legacyImps *prometheus.CounterVec requests *prometheus.CounterVec reqTimer *prometheus.HistogramVec adaptRequests *prometheus.CounterVec @@ -31,6 +32,25 @@ type Metrics struct { storedImpCacheResult *prometheus.CounterVec } +const ( + requestTypeLabel = "request_type" + demandSourceLabel = "demand_source" + browserLabel = "browser" + cookieLabel = "cookie" + responseStatusLabel = "response_status" + adapterLabel = "adapter" + adapterBidLabel = "adapter_bid" + markupTypeLabel = "markup_type" + bidTypeLabel = "bid_type" + adapterErrLabel = "adapter_error" + cacheResultLabel = "cache_result" + gdprBlockedLabel = "gdpr_blocked" + bannerLabel = "banner" + videoLabel = "video" + audioLabel = "audio" + nativeLabel = "native" +) + // NewMetrics constructs the appropriate options for the Prometheus metrics. Needs to be fed the promethus config // Its own function to keep the metric creation function cleaner. func NewMetrics(cfg config.PrometheusMetrics) *Metrics { @@ -38,12 +58,12 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { timerBuckets := prometheus.LinearBuckets(0.05, 0.05, 20) timerBuckets = append(timerBuckets, []float64{1.5, 2.0, 3.0, 5.0, 10.0, 50.0}...) - standardLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "response_status"} + standardLabelNames := []string{demandSourceLabel, requestTypeLabel, browserLabel, cookieLabel, responseStatusLabel} - adapterLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_bid", "adapter"} - bidLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_bid", "adapter", "bidtype", "markup_type"} - errorLabelNames := []string{"demand_source", "request_type", "browser", "cookie", "adapter_error", "adapter"} - impLabelNames := []string{"banner_imps", "video_imps", "audio_imps", "native_imps"} + adapterLabelNames := []string{demandSourceLabel, requestTypeLabel, browserLabel, cookieLabel, adapterBidLabel, adapterLabel} + bidLabelNames := []string{demandSourceLabel, requestTypeLabel, browserLabel, cookieLabel, adapterBidLabel, adapterLabel, bidTypeLabel, markupTypeLabel} + errorLabelNames := []string{demandSourceLabel, requestTypeLabel, browserLabel, cookieLabel, adapterErrLabel, adapterLabel} + impLabelNames := []string{bannerLabel, videoLabel, audioLabel, nativeLabel} metrics := Metrics{} metrics.Registry = prometheus.NewRegistry() @@ -54,11 +74,16 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { []string{"ErrorType"}, ) metrics.Registry.MustRegister(metrics.connError) - metrics.imps = newCounter(cfg, "imps_requested_total_by_type", + metrics.imps = newCounter(cfg, "imps_requested", "Count of Impressions by type and in total requested through PBS.", impLabelNames, ) metrics.Registry.MustRegister(metrics.imps) + metrics.legacyImps = newCounter(cfg, "legacy_imps_requested", + "Total number of impressions requested through legacy PBS.", + standardLabelNames, + ) + metrics.Registry.MustRegister(metrics.legacyImps) metrics.requests = newCounter(cfg, "requests_total", "Total number of requests made to PBS.", standardLabelNames, @@ -113,7 +138,7 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { metrics.Registry.MustRegister(metrics.cookieSync) metrics.adaptCookieSync = newCounter(cfg, "cookie_sync_returns", "Number of syncs generated for a bidder, and if they were subsequently blocked.", - []string{"adapter", "gdpr_blocked"}, + []string{adapterLabel, gdprBlockedLabel}, ) metrics.Registry.MustRegister(metrics.adaptCookieSync) metrics.userID = newCounter(cfg, "setuid_calls", @@ -193,6 +218,12 @@ func (me *Metrics) RecordImps(implabels pbsmetrics.ImpLabels) { me.imps.With(resolveImpLabels(implabels)).Inc() } +func (me *Metrics) RecordLegacyImps(labels pbsmetrics.Labels, numImps int) { + var lbls prometheus.Labels + lbls = resolveLabels(labels) + me.legacyImps.With(lbls).Add(float64(numImps)) +} + func (me *Metrics) RecordRequestTime(labels pbsmetrics.Labels, length time.Duration) { time := float64(length) / float64(time.Second) me.reqTimer.With(resolveLabels(labels)).Observe(time) @@ -228,12 +259,12 @@ func (me *Metrics) RecordCookieSync(labels pbsmetrics.Labels) { func (me *Metrics) RecordAdapterCookieSync(adapter openrtb_ext.BidderName, gdprBlocked bool) { labels := prometheus.Labels{ - "adapter": string(adapter), + adapterLabel: string(adapter), } if gdprBlocked { - labels["gdpr_blocked"] = "true" + labels[gdprBlockedLabel] = "true" } else { - labels["gdpr_blocked"] = "false" + labels[gdprBlockedLabel] = "false" } me.adaptCookieSync.With(labels).Inc() } @@ -241,7 +272,7 @@ func (me *Metrics) RecordAdapterCookieSync(adapter openrtb_ext.BidderName, gdprB // RecordStoredReqCacheResult records cache hits and misses when looking up stored requests func (me *Metrics) RecordStoredReqCacheResult(cacheResult pbsmetrics.CacheResult, inc int) { labels := prometheus.Labels{ - "cache_result": string(cacheResult), + cacheResultLabel: string(cacheResult), } me.storedReqCacheResult.With(labels).Add(float64(inc)) @@ -250,7 +281,7 @@ func (me *Metrics) RecordStoredReqCacheResult(cacheResult pbsmetrics.CacheResult // RecordStoredImpCacheResult records cache hits and misses when looking up stored imps func (me *Metrics) RecordStoredImpCacheResult(cacheResult pbsmetrics.CacheResult, inc int) { labels := prometheus.Labels{ - "cache_result": string(cacheResult), + cacheResultLabel: string(cacheResult), } me.storedImpCacheResult.With(labels).Add(float64(inc)) @@ -262,54 +293,54 @@ func (me *Metrics) RecordUserIDSet(userLabels pbsmetrics.UserLabels) { func resolveLabels(labels pbsmetrics.Labels) prometheus.Labels { return prometheus.Labels{ - "demand_source": string(labels.Source), - "request_type": string(labels.RType), + demandSourceLabel: string(labels.Source), + requestTypeLabel: string(labels.RType), // "pubid": labels.PubID, - "browser": string(labels.Browser), - "cookie": string(labels.CookieFlag), - "response_status": string(labels.RequestStatus), + browserLabel: string(labels.Browser), + cookieLabel: string(labels.CookieFlag), + responseStatusLabel: string(labels.RequestStatus), } } func resolveAdapterLabels(labels pbsmetrics.AdapterLabels) prometheus.Labels { return prometheus.Labels{ - "demand_source": string(labels.Source), - "request_type": string(labels.RType), + demandSourceLabel: string(labels.Source), + requestTypeLabel: string(labels.RType), // "pubid": labels.PubID, - "browser": string(labels.Browser), - "cookie": string(labels.CookieFlag), - "adapter_bid": string(labels.AdapterBids), - "adapter": string(labels.Adapter), + browserLabel: string(labels.Browser), + cookieLabel: string(labels.CookieFlag), + adapterBidLabel: string(labels.AdapterBids), + adapterLabel: string(labels.Adapter), } } func resolveBidLabels(labels pbsmetrics.AdapterLabels, bidType openrtb_ext.BidType, hasAdm bool) prometheus.Labels { bidLabels := prometheus.Labels{ - "demand_source": string(labels.Source), - "request_type": string(labels.RType), + demandSourceLabel: string(labels.Source), + requestTypeLabel: string(labels.RType), // "pubid": labels.PubID, - "browser": string(labels.Browser), - "cookie": string(labels.CookieFlag), - "adapter_bid": string(labels.AdapterBids), - "adapter": string(labels.Adapter), - "bidtype": string(bidType), - "markup_type": "unknown", + browserLabel: string(labels.Browser), + cookieLabel: string(labels.CookieFlag), + adapterBidLabel: string(labels.AdapterBids), + adapterLabel: string(labels.Adapter), + bidTypeLabel: string(bidType), + markupTypeLabel: "unknown", } if hasAdm { - bidLabels["markup_type"] = "adm" + bidLabels[markupTypeLabel] = "adm" } return bidLabels } func resolveAdapterErrorLabels(labels pbsmetrics.AdapterLabels, errorType string) prometheus.Labels { return prometheus.Labels{ - "demand_source": string(labels.Source), - "request_type": string(labels.RType), + demandSourceLabel: string(labels.Source), + requestTypeLabel: string(labels.RType), // "pubid": labels.PubID, - "browser": string(labels.Browser), - "cookie": string(labels.CookieFlag), - "adapter_error": errorType, - "adapter": string(labels.Adapter), + browserLabel: string(labels.Browser), + cookieLabel: string(labels.CookieFlag), + adapterErrLabel: errorType, + adapterLabel: string(labels.Adapter), } } @@ -322,22 +353,22 @@ func resolveUserSyncLabels(userLabels pbsmetrics.UserLabels) prometheus.Labels { func resolveImpLabels(labels pbsmetrics.ImpLabels) prometheus.Labels { var impLabels prometheus.Labels = prometheus.Labels{ - "banner_imps": "no", - "video_imps": "no", - "audio_imps": "no", - "native_imps": "no", + bannerLabel: "no", + videoLabel: "no", + audioLabel: "no", + nativeLabel: "no", } if labels.BannerImps { - impLabels["banner_imps"] = "yes" + impLabels[bannerLabel] = "yes" } if labels.VideoImps { - impLabels["video_imps"] = "yes" + impLabels[videoLabel] = "yes" } if labels.AudioImps { - impLabels["audio_imps"] = "yes" + impLabels[audioLabel] = "yes" } if labels.NativeImps { - impLabels["native_imps"] = "yes" + impLabels[nativeLabel] = "yes" } return impLabels } @@ -351,22 +382,21 @@ func initializeTimeSeries(m *Metrics) { } // Standard labels - labels = addDimension([]prometheus.Labels{}, "demand_source", demandTypesAsString()) - labels = addDimension(labels, "request_type", requestTypesAsString()) - labels = addDimension(labels, "browser", browserTypesAsString()) - labels = addDimension(labels, "cookie", cookieTypesAsString()) + labels = addDimension([]prometheus.Labels{}, demandSourceLabel, demandTypesAsString()) + labels = addDimension(labels, requestTypeLabel, requestTypesAsString()) + labels = addDimension(labels, browserLabel, browserTypesAsString()) + labels = addDimension(labels, cookieLabel, cookieTypesAsString()) adapterLabels := labels // save regenerating these dimensions for adapter status - labels = addDimension(labels, "response_status", requestStatusesAsString()) + labels = addDimension(labels, responseStatusLabel, requestStatusesAsString()) for _, l := range labels { - _ = m.imps.With(l) _ = m.requests.With(l) _ = m.reqTimer.With(l) } // Adapter labels - labels = addDimension(adapterLabels, "adapter", adaptersAsString()) + labels = addDimension(adapterLabels, adapterLabel, adaptersAsString()) errorLabels := labels // save regenerating these dimensions for adapter errors - labels = addDimension(labels, "adapter_bid", adapterBidsAsString()) + labels = addDimension(labels, adapterBidLabel, adapterBidsAsString()) for _, l := range labels { _ = m.adaptRequests.With(l) _ = m.adaptTimer.With(l) @@ -374,17 +404,17 @@ func initializeTimeSeries(m *Metrics) { _ = m.adaptPanics.With(l) } // AdapterBid labels - labels = addDimension(labels, "bidtype", bidTypesAsString()) - labels = addDimension(labels, "markup_type", []string{"unknown", "adm"}) + labels = addDimension(labels, bidTypeLabel, bidTypesAsString()) + labels = addDimension(labels, markupTypeLabel, []string{"unknown", "adm"}) for _, l := range labels { _ = m.adaptBids.With(l) } - labels = addDimension(errorLabels, "adapter_error", adapterErrorsAsString()) + labels = addDimension(errorLabels, adapterErrLabel, adapterErrorsAsString()) for _, l := range labels { _ = m.adaptErrors.With(l) } - cookieLabels := addDimension([]prometheus.Labels{}, "adapter", adaptersAsString()) - cookieLabels = addDimension(cookieLabels, "gdpr_blocked", []string{"true", "false"}) + cookieLabels := addDimension([]prometheus.Labels{}, adapterLabel, adaptersAsString()) + cookieLabels = addDimension(cookieLabels, gdprBlockedLabel, []string{"true", "false"}) for _, l := range cookieLabels { _ = m.adaptCookieSync.With(l) } @@ -395,10 +425,10 @@ func initializeTimeSeries(m *Metrics) { } // ImpType labels - impTypeLabels := addDimension([]prometheus.Labels{}, "banner_imps", []string{"yes", "no"}) - impTypeLabels = addDimension(impTypeLabels, "video_imps", []string{"yes", "no"}) - impTypeLabels = addDimension(impTypeLabels, "audio_imps", []string{"yes", "no"}) - impTypeLabels = addDimension(impTypeLabels, "native_imps", []string{"yes", "no"}) + impTypeLabels := addDimension([]prometheus.Labels{}, bannerLabel, []string{"yes", "no"}) + impTypeLabels = addDimension(impTypeLabels, videoLabel, []string{"yes", "no"}) + impTypeLabels = addDimension(impTypeLabels, audioLabel, []string{"yes", "no"}) + impTypeLabels = addDimension(impTypeLabels, nativeLabel, []string{"yes", "no"}) for _, l := range impTypeLabels { _ = m.imps.With(l) } diff --git a/pbsmetrics/prometheus/prometheus_test.go b/pbsmetrics/prometheus/prometheus_test.go index e8c5078056b..0930e3fee1a 100644 --- a/pbsmetrics/prometheus/prometheus_test.go +++ b/pbsmetrics/prometheus/prometheus_test.go @@ -71,6 +71,36 @@ func TestRequestMetrics(t *testing.T) { assertCounterValue(t, "requests[4]", &metrics4, 1) } +func TestLegacyImpMetrics(t *testing.T) { + proMetrics := newTestMetricsEngine() + + metrics0 := dto.Metric{} + metrics1 := dto.Metric{} + metrics2 := dto.Metric{} + metrics3 := dto.Metric{} + metrics4 := dto.Metric{} + + proMetrics.RecordLegacyImps(labels[0], 1) + proMetrics.RecordLegacyImps(labels[1], 5) + proMetrics.RecordLegacyImps(labels[0], 2) + proMetrics.RecordLegacyImps(labels[2], 2) + proMetrics.RecordLegacyImps(labels[0], 7) + proMetrics.RecordLegacyImps(labels[2], 1) + proMetrics.RecordLegacyImps(labels[4], 1) + + proMetrics.legacyImps.With(resolveLabels(labels[0])).Write(&metrics0) + proMetrics.legacyImps.With(resolveLabels(labels[1])).Write(&metrics1) + proMetrics.legacyImps.With(resolveLabels(labels[2])).Write(&metrics2) + proMetrics.legacyImps.With(resolveLabels(labels[3])).Write(&metrics3) + proMetrics.legacyImps.With(resolveLabels(labels[4])).Write(&metrics4) + + assertCounterValue(t, "legacyImps_requested[0]", &metrics0, 10) + assertCounterValue(t, "legacyImps_requested[1]", &metrics1, 5) + assertCounterValue(t, "legacyImps_requested[2]", &metrics2, 3) + assertCounterValue(t, "legacyImps_requested[3]", &metrics3, 0) + assertCounterValue(t, "legacyImps_requested[4]", &metrics4, 1) +} + func TestImpMetrics(t *testing.T) { proMetrics := newTestMetricsEngine() @@ -115,14 +145,14 @@ func TestRecordImps(t *testing.T) { for _, metricLabel := range sampleMetrics.Label { var impType string = *metricLabel.Name switch impType { - case "banner_imps": - assert.Equal(t, *metricLabel.Value, "no", "'banner_imps' should equal 'no'") - case "video_imps": - assert.Equal(t, *metricLabel.Value, "no", "'banner_imps' should equal 'no'") - case "audio_imps": - assert.Equal(t, *metricLabel.Value, "no", "'banner_imps' should equal 'no'") - case "native_imps": - assert.Equal(t, *metricLabel.Value, "yes", "'banner_imps' should equal 'yes'") + case "banner": + assert.Equal(t, *metricLabel.Value, "no", "'banner") + case "video": + assert.Equal(t, *metricLabel.Value, "no", "'video' should equal 'no'") + case "audio": + assert.Equal(t, *metricLabel.Value, "no", "'audio' should equal 'no'") + case "native": + assert.Equal(t, *metricLabel.Value, "yes", "'native' should equal 'yes'") default: fmt.Printf("'%s' is not recognized as a label \n", impType) } From 5c6442cf640c1dd6fbc86782f4c8b5fee3b0edc4 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 16 Jul 2019 11:09:26 -0400 Subject: [PATCH 15/17] corrected .travis.yml --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 59fe5274bd6..41426225a20 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,8 @@ language: go go: - - '1.9' - - '1.10' - '1.11.1' + - '1.12' go_import_path: github.com/prebid/prebid-server From 36e4a6a8b1e7384611e59e62348b96e90ccb3cb2 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 16 Jul 2019 11:37:29 -0400 Subject: [PATCH 16/17] trying to fix travis issue --- pbsmetrics/prometheus/prometheus.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index c9dbbb74d3f..9ddcbcca555 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -66,7 +66,6 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics { impLabelNames := []string{bannerLabel, videoLabel, audioLabel, nativeLabel} - metrics := Metrics{} metrics.Registry = prometheus.NewRegistry() metrics.connCounter = newConnCounter(cfg) From f0d29761066fd7dfbca8002d1311803a9e0b801a Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Wed, 17 Jul 2019 15:08:29 -0400 Subject: [PATCH 17/17] Small changes after Mansi's review --- pbsmetrics/config/metrics.go | 2 +- pbsmetrics/prometheus/prometheus.go | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/pbsmetrics/config/metrics.go b/pbsmetrics/config/metrics.go index d0d1349978b..fa25fb504ad 100644 --- a/pbsmetrics/config/metrics.go +++ b/pbsmetrics/config/metrics.go @@ -83,7 +83,7 @@ func (me *MultiMetricsEngine) RecordConnectionClose(success bool) { } } -// RecordImps across openRTB2 engines that support the 'Native' Imp type +//RecordsImps records imps with imp types across all metric engines func (me *MultiMetricsEngine) RecordImps(implabels pbsmetrics.ImpLabels) { for _, thisME := range *me { thisME.RecordImps(implabels) diff --git a/pbsmetrics/prometheus/prometheus.go b/pbsmetrics/prometheus/prometheus.go index 9ddcbcca555..f10a4fa316a 100644 --- a/pbsmetrics/prometheus/prometheus.go +++ b/pbsmetrics/prometheus/prometheus.go @@ -486,15 +486,6 @@ func requestTypesAsString() []string { return output } -func impTypesAsString() []string { - list := pbsmetrics.ImpTypes() - output := make([]string, len(list)) - for i, s := range list { - output[i] = string(s) - } - return output -} - func browserTypesAsString() []string { list := pbsmetrics.BrowserTypes() output := make([]string, len(list))