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

Sharethrough: Add support for GPID #1925

Merged
merged 2 commits into from
Jul 22, 2021
Merged

Sharethrough: Add support for GPID #1925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10db716
Add support for GPID
epechuzal Jul 14, 2021
9ae92da
Update schema to take into account imp[].ext.data.pbadslot
epechuzal Jul 21, 2021
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
10 changes: 10 additions & 0 deletions adapters/sharethrough/butler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const defaultTmax = 10000 // 10 sec
type StrAdSeverParams struct {
Pkey string
BidID string
GPID string
ConsentRequired bool
ConsentString string
USPrivacySignal string
Expand Down Expand Up @@ -97,6 +98,11 @@ func (s StrOpenRTBTranslator) requestFromOpenRTB(imp openrtb2.Imp, request *open
return nil, err
}

var gpid string
if strImpParams.Data != nil && strImpParams.Data.PBAdSlot != "" {
gpid = strImpParams.Data.PBAdSlot
}

usPolicySignal := ""
if usPolicy, err := ccpa.ReadFromRequest(request); err == nil {
usPolicySignal = usPolicy.Consent
Expand All @@ -107,6 +113,7 @@ func (s StrOpenRTBTranslator) requestFromOpenRTB(imp openrtb2.Imp, request *open
Uri: s.UriHelper.buildUri(StrAdSeverParams{
Pkey: pKey,
BidID: imp.ID,
GPID: gpid,
ConsentRequired: s.Util.gdprApplies(request),
ConsentString: userInfo.Consent,
USPrivacySignal: usPolicySignal,
Expand Down Expand Up @@ -191,6 +198,9 @@ func (h StrUriHelper) buildUri(params StrAdSeverParams) string {
v := url.Values{}
v.Set("placement_key", params.Pkey)
v.Set("bidId", params.BidID)
if params.GPID != "" {
v.Set("gpid", params.GPID)
}
v.Set("consent_required", fmt.Sprintf("%t", params.ConsentRequired))
v.Set("consent_string", params.ConsentString)
if params.USPrivacySignal != "" {
Expand Down
4 changes: 3 additions & 1 deletion adapters/sharethrough/butler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestSuccessRequestFromOpenRTB(t *testing.T) {
"Generates the correct AdServer request from Imp (no user provided)": {
inputImp: openrtb2.Imp{
ID: "abc",
Ext: []byte(`{ "bidder": {"pkey": "pkey", "iframe": true, "iframeSize": [10, 20], "bidfloor": 1.0} }`),
Ext: []byte(`{ "bidder": {"pkey": "pkey", "iframe": true, "iframeSize": [10, 20], "bidfloor": 1.0, "data": { "pbadslot": "adslot" } } }`),
Banner: &openrtb2.Banner{
Format: []openrtb2.Format{{H: 30, W: 40}},
},
Expand Down Expand Up @@ -435,6 +435,7 @@ func TestBuildUri(t *testing.T) {
inputParams: StrAdSeverParams{
Pkey: "pkey",
BidID: "bid",
GPID: "gpid",
ConsentRequired: true,
ConsentString: "consent",
USPrivacySignal: "ccpa",
Expand All @@ -449,6 +450,7 @@ func TestBuildUri(t *testing.T) {
"http://abc.com?",
"placement_key=pkey",
"bidId=bid",
"gpid=gpid",
"consent_required=true",
"consent_string=consent",
"us_privacy=ccpa",
Expand Down
15 changes: 10 additions & 5 deletions openrtb_ext/imp_sharethrough.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package openrtb_ext

type ExtImpSharethrough struct {
Pkey string `json:"pkey"`
Iframe bool `json:"iframe"`
IframeSize []int `json:"iframeSize"`
BidFloor float64 `json:"bidfloor"`
type ExtData struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking, do you need ExtData or could you remove it and just have PBAdSlot in ExtImpSharethrough with an omitempty?

Copy link
Contributor Author

@epechuzal epechuzal Jul 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@bsardo I am not sure. I am trying to get the GPID found in imp[].ext.data, similar to #1807.

In pbjs the method to implement was to extract the ortb2Imp.ext.data.pbadslot value. I am looking for the equivalent for prebid-server. As far as I can tell from checking PRs from other adapters they all implement it this way, but I was unable to find any official documentation that says exactly how this should be done.

Can you help? I see that you approved the PR from TheMediaGrid which does the same thing, is there something that our implementation is missing?

Copy link
Collaborator

@bsardo bsardo Jul 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sure I can help. I did some digging and I now believe your implementation is correct assuming the GPID, which I'm guessing stands for global placement ID, is considered ad-specific first party data.

The documentation here indicates that all PBS request ad-specific first party data belongs at imp[].ext.data:
https://docs.prebid.org/prebid-server/endpoints/openrtb2/pbs-endpoint-auction.html#prebid-server-ortb2-extension-summary

Copy link
Collaborator

Choose a reason for hiding this comment

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

@epechuzal, sorry I got confused above, hence the strikethrough. I still believe your implementation is correct but I wasn't clear why. Let me back up.

You're expecting PbAdslot and AdServer at imp[].ext.data in the PBS inbound request and you're putting GPID in the query string of the PBS outbound request to your server.
For inbound request data, if it is ad-specific first party data, it belongs at imp[]ext.data according to the referenced documentation above. It looks like PbAdslot and AdServer are indeed ad-specific and in the right place, similar to how The Media Grid does it in #1807.

As for GPID, you and The Media Grid are both setting it to PbAdSlot but in different places in your bid requests. You're setting it in each query string and they are setting it in each imp.ext which is fine since that is where your servers are expecting to find that data.

I hope that helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsardo thank you for looking into it! let me know if there is anything that I should change for this PR to go in

PBAdSlot string `json:"pbadslot"`
}

// ExtImpSharethrough defines the contract for bidrequest.imp[i].ext.sharethrough
type ExtImpSharethrough struct {
Pkey string `json:"pkey"`
Iframe bool `json:"iframe"`
IframeSize []int `json:"iframeSize"`
BidFloor float64 `json:"bidfloor"`
Data *ExtData `json:"data,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

You defined a new bidder parameter. Can you please update static/bidder-params/sharethrough.json file as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@VeronikaSolovei9 do we have to? None of the other adapters which implement some sort of ExtData on their bidder parameters have declared it there. What is the correct way to do this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't have to but it is definitely preferred/recommended. You'll get parameter validation by PBS core upstream so you can be sure you have valid data in your ext when your adapter code executes.

Bidder param documentation can be found here: https://docs.prebid.org/prebid-server/developers/add-new-bidder-go.html#bidder-parameters

Copy link
Collaborator

Choose a reason for hiding this comment

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

@epechuzal I discussed this with the team and this is actually required because other systems and front-ends may use the defined schema.

}

type ExtImpSharethroughResponse struct {
AdServerRequestID string `json:"adserverRequestId"`
BidID string `json:"bidId"`
Expand Down
10 changes: 10 additions & 0 deletions static/bidder-params/sharethrough.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@
"bidfloor": {
"type": "number",
"description": "The floor price, or minimum amount, a publisher will accept for an impression, given in CPM in USD"
},
"data": {
"type": "object",
"description": "Ad-specific first party data",
"properties": {
"pbadslot": {
"type": "string",
"description": "Prebid Ad Slot, see: https://docs.prebid.org/features/pbAdSlot.html"
}
}
}
},
"required": ["pkey"]
Expand Down