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.

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

RE-3735: Release 17th Sept 2024 #907

Merged
merged 3 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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) {
}
Loading