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

Pubmatic adapter only uses adSlot param for banners #1341

Closed
jbartek25 opened this issue Jun 28, 2021 · 5 comments · Fixed by #1371
Closed

Pubmatic adapter only uses adSlot param for banners #1341

jbartek25 opened this issue Jun 28, 2021 · 5 comments · Fixed by #1371

Comments

@jbartek25
Copy link
Contributor

I'm using PBS for video HB and Pubmatic adapter (based on the info in pubmatic.yaml) supports video. I couldn't find my video requests/imps in Pubmatic reporting and on closer look at the bid requests sent to Pubmatic, the adapter code here only ever passes the adapter adSlot param for the banner requests (in imp.tagid field) but not for any other media type. I believe this is a bug as after modifying the adapter to set imp.tagid for video I see the bids in Pubmatic reports + Pubmatic started bidding much more frequently than before.

@rpanchyk
Copy link
Contributor

rpanchyk commented Jul 9, 2021

Hi @jbartek25
We've ported the code from Go - https://github.com/prebid/prebid-server/blob/e7f7b558951684f1fb75bc4c180506cdc91b5f98/adapters/pubmatic/pubmatic.go#L160
Actually, they do the same.

Technically, we can fix it but sure if it ok from requirements. @bretg can you help?

@jbartek25
Copy link
Contributor Author

jbartek25 commented Jul 9, 2021

@rpanchyk any logic setting tagid in the Java adapter runs only for banners whereas the logic in Go here seemingly runs for all ad types. I see tagid populated in PBS Go video requests but not PBS Java so bits of logic got missed in the Java port.

@rpanchyk
Copy link
Contributor

ahhh... thank you guys, was looking into old Call(..) instead of MakeRequests(..) method.
Btw, didn't expect imp modification in validateAdSlot(..) method.
Anyway, we'll fix the port.

@jbartek25
Copy link
Contributor Author

@rpanchyk thank you

@SerhiiNahornyi SerhiiNahornyi linked a pull request Jul 19, 2021 that will close this issue
@SerhiiNahornyi
Copy link
Collaborator

SerhiiNahornyi commented Jul 19, 2021

@jbartek25 Code of Pubmatic adapter updated in linked PR, so now everything worked as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants