prebid-server icon indicating copy to clipboard operation
prebid-server copied to clipboard
verified publisher icon prebid

New Adapter: Nativo

Open rafataveira opened this issue 1 year ago • 19 comments

rafataveira avatar Jul 03 '24 06:07 rafataveira

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 8c798c9096649c726986780acd8f3a98aa67fc69

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:20:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:27:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:48:	MakeBids		92.9%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:89:	getMediaTypeForImp	100.0%
total:									(statements)		93.8%

github-actions[bot] avatar Jul 03 '24 06:07 github-actions[bot]

@onkarvhanumante Is there any ETA for reviewing this PR? When is the next version expected to be released? TIA

rafataveira avatar Jul 18 '24 16:07 rafataveira

@rafataveira PR tests are failing due to file miss format. Requesting to format files

Run ./validate.sh --nofmt --cov --race 10
gofmt needs to be run, 2 files have issues.  Below is a list of files to review:
adapters/nativo/nativo.go
adapters/nativo/nativo_test.go

onkarvhanumante avatar Aug 01 '24 17:08 onkarvhanumante

@rafataveira Does this bidder supports request.test parameter and respond with dummy response ? or is there any way this bidder can return dummy response for test purpose ?

ashishshinde-pubm avatar Aug 08 '24 03:08 ashishshinde-pubm

@onkarvhanumante I will review all comments and make the changes accordingly.

rafataveira avatar Aug 15 '24 20:08 rafataveira

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 46e7869906344b2961b6b29f1239f0a335b28c51

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:20:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:27:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:48:	MakeBids		92.9%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:89:	getMediaTypeForImp	100.0%
total:									(statements)		93.8%

github-actions[bot] avatar Aug 15 '24 22:08 github-actions[bot]

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 3e8a7dbb392b8115844d8c2522084f42c29b13e5

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:20:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:27:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:48:	MakeBids		92.9%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:89:	getMediaTypeForImp	100.0%
total:									(statements)		93.8%

github-actions[bot] avatar Aug 27 '24 18:08 github-actions[bot]

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, e568b98d40cacf493c96f0311377544e5b5dbf16

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:46:	MakeBids		83.3%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:83:	getMediaTypeForImp	88.9%
total:									(statements)		86.1%

github-actions[bot] avatar Aug 28 '24 17:08 github-actions[bot]

Hi @rafataveira, your tests are failing because you need to run go fmt.

bsardo avatar Sep 09 '24 15:09 bsardo

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, f98e66418725f01df493945ca9e79ddc52b30753

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:46:	MakeBids		83.3%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:83:	getMediaTypeForImp	88.9%
total:									(statements)		86.1%

github-actions[bot] avatar Sep 09 '24 18:09 github-actions[bot]

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 5d3c8bb6c70f5013f0c073d029fdba1ea70b7531

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:46:	MakeBids		83.3%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:83:	getMediaTypeForImp	88.9%
total:									(statements)		86.1%

github-actions[bot] avatar Sep 13 '24 16:09 github-actions[bot]

@rafataveira , redirect url is not working

@przemkaczmarek could you check if redirect url is working for you

http://jadserve.postrelease.com/suid/101787?gdpr=&gdpr_consent=&us_privacy=&ntv_gpp_consent=&ntv_r=https%3A%2F%2Fprebid.adnxs.com%2Fpbs%2Fv1%2Fsetuid%3Fbidder%3DNativo%26gdpr%3D%26gdpr_consent%3D%26f%3Di%26uid%3D%22NTV_USER_ID%22

onkarvhanumante avatar Sep 24 '24 09:09 onkarvhanumante

@onkarvhanumante you need to use the secure url https:// provided in the nativo.yaml instead of http://

https://jadserve.postrelease.com/suid/101787?gdpr=&gdpr_consent=&us_privacy=&ntv_gpp_consent=&ntv_r=https%3A%2F%2Fprebid.adnxs.com%2Fpbs%2Fv1%2Fsetuid%3Fbidder%3DNativo%26gdpr%3D%26gdpr_consent%3D%26f%3Di%26uid%3D%22NTV_USER_ID%22

rafataveira avatar Sep 24 '24 14:09 rafataveira

works: image

przemkaczmarek avatar Sep 25 '24 12:09 przemkaczmarek

works: image

I see this 200 response as well but it is supposed to redirect (302) to the PBS setuid endpoint which is specified in your ntv_r parameter. @rafataveira

bsardo avatar Sep 25 '24 15:09 bsardo

@bsardo This is the result of the redirect I get: https://prebid.adnxs.com/pbs/v1/setuid?bidder=Nativo&gdpr=&gdpr_consent=&f=i&uid=%22c0ed3c17-4ba0-4d2e-9cf2-cbf8f9037793%22 image

rafataveira avatar Sep 26 '24 16:09 rafataveira

@bsardo This is the result of the redirect I get: https://prebid.adnxs.com/pbs/v1/setuid?bidder=Nativo&gdpr=&gdpr_consent=&f=i&uid=%22c0ed3c17-4ba0-4d2e-9cf2-cbf8f9037793%22 image

Hi @rafataveira, I suggest testing your user sync with localhost so that your bidder is known and so that you can ensure your server redirects to the setuid endpoint of the PBS host, which in this case will be the localhost setuid endpoint. Start this process by first hitting the localhost PBS cookie sync endpoint: POST localhost:8000/cookie_sync with body {"bidders": ["nativo"]} and that will give you the url that would be dropped as a pixel kicking off the cookie sync that should ultimately redirect to the setuid endpoint.

