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

fix GPT Pre-Auction PBS path #5650

Merged

Conversation

idettman
Copy link
Contributor

Type of change

  • Bugfix

Description of change

The implementation of #5216 has a problem:

The spec said:

pass fpd.context.adserver.name in the OpenRTB as imp[].ext.context.data.adserver.name
pass fpd.context.adserver.adSlot in the OpenRTB as imp[].ext.context.data.adserver.adslot
Both pbsBidAdapter and the rubiconBidAdapter were implemented incorrectly:

      const gamAdUnit = utils.deepAccess(adUnit, 'fpd.context.adServer.adSlot');
      if (typeof gamAdUnit === 'string' && gamAdUnit) {
        utils.deepSetValue(imp, 'ext.context.data.adslot', gamAdUnit);  // missing the 'adserver' level and not passing adserver name at all.
      }

nakamoto and others added 30 commits February 16, 2019 21:30
# Conflicts:
#	modules/advangelistsBidAdapter.js
#	test/spec/modules/advangelistsBidAdapter_spec.js
@idettman idettman added needs 2nd review Core module updates require two approvals from the core team needs review labels Aug 21, 2020
@patmmccann
Copy link
Collaborator

fyi @gpolaert

@gpolaert
Copy link
Contributor

Thanks @patmmccann @idettman

FYI, last week, I've had an issue with the bid.imp[].ext.context field with the latest version of the prebid server (golang edition).
The error message was: context is not a bidder, did you add the corresponding an alias? (or something like that)

In the source code, it seems the key of extra data/meta should be "below" bid.imp[].ext.prebid, not directly context.

Where I can find the definition of the request send to the Prebid Server? Is there documentation somewhere?

Copy link
Collaborator

@robertrmartinez robertrmartinez left a comment

Choose a reason for hiding this comment

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

Looks good to me

@robertrmartinez
Copy link
Collaborator

Thanks @patmmccann @idettman

FYI, last week, I've had an issue with the bid.imp[].ext.context field with the latest version of the prebid server (golang edition).
The error message was: context is not a bidder, did you add the corresponding an alias? (or something like that)

In the source code, it seems the key of extra data/meta should be "below" bid.imp[].ext.prebid, not directly context.

Where I can find the definition of the request send to the Prebid Server? Is there documentation somewhere?

@bretg @idettman

This seems to be correct, right?

imp.ext are usually where bidders go, besides the reserved word "prebid".

Maybe this does need to go under imp.ext.prebid.???

@robertrmartinez
Copy link
Collaborator

Actually, I just tested this using Rubicon's Prebid Server (Java) and it does not throw the error.

Seems context WAS added to the PBS code as a reserved word.

And I can see the Rubicon PBS adapter picking up these attributes and passing them to the exchange.

@gpolaert It is possible the required change for context in the ext object are not yet in the PBS you are testing.

Please provide more context (no pun intended)

@gpolaert
Copy link
Contributor

gpolaert commented Aug 27, 2020

@robertrmartinez
The prebid server (Go) checks if the bidder names are valid.

Check here: https://github.com/prebid/prebid-server/blob/master/endpoints/openrtb2/auction.go#L759-L777

Note: openrtb_ext.PrebidExtKey = "prebid"

As context is not excluded from the condition, the server returns an error.

@robertrmartinez
Copy link
Collaborator

@bretg

Is the change to not validate the context object going to be ported to PBS GO?

@bretg
Copy link
Collaborator

bretg commented Sep 2, 2020

Is the change to not validate the context object going to be ported to PBS GO?

I've put it on the list to discuss at Friday's PBS meeting.

@robertrmartinez
Copy link
Collaborator

@gpolaert

Looks like a PR has been submitted to update the PBS GO to handle this. (thanks @bretg for pointing it out)

prebid/prebid-server#1479

I am merging this PR now.

@robertrmartinez robertrmartinez merged commit 1676c76 into prebid:master Sep 8, 2020
BrightMountainMediaInc pushed a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020
* Add microadBidAdapter

* Remove unnecessary encodeURIComponent from microadBidAdapter

* Submit Advangelists Prebid Adapter

* Submit Advangelists Prebid Adapter 1.1

* Correct procudtion endpoint for prebid

* analytics update with wrapper name

* reverted error merge

* update changed default value of netRevenue to true

* Re-add rubicon analytics without deprecated getTopWindowUrl util

* Cache referrer on auction_init instead of bid_requested

* merged remote master changes

* updated unit tests for object path changes

* updated rp bid adapter unit tests for GAM object path changes

* updated naming and changed iterator to use arrow syntax

* continue renaming and cleanup

Co-authored-by: nakamoto <nakamoto_tatsuya@microad.co.jp>
Co-authored-by: Chandra Prakash <chandra.prakash@advangelists.com>
Co-authored-by: Eric Harper <eharper@rubiconproject.com>
Co-authored-by: TJ Eastmond <teastmond@la-wlandaverde-mac.local>
Co-authored-by: Mark Monday <mmonday@rubiconproject.com>
Co-authored-by: msm0504 <51493331+msm0504@users.noreply.github.com>
BrightMountainMediaInc pushed a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020
@robertrmartinez robertrmartinez deleted the fix-gpt-preauction-pbs-path branch July 5, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants