Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Native-Specific Metrics to Prebid Server #930

Merged
merged 19 commits into from
Jul 18, 2019
Merged

Add Native-Specific Metrics to Prebid Server #930

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
00f48e3
going out real quick, brb
Jun 5, 2019
6d8bf61
brb to attend 395
Jun 5, 2019
654fb2c
Test cases still not ready, but touching base to see if we are on the…
Jun 7, 2019
aaa9a00
removed some commented lines
Jun 10, 2019
5d7fa8f
Counting Imp types inside go_metrics.RecordImps function
Jun 14, 2019
bcd4b12
Added >0 check in go_metrics.RecordImps
Jun 14, 2019
d484938
working on the prometheus part
Jun 17, 2019
d5a6afe
going out real quick
Jun 18, 2019
d919002
Finally working as a counterVector for banner. Replicate for the others
Jun 20, 2019
5d76416
Prometheus implemented
Jun 20, 2019
7661993
Test case for RecordImps() function
Jun 20, 2019
73e7676
refactored imp types to be boolean values inside a new ImpLabels type
Jun 26, 2019
569e649
refactoring prometheus.go
Jul 9, 2019
44096ba
Hans' refactoring and RecordLegacyImps() implemented
Jul 9, 2019
63dd6c1
Merge branch 'master' into PBS-108
guscarreon Jul 15, 2019
5c6442c
corrected .travis.yml
Jul 16, 2019
789d60f
Merge branch 'PBS-108' of https://github.com/guscarreon/prebid-server…
Jul 16, 2019
36e4a6a
trying to fix travis issue
Jul 16, 2019
f0d2976
Small changes after Mansi's review
Jul 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions endpoints/openrtb2/amp_auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ 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,
Expand Down
4 changes: 4 additions & 0 deletions endpoints/openrtb2/auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ 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,
Expand Down
4 changes: 4 additions & 0 deletions endpoints/openrtb2/video_auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ 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,
Expand Down
15 changes: 15 additions & 0 deletions exchange/exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,21 @@ func (e *exchange) HoldAuction(ctx context.Context, bidRequest *openrtb.BidReque
}
}

for _, impInRequest := range bidRequest.Imp {
guscarreon marked this conversation as resolved.
Show resolved Hide resolved
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
}
}

// 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)
Expand Down
19 changes: 19 additions & 0 deletions pbsmetrics/config/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ 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,
Expand All @@ -54,6 +58,10 @@ 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,
Expand All @@ -63,6 +71,10 @@ 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,
Expand All @@ -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)
Expand All @@ -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)

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)
VerifyMetrics(t, "NoCookieMeter", goEngine.NoCookieMeter.Count(), 0)
Expand Down
30 changes: 30 additions & 0 deletions pbsmetrics/go_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
accountMetrics map[string]*accountMetrics
Expand Down Expand Up @@ -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),

ImpsTypeBanner: blankMeter,
ImpsTypeVideo: blankMeter,
ImpsTypeAudio: blankMeter,
ImpsTypeNative: blankMeter,

AdapterMetrics: make(map[openrtb_ext.BidderName]*AdapterMetrics, len(exchanges)),
accountMetrics: make(map[string]*accountMetrics),

Expand Down Expand Up @@ -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.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)
newMetrics.AppRequestMeter = metrics.GetOrRegisterMeter("app_requests", registry)
Expand Down Expand Up @@ -299,6 +317,18 @@ func (me *Metrics) RecordRequest(labels Labels) {

func (me *Metrics) RecordImps(labels Labels, numImps int) {
me.ImpMeter.Mark(int64(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) RecordConnectionAccept(success bool) {
Expand Down
19 changes: 19 additions & 0 deletions pbsmetrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ 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
Expand All @@ -20,6 +24,10 @@ 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
Expand All @@ -33,6 +41,9 @@ type AdapterLabels struct {
// DemandSource : Demand source enumeration
type DemandSource string

// ImpMediaType : Media type described in the "imp" JSON object TODO is this still needed?
type ImpMediaType string

// RequestType : Request type enumeration
type RequestType string

Expand Down Expand Up @@ -78,6 +89,14 @@ const (
ReqTypeVideo RequestType = "video"
)

// The media types described in the "imp" json objects TODO is this still needed?
const (
ImpTypeBanner ImpMediaType = "banner"
ImpTypeVideo ImpMediaType = "video"
ImpTypeAudio ImpMediaType = "audio"
ImpTypeNative ImpMediaType = "native"
)

func RequestTypes() []RequestType {
return []RequestType{
ReqTypeLegacy,
Expand Down
31 changes: 31 additions & 0 deletions pbsmetrics/prometheus/prometheus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package prometheusmetrics

import (
"strconv"
"time"

"github.com/prebid/prebid-server/config"
Expand All @@ -16,6 +17,7 @@ type Metrics struct {
connCounter prometheus.Gauge
connError *prometheus.CounterVec
imps *prometheus.CounterVec
impTypes *prometheus.CounterVec
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it may be best to refactor RecordImps() rather than add impTypes as a redundant metric. The key here is prometheus is set up so it could easily track multiformat counts. The way it is currently coded, if you have an imp that allows both banner and video ads, and you ask what the total number of imps are, it will tell you two. But if you refactor how imps are recorded, you can preserve the information that there was one imp, and it was valid as both a banner imp and a video imp.

What we would want to do is loop over the imps, and call RecordImps() once per imp. We would make the values for the imp types boolean, and then add one imp recording with the proper flags for each call. This will allow us to track the true total number of imps along with counts for each imp type.

We probably want to have a new ImpLabels type to help keep things clean, and make it clear that the imp type booleans only apply to RecordImps() and not the other metrics.

This will also require the influx metrics to be refactored a bit.

requests *prometheus.CounterVec
reqTimer *prometheus.HistogramVec
adaptRequests *prometheus.CounterVec
Expand Down Expand Up @@ -44,6 +46,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_imps", "video_imps", "audio_imps", "native_imps"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comparing to the latest master, I see all these label names have been converted to constants ... probably should do the same for impLabelNames

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also can drop _imps from the label name, as these are labels on the imps_requested_total counter. Looking at imps_requested_total.banner_imps looks a bit redundant. :)


metrics := Metrics{}
metrics.Registry = prometheus.NewRegistry()
metrics.connCounter = newConnCounter(cfg)
Expand All @@ -58,6 +62,11 @@ func NewMetrics(cfg config.PrometheusMetrics) *Metrics {
standardLabelNames,
)
metrics.Registry.MustRegister(metrics.imps)
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,
Expand Down Expand Up @@ -190,6 +199,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))
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) {
Expand Down Expand Up @@ -319,6 +341,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
Expand Down
Loading