diff --git a/adapters/vastbidder/etree_parser.go b/adapters/vastbidder/etree_parser.go index ed8163783a5..fcf06a40c98 100644 --- a/adapters/vastbidder/etree_parser.go +++ b/adapters/vastbidder/etree_parser.go @@ -2,7 +2,6 @@ package vastbidder import ( "strconv" - "strings" "github.com/beevik/etree" "github.com/prebid/prebid-server/v2/openrtb_ext" @@ -70,7 +69,7 @@ func (p *etreeXMLParser) GetPricingDetails() (price float64, currency string) { return 0.0, "" } - priceValue, err := strconv.ParseFloat(strings.TrimSpace(node.Text()), 64) + priceValue, err := strconv.ParseFloat(node.TrimmedText(), 64) if nil != err { return 0.0, "" } @@ -89,7 +88,7 @@ func (p *etreeXMLParser) GetAdvertiser() (advertisers []string) { if ext.SelectAttrValue("type", "") == "advertiser" { ele := ext.SelectElement("Advertiser") if ele != nil { - if value := strings.TrimSpace(ele.Text()); len(value) > 0 { + if value := ele.TrimmedText(); len(value) > 0 { advertisers = append(advertisers, value) } } @@ -98,7 +97,7 @@ func (p *etreeXMLParser) GetAdvertiser() (advertisers []string) { case vastVersion4x: if ele := p.adElement.SelectElement("Advertiser"); ele != nil { - if value := strings.TrimSpace(ele.Text()); len(value) > 0 { + if value := ele.TrimmedText(); len(value) > 0 { advertisers = append(advertisers, value) } } @@ -126,7 +125,7 @@ func (p *etreeXMLParser) GetDuration() (int, error) { if node == nil { return 0, errEmptyVideoDuration } - return parseDuration(strings.TrimSpace(node.Text())) + return parseDuration(node.TrimmedText()) } func (p *etreeXMLParser) getAdElement(vast *etree.Element) *etree.Element { diff --git a/adapters/vastbidder/etree_parser_test.go b/adapters/vastbidder/etree_parser_test.go index dd50ded2dad..43b79a46562 100644 --- a/adapters/vastbidder/etree_parser_test.go +++ b/adapters/vastbidder/etree_parser_test.go @@ -342,6 +342,12 @@ func getPricingDetailsTestCases() []struct { wantPrice: 0, wantCurrency: "", }, + { + name: "bug", + vastXML: "\n\t\n\t\t\n\t\t\tPubMatic\n\t\t\tscenario_9\n\t\t\tTest Ad scenario_9\n\t\t\t\n\t\t\t\t\n\t\t\t\n\t\t\t\n\t\t\t\t\n\t\t\t\n\t\t\tTest Advertiser\n\t\t\tIAB1-1\t\n\t\t\t\n\t\t\t\t\n\t\t\t\t\t8465_scenario_9\n\t\t\t\t\t\n\t\t\t\t\t\t00:00:10\n\t\t\t\t\t\t\n\t\t\t\t\t\t\t\n\t\t\t\t\t\t\t\t\n\t\t\t\t\t\t\t\n\t\t\t\t\t\t\n\t\t\t\t\t\n\t\t\t\t\n\t\t\t\n\t\t\n\t\n ", + wantPrice: 5, + wantCurrency: "USD", + }, // TODO: Add test cases. } } diff --git a/go.mod b/go.mod index 4fcbd4a1f1c..fdf70090dc8 100644 --- a/go.mod +++ b/go.mod @@ -100,6 +100,6 @@ replace github.com/prebid/prebid-server/v2 => ./ replace github.com/prebid/openrtb/v20 => github.com/PubMatic-OpenWrap/prebid-openrtb/v20 v20.0.0-20240222072752-2d647d1707ef -replace github.com/beevik/etree v1.0.2 => github.com/PubMatic-OpenWrap/etree v1.0.2-0.20240909135535-5d3df9e9a959 +replace github.com/beevik/etree v1.0.2 => github.com/PubMatic-OpenWrap/etree v1.0.2-0.20240914050009-a916f68552f5 replace github.com/beevik/etree/110 => github.com/beevik/etree v1.1.0 diff --git a/go.sum b/go.sum index 50bc4a5977d..51dad9957bb 100644 --- a/go.sum +++ b/go.sum @@ -65,8 +65,8 @@ github.com/IABTechLab/adscert v0.34.0/go.mod h1:pCLd3Up1kfTrH6kYFUGGeavxIc1f6Tvv github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I= github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= -github.com/PubMatic-OpenWrap/etree v1.0.2-0.20240909135535-5d3df9e9a959 h1:UM0zl3LogmkLc7/ttGDw+UmuWaVUQHcxUop4c/g6yto= -github.com/PubMatic-OpenWrap/etree v1.0.2-0.20240909135535-5d3df9e9a959/go.mod h1:5Y8qgcuDoy3XXG907UXkGnVTwihF16rXyJa4zRT7hOE= +github.com/PubMatic-OpenWrap/etree v1.0.2-0.20240914050009-a916f68552f5 h1:qNRDZVW/TJI0O4hPVdk5YCl+WxD6alYdaCG0im73lNo= +github.com/PubMatic-OpenWrap/etree v1.0.2-0.20240914050009-a916f68552f5/go.mod h1:5Y8qgcuDoy3XXG907UXkGnVTwihF16rXyJa4zRT7hOE= github.com/PubMatic-OpenWrap/fastxml v0.0.0-20240826060652-d9d5d05fdad2 h1:4zaGImZVnKCJudxKfsVNJAqGhCPxbjApAo0QvEThwpw= github.com/PubMatic-OpenWrap/fastxml v0.0.0-20240826060652-d9d5d05fdad2/go.mod h1:TGGzSA5ziWpfLsKvqOzgdPGEZ7SJIQjHbcJw6lWoyHU= github.com/PubMatic-OpenWrap/prebid-openrtb/v20 v20.0.0-20240222072752-2d647d1707ef h1:CXsyYtEgZz/0++fiug6QZXrRYG6BBrl9IGveCxsnhiE= diff --git a/modules/pubmatic/openwrap/metrics/config/multimetrics.go b/modules/pubmatic/openwrap/metrics/config/multimetrics.go index 72a31262ea5..405a15cb20e 100644 --- a/modules/pubmatic/openwrap/metrics/config/multimetrics.go +++ b/modules/pubmatic/openwrap/metrics/config/multimetrics.go @@ -529,3 +529,17 @@ func (me *MultiMetricsEngine) RecordSignalDataStatus(pubid, profileid, signalTyp thisME.RecordSignalDataStatus(pubid, profileid, signalType) } } + +// RecordBidRecoveryStatus across all engines +func (me *MultiMetricsEngine) RecordBidRecoveryStatus(publisher, profile string, success bool) { + for _, thisME := range *me { + thisME.RecordBidRecoveryStatus(publisher, profile, success) + } +} + +// RecordBidRecoveryResponseTime across all engines +func (me *MultiMetricsEngine) RecordBidRecoveryResponseTime(publisher, profile string, responseTime time.Duration) { + for _, thisME := range *me { + thisME.RecordBidRecoveryResponseTime(publisher, profile, responseTime) + } +} diff --git a/modules/pubmatic/openwrap/metrics/config/multimetrics_test.go b/modules/pubmatic/openwrap/metrics/config/multimetrics_test.go index 2b206de8715..554ae4dc438 100644 --- a/modules/pubmatic/openwrap/metrics/config/multimetrics_test.go +++ b/modules/pubmatic/openwrap/metrics/config/multimetrics_test.go @@ -223,6 +223,8 @@ func TestRecordFunctionForMultiMetricsEngine(t *testing.T) { mockEngine.EXPECT().RecordOWServerPanic("endpoint", "methodName", "nodeName", "podName") mockEngine.EXPECT().RecordAmpVideoRequests("pubid", "profileid") mockEngine.EXPECT().RecordAmpVideoResponses("pubid", "profileid") + mockEngine.EXPECT().RecordBidRecoveryStatus(publisher, profile, true) + mockEngine.EXPECT().RecordBidRecoveryResponseTime(publisher, profile, time.Duration(200)) // create the multi-metric engine multiMetricEngine := MultiMetricsEngine{} @@ -291,4 +293,6 @@ func TestRecordFunctionForMultiMetricsEngine(t *testing.T) { multiMetricEngine.RecordOWServerPanic("endpoint", "methodName", "nodeName", "podName") multiMetricEngine.RecordAmpVideoRequests("pubid", "profileid") multiMetricEngine.RecordAmpVideoResponses("pubid", "profileid") + multiMetricEngine.RecordBidRecoveryStatus(publisher, profile, true) + multiMetricEngine.RecordBidRecoveryResponseTime(publisher, profile, time.Duration(200)) } diff --git a/modules/pubmatic/openwrap/metrics/metrics.go b/modules/pubmatic/openwrap/metrics/metrics.go index dfe45a8d500..6af13a88689 100644 --- a/modules/pubmatic/openwrap/metrics/metrics.go +++ b/modules/pubmatic/openwrap/metrics/metrics.go @@ -25,6 +25,8 @@ type MetricsEngine interface { RecordPublisherRequests(endpoint string, publisher string, platform string) RecordReqImpsWithContentCount(publisher, contentType string) RecordInjectTrackerErrorCount(adformat, publisher, partner string) + RecordBidRecoveryStatus(publisher, profile string, success bool) + RecordBidRecoveryResponseTime(publisher, profile string, responseTime time.Duration) // not-captured in openwrap module, dont provide enough insights RecordPBSAuctionRequestsStats() diff --git a/modules/pubmatic/openwrap/metrics/mock/mock.go b/modules/pubmatic/openwrap/metrics/mock/mock.go index b98fde7c783..d70dc6a196c 100644 --- a/modules/pubmatic/openwrap/metrics/mock/mock.go +++ b/modules/pubmatic/openwrap/metrics/mock/mock.go @@ -815,6 +815,30 @@ func (mr *MockMetricsEngineMockRecorder) RecordVideoInstlImpsStats(arg0, arg1 in return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordVideoInstlImpsStats", reflect.TypeOf((*MockMetricsEngine)(nil).RecordVideoInstlImpsStats), arg0, arg1) } +// RecordBidRecoveryStatus mocks base method. +func (m *MockMetricsEngine) RecordBidRecoveryStatus(arg0 string, arg1 string, arg2 bool) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RecordBidRecoveryStatus", arg0, arg1, arg2) +} + +// RecordBidRecoveryStatus indicates an expected call of RecordBidRecoveryStatus. +func (mr *MockMetricsEngineMockRecorder) RecordBidRecoveryStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordBidRecoveryStatus", reflect.TypeOf((*MockMetricsEngine)(nil).RecordBidRecoveryStatus), arg0, arg1, arg2) +} + +// RecordBidRecoveryResponseTime mocks base method. +func (m *MockMetricsEngine) RecordBidRecoveryResponseTime(arg0 string, arg1 string, arg2 time.Duration) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RecordBidRecoveryResponseTime", arg0, arg1, arg2) +} + +// RecordBidRecoveryResponseTime indicates an expected call of RecordBidRecoveryStatus. +func (mr *MockMetricsEngineMockRecorder) RecordBidRecoveryResponseTime(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordBidRecoveryResponseTime", reflect.TypeOf((*MockMetricsEngine)(nil).RecordBidRecoveryResponseTime), arg0, arg1, arg2) +} + // Shutdown mocks base method. func (m *MockMetricsEngine) Shutdown() { m.ctrl.T.Helper() diff --git a/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go b/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go index 2117caabaff..055cdf0a4ee 100644 --- a/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go +++ b/modules/pubmatic/openwrap/metrics/prometheus/prometheus.go @@ -34,6 +34,8 @@ type Metrics struct { pubNoBidResponseErrors *prometheus.CounterVec pubResponseTime *prometheus.HistogramVec pubImpsWithContent *prometheus.CounterVec + pubBidRecoveryStatus *prometheus.CounterVec + pubBidRecoveryTime *prometheus.HistogramVec // publisher-partner-platform level metrics pubPartnerPlatformRequests *prometheus.CounterVec @@ -360,6 +362,19 @@ func newMetrics(cfg *config.PrometheusMetrics, promRegistry *prometheus.Registry []string{pubIdLabel, profileIDLabel}, ) + metrics.pubBidRecoveryTime = newHistogramVec(cfg, promRegistry, + "bid_recovery_response_time", + "Total time taken by request for secondary auction in ms at publisher profile level.", + []string{pubIDLabel, profileIDLabel}, + []float64{100, 200, 300, 400}, + ) + + metrics.pubBidRecoveryStatus = newCounter(cfg, promRegistry, + "bid_recovery_response_status", + "Count bid recovery status for secondary auction", + []string{pubIDLabel, profileIDLabel, successLabel}, + ) + newSSHBMetrics(&metrics, cfg, promRegistry) return &metrics @@ -680,3 +695,18 @@ func (m *Metrics) RecordPrebidCacheRequestTime(success bool, length time.Duratio successLabel: strconv.FormatBool(success), }).Observe(float64(length.Milliseconds())) } + +func (m *Metrics) RecordBidRecoveryStatus(publisherID, profileID string, success bool) { + m.pubBidRecoveryStatus.With(prometheus.Labels{ + pubIDLabel: publisherID, + profileIDLabel: profileID, + successLabel: strconv.FormatBool(success), + }).Inc() +} + +func (m *Metrics) RecordBidRecoveryResponseTime(publisherID, profileID string, responseTime time.Duration) { + m.pubBidRecoveryTime.With(prometheus.Labels{ + pubIDLabel: publisherID, + profileIDLabel: profileID, + }).Observe(float64(responseTime.Milliseconds())) +} diff --git a/modules/pubmatic/openwrap/metrics/prometheus/prometheus_test.go b/modules/pubmatic/openwrap/metrics/prometheus/prometheus_test.go index faff7bb23dc..9872b7356a9 100644 --- a/modules/pubmatic/openwrap/metrics/prometheus/prometheus_test.go +++ b/modules/pubmatic/openwrap/metrics/prometheus/prometheus_test.go @@ -374,6 +374,30 @@ func TestRecordAdruleValidationFailure(t *testing.T) { }) } +func TestRecordBidRecoveryStatus(t *testing.T) { + m := createMetricsForTesting() + + m.RecordBidRecoveryStatus("5890", "123", true) + + expectedCount := float64(1) + assertCounterVecValue(t, "", "bid_recovery_response_status", m.pubBidRecoveryStatus, + expectedCount, prometheus.Labels{ + pubIDLabel: "5890", + profileIDLabel: "123", + successLabel: "true", + }) +} + +func TestRecordBidRecoveryResponseTime(t *testing.T) { + m := createMetricsForTesting() + + m.RecordBidRecoveryResponseTime("5890", "12345", time.Duration(70)*time.Millisecond) + m.RecordBidRecoveryResponseTime("5890", "12345", time.Duration(130)*time.Millisecond) + resultingHistogram := getHistogramFromHistogramVecByTwoKeys(m.pubBidRecoveryTime, + pubIDLabel, "5890", profileIDLabel, "12345") + assertHistogram(t, "bid_recovery_response_time", resultingHistogram, 2, 200) +} + func getHistogramFromHistogramVec(histogram *prometheus.HistogramVec, labelKey, labelValue string) dto.Histogram { var result dto.Histogram processMetrics(histogram, func(m dto.Metric) { diff --git a/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go b/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go index 94ba5931eaa..82599a54489 100644 --- a/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go +++ b/modules/pubmatic/openwrap/metrics/stats/tcp_stats.go @@ -350,5 +350,8 @@ func (st *StatsTCP) RecordAdruleEnabled(pubId, profId string) func (st *StatsTCP) RecordAdruleValidationFailure(pubId, profId string) {} func (st *StatsTCP) RecordSignalDataStatus(pubid, profileid, signalType string) {} func (st *StatsTCP) RecordPrebidCacheRequestTime(success bool, length time.Duration) {} +func (st *StatsTCP) RecordBidRecoveryStatus(pubID string, profile string, success bool) {} +func (st *StatsTCP) RecordBidRecoveryResponseTime(pubID string, profile string, responseTime time.Duration) { +} func (st *StatsTCP) RecordPrebidAuctionBidResponse(publisher string, partnerName string, bidderCode string, adapterCode string) { }