OTT-130: Injecting Video Tracking Events in VAST #130

ShriprasadM · 2021-03-04T09:32:14Z

No description provided.

…t trackers in VAST

ShriprasadM · 2021-03-04T11:59:28Z

endpoints/events/vtrack.go

+			}
+		}
+		// replace standard macros
+		eventURL = replaceMacro(eventURL, VASTAdTypeMacro, "")


add the value for ad type

Done. Passing video type always

ShriprasadM · 2021-03-04T12:00:06Z

endpoints/events/vtrack.go

+	if "" == strings.Trim(trackerURL, " ") {
+		return eventURLMap
+	}
+	for _, event := range []string{"firstQuartile", "midpoint", "thirdQuartile", "complete"} {


Move it to global private variable

Done. introduced global private variable - trackingEvents

ShriprasadM · 2021-03-04T12:08:56Z

endpoints/events/vtrack_test.go

+			},
+			want: want{
+				eventURLs: map[string][]string{
+					"firstQuartile": []string{"http://something.com", "http://company.tracker.com?eventId=1004&appbundle=abc"},


Add missing expectations for midpoint, thirdQuartile and complete

Done. added required assertions

ShriprasadM · 2021-03-04T12:12:20Z

endpoints/events/vtrack_test.go

+					"complete":      {"http://company.tracker.com?eventId=6&appbundle=abc"},
+				},
+			},
+		},


Add testcase when adm="" but we have nurl

ShriprasadM · 2021-03-04T12:25:04Z

exchange/events.go

+		fmt.Println(string(newVastXML))
+		fmt.Println(bid.AdM)


Remove println

pm-viral-vala

Once changes are done, Please go ahead with QADrop.

pm-viral-vala · 2021-03-18T04:24:12Z

endpoints/events/vtrack.go