bsardo avatar Sep 30 '24 14:09 bsardo

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, fec946329d2bafb6fba74d5cecf37ec723fb9d22

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:46:	MakeBids		83.3%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:83:	getMediaTypeForImp	88.9%
total:									(statements)		86.1%

github-actions[bot] avatar Oct 01 '24 15:10 github-actions[bot]

@bsardo This is the result of the redirect I get: https://prebid.adnxs.com/pbs/v1/setuid?bidder=Nativo&gdpr=&gdpr_consent=&f=i&uid=%22c0ed3c17-4ba0-4d2e-9cf2-cbf8f9037793%22 image

Hi @rafataveira, I suggest testing your user sync with localhost so that your bidder is known and so that you can ensure your server redirects to the setuid endpoint of the PBS host, which in this case will be the localhost setuid endpoint. Start this process by first hitting the localhost PBS cookie sync endpoint: POST localhost:8000/cookie_sync with body {"bidders": ["nativo"]} and that will give you the url that would be dropped as a pixel kicking off the cookie sync that should ultimately redirect to the setuid endpoint.

I found the issue with the double quotes on userMacro. Now it should work as expected.

rafataveira avatar Oct 01 '24 15:10 rafataveira

@rafataveira we're still seeing an issue with the user sync. Your server is responding with a 200. Since you've classified this user sync as type redirect, your server should be responding with a 302 redirect to the url defined in the ntv_r parameter with the user id macro populated.

bsardo avatar Oct 03 '24 14:10 bsardo

@bsardo I couldn't reproduce this behaviour. As you can see, our server responds with 302 and correctly redirects to the URL defined in the ntv_r parameter with the user ID macro populated. image This is the same cookie sync URL used by Prebid Server Java version and other partners. Also, make sure to use the secure URL (https).

https://jadserve.postrelease.com/suid/101787?gdpr=&gdpr_consent=&us_privacy=&ntv_gpp_consent=&ntv_r=https%3A%2F%2Fprebid.adnxs.com%2Fpbs%2Fv1%2Fsetuid%3Fbidder%3DNativo%26gdpr%3D%26gdpr_consent%3D%26f%3Di%26uid%3DNTV_USER_ID

rafataveira avatar Oct 07 '24 19:10 rafataveira

@onkarvhanumante @przemkaczmarek @bsardo, is the cookie sync URL the only thing preventing you from approving this PR? We have some clients waiting for the GO version to go live for quite some time. Could you please go ahead and approve it? The cookie sync URL is the same as prebid-server-java and other partners, and it is working as expected.

rafataveira avatar Oct 17 '24 15:10 rafataveira

@onkarvhanumante @przemkaczmarek @bsardo, is the cookie sync URL the only thing preventing you from approving this PR? We have some clients waiting for the GO version to go live for quite some time. Could you please go ahead and approve it? The cookie sync URL is the same as prebid-server-java and other partners, and it is working as expected.

Hi @rafataveira, yes the user sync is the only thing holding this up. We tried the URL you provided as well and it is still not working for me or Peter and we are based in the US and Poland respectively in case this is a geographic region issue. We are still seeing a 200 instead of a 302 redirect. This issue needs to be resolved before merging this PR as is unless you want to break the user sync functionality out into a separate PR so that the rest of this can be merged now. At this point, I am leaving the user sync investigation up to you though I will triple check with another team member that the 200 issue is occurring and report back later today.

bsardo avatar Oct 17 '24 18:10 bsardo

@bsardo I see what is happening to your validation. This is a production cookie sync URL which has different kinds of internal validations to avoid fraud and won't redirect if a possible fraud is detected. It works for me because I am in a VPN environment. This has nothing to do with our GO version adapter at all. If this is still a blocker for you to approve this PR, (even though it is not a problem) we can remove it from this PR and also, change our validation for Prebid Server to always redirect. Let me know what you recommend.

rafataveira avatar Oct 18 '24 18:10 rafataveira

@rafataveira we're required to verify the user sync endpoints works. Can you either provide details on how to construct a request so that it is not flagged by your servers as potential fraud or change your server validations so that the URL generated by the cookie sync endpoint always results in a redirect? I'll leave that decision to you at this time as I'm not aware of how your validation works.

bsardo avatar Oct 22 '24 15:10 bsardo

@bsardo we have pushed a fix for the redirect validation. Can you please validate and if all looks good can we merge the PR?

rafataveira avatar Oct 25 '24 23:10 rafataveira

@bsardo can we get an update on the cookie sync validation ?

aparekh-nativo avatar Oct 29 '24 16:10 aparekh-nativo

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 6d4567800942dfe5c3993b09f0708ecf9f3cf54b

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:46:	MakeBids		83.3%
github.com/prebid/prebid-server/v2/adapters/nativo/nativo.go:83:	getMediaTypeForImp	88.9%
total:									(statements)		86.1%

github-actions[bot] avatar Nov 01 '24 15:11 github-actions[bot]

Hi @aparekh-nativo, @rafataveira tests are failing and your PR branch does not run because you have some incorrect package references after merging with master. All package imports should reference v3 instead of v2. It looks like the offending files are adapter_builders.go, nativo.go and nativo_test.go. Yesterday we released PBS 3.0 which updated all package references to v3. See #4029.

bsardo avatar Nov 01 '24 16:11 bsardo

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 96b26e918349d05b4ce85267f9dd725b96487e99

nativo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:19:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:26:	MakeRequests		85.7%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:46:	MakeBids		83.3%
github.com/prebid/prebid-server/v3/adapters/nativo/nativo.go:83:	getMediaTypeForImp	88.9%
total:									(statements)		86.1%

github-actions[bot] avatar Nov 04 '24 16:11 github-actions[bot]