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

Sharethrough: Support native ads #2780

Conversation

maxime-dupuis
Copy link
Contributor

No description provided.

@SyntaxNode SyntaxNode changed the title [IG-178207154] Sharethrough adapter - Support native ads Sharethrough: Support native ads May 16, 2023
balbinas
balbinas previously approved these changes May 16, 2023
@Sonali-More-Xandr
Copy link
Contributor

Please update the dev-docs to reflect added support for native.

@maxime-dupuis
Copy link
Contributor Author

Please update the dev-docs to reflect added support for native.

Thanks @Sonali-More-Xandr, here's the doc PR

@jefftmahoney
Copy link
Contributor

Hi @Sonali-More-Xandr and @gargcreation1992 - is there anything else we need to do for this PR to be approved? Balbina is on the dev team here at Sharethrough, and while we value her approval of our code changes, my understanding is that we need a Prebid contributor to provide one as well. Let us know if you have any questions or need anything further from us. Thank you for your efforts!

@@ -137,6 +137,9 @@ func (a *adapter) MakeBids(request *openrtb2.BidRequest, requestData *adapters.R
if bidReq.Imp[0].Video != nil {
bidType = openrtb_ext.BidTypeVideo
}
if bidReq.Imp[0].Native != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning:- we recommend setting the bidType based on bidder response. Your bidder should be able to set seatBid.Bid.Ext.Prebid.Type field and use that field here to get bidType.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed with above comment.
This is an anti-pattern. This code assumes that if there is multi-format request then mediatype is defaulting to native. Ideally, we would like to set media type based on adapter response. bidresponse.seatbid.bid[i].ext.prebid.type field identifies mediaType. So, your adapter should set this field in its response to define media type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the great feedback @Sonali-More-Xandr @gargcreation1992 . Updates in new commit (96c978f).

Copy link
Contributor

@gargcreation1992 gargcreation1992 left a comment

Choose a reason for hiding this comment

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

left a comment over multi-format request.

* Changing how we set media type in bid response (inside the `MakeBids` function).
* Updating / expanding test data sets so that the new way we do this can be validated.
}

