Skip to content

Commit

Permalink
Merge pull request #907 from PubMatic-OpenWrap/ci
Browse files Browse the repository at this point in the history
RE-3735: Release 17th Sept 2024
  • Loading branch information
PubMatic-OpenWrap authored Sep 16, 2024
2 parents 0affd2c + 5d3b3de commit aee0cb6
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 8 deletions.
9 changes: 4 additions & 5 deletions adapters/vastbidder/etree_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package vastbidder

import (
"strconv"
"strings"

"github.com/beevik/etree"
"github.com/prebid/prebid-server/v2/openrtb_ext"
Expand Down Expand Up @@ -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, ""
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions adapters/vastbidder/etree_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,12 @@ func getPricingDetailsTestCases() []struct {
wantPrice: 0,
wantCurrency: "",
},
{
name: "bug",
vastXML: "<VAST\n\txmlns:xs=\"https://www.w3.org/2001/XMLSchema\"\n\txmlns=\"https://www.iab.com/VAST\" version=\"4.1\">\n\t<Ad id=\"scenario_9_adid\" sequence=\"1\">\n\t\t<InLine>\n\t\t\t<AdSystem version=\"4.1\">PubMatic</AdSystem>\n\t\t\t<AdServingId>scenario_9</AdServingId>\n\t\t\t<AdTitle>Test Ad scenario_9</AdTitle>\n\t\t\t<Pricing model=\"cpm\" currency=\"USD\">\n\t\t\t\t<![CDATA[ 5.00 ]]>\n\t\t\t</Pricing>\n\t\t\t<Impression>\n\t\t\t\t<![CDATA[ https://example.com/impression/ ]]>\n\t\t\t</Impression>\n\t\t\t<Advertiser>Test Advertiser</Advertiser>\n\t\t\t<Category authority=\"https://www.iabtechlab.com/categoryauthority\">IAB1-1\t</Category>\n\t\t\t<Creatives>\n\t\t\t\t<Creative id=\"5481\" sequence=\"1\" adId=\"2447226_scenario_9\">\n\t\t\t\t\t<UniversalAdId idRegistry=\"Ad-ID\">8465_scenario_9</UniversalAdId>\n\t\t\t\t\t<Linear>\n\t\t\t\t\t\t<Duration>00:00:10</Duration>\n\t\t\t\t\t\t<MediaFiles>\n\t\t\t\t\t\t\t<MediaFile delivery=\"progressive\" type=\"video/mp4\" bitrate=\"500\" width=\"400\" height=\"300\" scalable=\"true\" maintainAspectRatio=\"true\">\n\t\t\t\t\t\t\t\t<![CDATA[ https://ads.pubmatic.com/AdServer/js/ott/sampleads/10_seconds_ad.mp4 ]]>\n\t\t\t\t\t\t\t</MediaFile>\n\t\t\t\t\t\t</MediaFiles>\n\t\t\t\t\t</Linear>\n\t\t\t\t</Creative>\n\t\t\t</Creatives>\n\t\t</InLine>\n\t</Ad>\n</VAST> ",
wantPrice: 5,
wantCurrency: "USD",
},
// TODO: Add test cases.
}
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
14 changes: 14 additions & 0 deletions modules/pubmatic/openwrap/metrics/config/multimetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
4 changes: 4 additions & 0 deletions modules/pubmatic/openwrap/metrics/config/multimetrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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))
}
2 changes: 2 additions & 0 deletions modules/pubmatic/openwrap/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
24 changes: 24 additions & 0 deletions modules/pubmatic/openwrap/metrics/mock/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions modules/pubmatic/openwrap/metrics/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()))
}
24 changes: 24 additions & 0 deletions modules/pubmatic/openwrap/metrics/prometheus/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions modules/pubmatic/openwrap/metrics/stats/tcp_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}

0 comments on commit aee0cb6

Please sign in to comment.