+	creatives = append(creatives, doc.FindElements("VAST/Ad/InLine/Creatives/Creative/NonLinearAds")...)
+	creatives = append(creatives, doc.FindElements("VAST/Ad/Wrapper/Creatives/Creative/NonLinearAds")...)
+
+	if "" == strings.Trim(bid.AdM, " ") || strings.HasPrefix(strings.TrimLeft(strings.TrimRight(bid.AdM, "]]>"), "<![CDATA["), "http") {


use strings.TrimSpace() instead of strings.Trim()

bid.AdM never comes inside CDATA Tag if it is returning VAST Tag URL. check trimmed bid.AdM starts with http

pm-viral-vala · 2021-03-18T04:33:50Z

endpoints/events/vtrack.go

+	if nil != err {
+		err := fmt.Sprintf("Error parsing VAST XML. '%v'", err.Error())
+		glog.Errorf(err)
+		return []byte(vastXML), false, errors.New(err) // false indicates events trackers are not injected


return err itself

pm-viral-vala · 2021-03-18T04:35:14Z

endpoints/events/vtrack.go

+	doc := etree.NewDocument()
+	err := doc.ReadFromString(vastXML)
+	if nil != err {
+		err := fmt.Sprintf("Error parsing VAST XML. '%v'", err.Error())


fmt.Errorf use instead Sprintf

pm-viral-vala · 2021-03-18T04:35:55Z

endpoints/events/vtrack.go

@@ -302,3 +329,147 @@ func ModifyVastXmlJSON(externalUrl string, data json.RawMessage, bidid, bidder,
 	}
 	return json.RawMessage(vast)
 }
+
+//InjectVideoEventTrackers injects the video tracking events


write parameter single line description

pm-viral-vala · 2021-03-18T04:50:20Z

endpoints/events/vtrack.go

+		// replace standard macros
+		eventURL = replaceMacro(eventURL, VASTAdTypeMacro, string(openrtb_ext.BidTypeVideo))
+		if nil != req && nil != req.App {
+			eventURL = replaceMacro(eventURL, VASTAppBundleMacro, req.App.Bundle)


Check Macro here, Domain or Bundle

[DOMAIN] will be req.App.Bundle in case of Apps

pm-viral-vala · 2021-03-18T04:54:13Z

endpoints/events/vtrack.go

+}
+
+func replaceMacro(trackerURL, macro, value string) string {
+	if strings.HasPrefix(macro, "[") && strings.HasSuffix(macro, "]") && len(strings.Trim(value, " ")) > 0 {


will it replace " [MACRO] "? Check with trimmed value.

Added TrimSpace

pm-viral-vala · 2021-03-18T04:55:27Z

exchange/auction.go

+	if adm == "" {
+		return fmt.Sprintf(wrapperVASTTemplate, bid.NURL) // set nurl as VASTAdTagURI
+	}
+	if strings.HasPrefix(adm, "<![CDATA[") {


adm won't have CDATA

Removed the block

pm-viral-vala

👍

* OTT-130: Injecting Video Tracking Events in VAST (#130) * OTT-105_VCR: basic changes * OTT-105: Changes for injecting Tracking Events * OTT-130_VCR: Added framework with unit tests for injecting Video Event trackers in VAST * OTT-130_VCR: Reverted Path replace * OTT-130: Replaced etree dep with original repository * OTT-130: Refactored as per latest master * OTT-130: Refactored * OTT-130: Refactored * OTT-130: Reverted unwanted changes * OTT-130: Added unit tests and refactoring * OTT-130: InjectVideoEventTrackers will now return error if any * OTT-130: Refactored * OTT-130: Removed custom macro support * OTT-130: Reverted the unwanted changes * OTT-130: Reverted unwanted changes * OTT-130: Added Url Query Escape Functionality * OTT-130: Fixed test failures * OTT-130: Refactored * OTT-130: Addressed code review comments from Viral Co-authored-by: Shriprasad <[email protected]> * OTT-131: VCR Custom Macro (#132) * OTT-131: Added support for consuming the custom macros from host company * OTT-131: Removed unwanted comments * OTT-131: Addressed code review comments Co-authored-by: Shriprasad <[email protected]> * OTT-174: Added bid ID , Ad Unit as tracking parameters and ADomain handling change (#134) * OTT-174: Added AdUnit parameter inside video event tracker URL * OTT-174: Added bid id, au tracker parameters. in case of advetiser_id, OW will now send only value at index 0 from bid.ADomain array by extracting the domain Also handled the CTV/Ad Pod bid.id custom use case * OTT-174: Ensured bidid value is replaced only in case of OW trackers. (95% reliable) Also ensured that bidid is not getting added to other trackers, if not present * OTT-174: Fixed code review comments Co-authored-by: Shriprasad <[email protected]> Co-authored-by: Shriprasad <[email protected]>

* OTT-130: Injecting Video Tracking Events in VAST (#130) * OTT-105_VCR: basic changes * OTT-105: Changes for injecting Tracking Events * OTT-130_VCR: Added framework with unit tests for injecting Video Event trackers in VAST * OTT-130_VCR: Reverted Path replace * OTT-130: Replaced etree dep with original repository * OTT-130: Refactored as per latest master * OTT-130: Refactored * OTT-130: Refactored * OTT-130: Reverted unwanted changes * OTT-130: Added unit tests and refactoring * OTT-130: InjectVideoEventTrackers will now return error if any * OTT-130: Refactored * OTT-130: Removed custom macro support * OTT-130: Reverted the unwanted changes * OTT-130: Reverted unwanted changes * OTT-130: Added Url Query Escape Functionality * OTT-130: Fixed test failures * OTT-130: Refactored * OTT-130: Addressed code review comments from Viral Co-authored-by: Shriprasad <[email protected]> * OTT-131: VCR Custom Macro (#132) * OTT-131: Added support for consuming the custom macros from host company * OTT-131: Removed unwanted comments * OTT-131: Addressed code review comments Co-authored-by: Shriprasad <[email protected]> * OTT-174: Added bid ID , Ad Unit as tracking parameters and ADomain handling change (#134) * OTT-174: Added AdUnit parameter inside video event tracker URL * OTT-174: Added bid id, au tracker parameters. in case of advetiser_id, OW will now send only value at index 0 from bid.ADomain array by extracting the domain Also handled the CTV/Ad Pod bid.id custom use case * OTT-174: Ensured bidid value is replaced only in case of OW trackers. (95% reliable) Also ensured that bidid is not getting added to other trackers, if not present * OTT-174: Fixed code review comments Co-authored-by: Shriprasad <[email protected]> * OTT-105: Changes for advertiser id Co-authored-by: Shriprasad <[email protected]>

pm-shriprasad-marathe added 10 commits February 26, 2021 13:18

OTT-105_VCR: basic changes

b6d4c5c

OTT-105: Changes for injecting Tracking Events

574d43a

OTT-130_VCR: Added framework with unit tests for injecting Video Even…

ffa25db

…t trackers in VAST

OTT-130_VCR: Reverted Path replace

b7bfc53

OTT-130: Replaced etree dep with original repository

d5cdfae

Merge branch 'master' into OTT-130_VCR

de1df06

OTT-130: Refactored as per latest master

edf6712

Merge branch 'master' into OTT-130_VCR

e352622

OTT-130: Refactored

d932936

OTT-130: Refactored

6f26efc

ShriprasadM marked this pull request as ready for review March 4, 2021 09:32

OTT-130: Reverted unwanted changes

1ab93c1

ShriprasadM commented Mar 4, 2021

View reviewed changes

pm-shriprasad-marathe added 10 commits March 15, 2021 12:48

OTT-130: Added unit tests and refactoring

fcde3f4

OTT-130: InjectVideoEventTrackers will now return error if any

6e4084b

OTT-130: Refactored

1debe67

OTT-130: Removed custom macro support

da3a63a

OTT-130: Reverted the unwanted changes

726b779

OTT-130: Reverted unwanted changes

b9b8efa

Merge branch 'OTT-105_VCR' into OTT-130_VCR

992afa2

OTT-130: Added Url Query Escape Functionality

4abfaac

OTT-130: Fixed test failures

d1585a4

OTT-130: Refactored

5d968e3

pm-viral-vala requested changes Mar 18, 2021

View reviewed changes

OTT-130: Addressed code review comments from Viral

dab2ff2

pm-viral-vala approved these changes Mar 18, 2021

View reviewed changes

ShriprasadM merged commit 2b3b74a into OTT-105_VCR Mar 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OTT-130: Injecting Video Tracking Events in VAST #130

OTT-130: Injecting Video Tracking Events in VAST #130

ShriprasadM commented Mar 4, 2021

ShriprasadM Mar 4, 2021

ShriprasadM Mar 5, 2021 •

edited

Loading

ShriprasadM Mar 4, 2021

ShriprasadM Mar 5, 2021 •

edited

Loading

ShriprasadM Mar 4, 2021

ShriprasadM Mar 5, 2021

ShriprasadM Mar 4, 2021

ShriprasadM Mar 17, 2021

ShriprasadM Mar 4, 2021

ShriprasadM Mar 17, 2021

pm-viral-vala left a comment

pm-viral-vala Mar 18, 2021

ShriprasadM Mar 18, 2021

pm-viral-vala Mar 18, 2021

ShriprasadM Mar 18, 2021

pm-viral-vala Mar 18, 2021

ShriprasadM Mar 18, 2021

pm-viral-vala Mar 18, 2021 •

edited

Loading

ShriprasadM Mar 18, 2021

pm-viral-vala Mar 18, 2021

ShriprasadM Mar 18, 2021

pm-viral-vala Mar 18, 2021

ShriprasadM Mar 18, 2021

pm-viral-vala Mar 18, 2021

ShriprasadM Mar 18, 2021

pm-viral-vala left a comment

OTT-130: Injecting Video Tracking Events in VAST #130

OTT-130: Injecting Video Tracking Events in VAST #130

Conversation

ShriprasadM commented Mar 4, 2021

Choose a reason for hiding this comment

ShriprasadM Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShriprasadM Mar 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pm-viral-vala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pm-viral-vala Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pm-viral-vala left a comment

Choose a reason for hiding this comment

ShriprasadM Mar 5, 2021 •

edited

Loading

ShriprasadM Mar 5, 2021 •

edited

Loading

pm-viral-vala Mar 18, 2021 •

edited

Loading