bidderResponse.Bids = append(bidderResponse.Bids, &adapters.TypedBid{
BidType: bidType,
Bid: &seatBid.Bid[i],
Bid: &bid,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use &seatBid.Bid[i] as you were doing previously. With this updated way, you will end up with a pointer pointing at the LAST element in the seatBid.Bid slice (since the iteration stops at that point) because bid variable is getting a copy of each entry, not the entry itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks I changed it here: 22f2e3e

@jefftmahoney
Copy link
Contributor

Hello @gargcreation1992 - do you need anything more from us for this PR? Does it look OK to you? And if yes, do know when it might get merged (is there a schedule when that happens)?

@gargcreation1992 gargcreation1992 merged commit de804a5 into prebid:master May 26, 2023
3 checks passed
@maxime-dupuis maxime-dupuis deleted the md-jm/IG-178207154/Sharethrough-Native branch May 26, 2023 13:03
blueseasx pushed a commit to blueseasx/prebid-server that referenced this pull request Jun 2, 2023
Co-authored-by: Jeff Mahoney <jeff.t.mahoney@gmail.com>
pm-nilesh-chate added a commit to PubMatic-OpenWrap/prebid-server that referenced this pull request Jun 5, 2023
ShriprasadM pushed a commit to PubMatic-OpenWrap/prebid-server that referenced this pull request Jul 14, 2023
* add improvements to ow module

* use concrete adunit, major refactor, complete tracker

* complete logger

* logger complete

* go mod tidy

* INCOMPLETE: move hookexecutor at request level

* multi cur and pubmatic seat = 0

* INCOMPLETE: move hookexecutor at request level

* bug fixes 1

* add module rejection and start adunitconfig

* adunit cfg done

* minor fixes

* minor fixes 2

* use rctx for response, logger, tracker

* typo

* minor improvements

* add processed auction hook

* send all bids done

* minor fix: send all bids

* fix: vast tag cache timeout

* assert ow bidder initialisation

* fix: log dropped bids in owlogger

* fix the condition to eval droppedbids

* owlogger always logs wb

* fix account level bidderparams

* add nobid

* fix: false nobids

* remove stale non prebid ext

* matched impression

* user seeat for partnerid in wl

* complete video bid ext

* fix pn in wl

* complete marketplace wl and minor request patches

* complete regex and minor bug fixes

* fix adapter throttle

* fix wl pn

* verbose logging for module panic

* add module ctx assertion logs

* fix pn in wt

* af for nobid

* TEMP: LOCAL DOCKER

* add response.ext.sendallbids

* fix af

* dont log throttle-nobid in wl

* ignore manual_build

* fix: dont log throttle-nobid in wl

* ab test

* temp. fix bid.ext.prebid.video

* tgid if wl

* case insensitive slotname

* fix: client config

* typo

* fix1 GetClientConfigForMediaType

* remove duplicate bidext.perbid

* pubmatic default mapping refactor, regex todo

* add bidExt.Prebid nil assertion

* REVIEW: kgp, kgpv, kgpsv

* fix imp.ext.reward

* nobid for non-mapped bidders

* assert module rejection + alias

* fix bidder param hw typo

* slot regex + package cache refactor

* fix generate slot regex based name

* fix slot key gen

* regex slot wl fix

* wt kgpv

* fix dmx alias

* kgpsv typo

* macro ssai fix

* drop bids safely

* improve contenttransperencyobject.go

* fix video adunit apply typo

* sendallbids typo

* sendallbids typo 2

* video adunit battr typo

* go fmt

* change gomod name

* refactor nobid cases

* fix: fill fieldMap for pubmatic

* fix marketplace wl wt kgp*

* fix pubmatic kgpv overwrite

* fix pn pubmatic for marketplace bids

* fix: adunit regex check and req.user nil  typo

* fix wl gdpr

* fix user consent in wl

* add request.device validation

* minor wl refactor

* client client config banner format condition

* do not sent client config if it is disabled

* multiformat wl slot

* fix typo

* ignore 0x0 in sz

* fix last bad fix

* refactor sz

* refactor GetClientConfigForMediaType

* handle nobid GetClientConfigForMediaType

* handle imp type + fix ExtDevice

* fix multiformat kgpv kgpsv

* Check when bid.H and bid.W will be zero with Price !=0. Ex

* handle nil device.ext

* p1 GetClientConfigForMediaType

* get AU for V and B but apply only if req not nil

* move ow bidder param to pbs

* compete tracker

* fix tracker wrong param reference

* inject only one tracker in video

* fix dmx

* logInfo and minor refactor

* loginfo minor typos

* fix device ext ifa missing cases

* fix test = 1

* basic nbr, remove errorcodes

* add nbr scenarios

* fix auction race condition and floors adunit type

* add floor rules wrapper logger changes

* add pyroscope

* fix race condition

* add l1 in wl and minor wl refactor

* add rejected bids to wl

* rebuild tracker url for groupm

* fix fv in wl

* minor improvement for fr in wl

* default imp floor assertion

* add bidderparams for pubmatic2

* dv 360

* fix pubmatic2

* fix pb2

* add seatnobid in response

* typo

* include full bid in seatnobid

* update response ext with nbr

* minor imp

* update responseext with ow fields

* refactor response ext

* minor resp rctx refactor

* fix analyctis's request is nil for entry hook rejection

* minor refactor

* use concrete response ext type

* add client config for seatnobid

* fix wl for nbr

* refactor tmax

* add pprof

* add http_counter metrics

* move db queries to config

* refactor db query create

* fix wl client

* temp: video endpoint

* minor debug log improvements

* pyroscope read hostname

* pbs fix and minor improvement

* minor improvement in wl

* wl remove dead code

* use internal wl endpoint for call from s2s

* use gocommon client for wl

* add db connect log

* minor refactor

* add entrypoint hook test

* minor entrypoint refactor and test update

* prc 1

* bidderparams refactor

* remove redudant module to get account id

* remove pyroscope

* minor refactor

* fix: rebase typos

* club ow + sshb

* fix unimportable main

* update openrtb v17 to v19

* revert wrong commit

* use empty hook

* comment seatnonbid

* revert unintended changes

* fix typo

* remove duplicate content

* rename pbs.yaml

* remove config param mapping

* fix formatting

* fix module name

* add sshb flag

* fix sshb path checks

* update sshb tests

* fix FloorValueUSD UT

* Revert "Sharethrough: Support native ads (prebid#2780)"

This reverts commit de804a5.

* Revert "Appnexus: Add ext_inv_code and external_imp_id parameters (prebid#2792)"

This reverts commit 225e31e.

* Revert "Move httputil/responseutil to adapters/util and adapters/openrtb_util to exchange/util (prebid#2788)"

This reverts commit 0615109.

* Revert "PREB-39 added support for imp.ext and added imp.ext.skadn to it (prebid#2770)"

This reverts commit d527e2f.

* Revert "HuaweiAds: Handle getDeviceIDFromUserExt when ifa and user.ext are set (prebid#2796)"

This reverts commit 3a5d7c9.

* Revert "GPP in the cookie sync endpoint (prebid#2757)"

This reverts commit 708d917.

* OTT-1104: Move the stat-server from HB to prebid

* address test = 1 and 2

* fix test=2

* rename analytics and remove unused files

* remove wrong commit

* minor refactor

* add mockgen and db test

* migrate fsc db and cache code

* create cache mock

* update db mock

* add db sql driver mock

* OTT-1124: add graceful shutdown function for stats-server (#512)

* update universalpixel in auc

* make UniversalPixel ptr in auc

* correct UniversalPixel type in auc

* OTT-1073: add stats at before-validation stage

* OTT-1073: updated go.mod

* OTT-1124: add shutdown function for stats metric-engine

* OTT-1073: add shutdown across all metric-engine

* OTT-1073: adding stats at auction-response stage

* OTT-1073: add pubIDStr and profIDStr in rCtx

* OTT-1073: added UT for multi metric engine

* OTT-1073: remove unwanted comments

* OTT-1127 : Removed un-used stats metrics (#517)

* OTT-1127: remove unused functions from metricEngine

* auc: add native

* auc: add lower case kv of config

* auc: fix config json tag

* auc: add omitempty

* OTT-1073: record bad-requests, bidder-error stats, resp-time

* auc: uncomment json tag

* OTT-1073: record wrapper-logger-failures

* fix err typo in GetActivePartnerConfigurations

* disable floors wl code until modules is enabled

* Revert "disable floors wl code until modules is enabled"

This reverts commit 3354753.

* TEMP: coment FloorValueUSD

* OTT-1073: RecordMisConfigurationErrorStats

* OTT-1073: RecordPublisherPartnerNoCookieStats

* OTT-1073:RecordPublisherRequests

* OTT-1073: address review comments

* OTT-1073: address review comments

* OTT-1073: minor fix

* OTT-1073: fix GetLogAuctionObjectAsURL

* OTT-1073: make metricsengine as a part of request-ctx

* OTT-1073: do not record stat-metric for owlogger failure

* OTT-1073: fix error statement in logAuctionObject

* OTT-1073: remove log statement from LogAuctionObject

---------

Co-authored-by: Nilesh Chate <nilesh.chate@pubmatic.com>
Co-authored-by: Ankit-Pinge <128145896+Ankit-Pinge@users.noreply.github.com>
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 this pull request may close these issues.

None yet

5 participants