Showheroes update

[FilipStamenkovic]

Type of change

• Updated bidder adapter <!-- IMPORTANT: (1) consider whether you need to upgrade your bidder parameter documentation in

Description of change

Update the showheroes-bs bidder adapter.

PR has following updates:

• remove legacy code that is not supported anymore
• replace the old endpoint with prebid-sh with new openrtb2/auction
• use ortbConverter to construct bid request
• in case of outstream ads, provide the renderer related parameters in the response (old static renderer is not working anymore)
• by using openRTB, automatically add support for: fpd, floor module, eids, etc.

Other information

[github-actions]

Tread carefully! This PR adds 1 linter error (possibly disabled through directives):

• modules/showheroes-bsBidAdapter.js (+1 error)

[musikele]

@FilipStamenkovic the file showheroes-bsBidAdapter.md should be reviewed and updated, because there are two sets of test parameters

[musikele]

LGTM - no point in adding banner example ?

[musikele]

LGTM

[francesconistri]

I think we are mixing different things together here.

The previous banner implementation where based on the "legacy" ShowHeroes Player (pre viralize).

I don't really know if it's still up, I'm just seeing that https://bs.showheroes.com/api/v1/bid is returning 503 error.
As far as I remember that wasn't based on prebid server, thus I don't understand how can we use getBannerHtml with the current prebid server response.

I think we have to gather all the information about the legacy banner implementation and decide what to do in the new one.

[FilipStamenkovic]

We already have the banner implementation in the viralize repo: https://github.com/showheroes/viralize/blob/master/viralize/web/prebid.py#L43

I honestly don't know how it's working, but that part is unchanged. In our roadmap, we have to work on banner support for our bidder because, as far as I know, it currently works: it will just load the player and start the waterfall.

[jmsalamon]

nobody is using the banner prebid and legacy showheroes is down. Let's remove the display part for now.

[francesconistri]

typo in the variable name

[musikele]

thanks!

[francesconistri]

We are choosing banner as the default context, however our banner integration might be broken (look at the comment below)

[musikele]

I'll set VIDEO as default given that banner doesn't seem to be working

[francesconistri]

are these two modifications needed?

Can they just be ignored server side?

[FilipStamenkovic]

I get it's premature optimization, but I just thought about not sending additional bytes of data for the things we are not using.

[francesconistri]

What is the effect of setting this property?

Normally on QA we perform e2e tests with settings very similar to production

[musikele]

On adserver, we serve a rule that always returns an ad

[francesconistri]

What kind of ad?

If we want to performa a complete e2e test in qa, it will be impossible to try the prebid server with some real bidders?

[francesconistri]

Let's rename the variable.

The duality between SH and viralize was introduced to have a single bidder handling two different integrations, one with the ShowHeroes legacy player, the other with the newly acquired Viralize.

[musikele]

Renamed to SHOWHEROES_ENDPOINT, is it ok ?

[francesconistri]

Or just ENDPOINT

[francesconistri]

All this code might be obsolete if we will remove the legacy code (referring to banner)

[musikele]

do we want to remove banner? for the info in my possession, we have very small traffic for it, but I am not aware of plans to dismiss banner

[francesconistri]

I think we need to dig deeper on how the current adapter works, I'm referring to the "legacy" stuff.

All the services behind showheroes.com domain are not belonging to MAX, they are considered "legacy" (video-library.showheroes.com static.showheroes.com).

Moreover, the current Max banner implementation was never delivered through the Prebid adapter.

[francesconistri]

Since we have completely changed the server returning the bidResponse, we should double check if all of these params are needed.
For example, is still needed to add prefix to creativeId ?
What is advertiserDomains ?

[FilipStamenkovic]

Most probably creativeId is not needed. My guess was it was needed for the previous renderer, but since that is no longer working it's not needed. But then again, on the other hand, we don't have a renderer in place, so at this point, we can't be really sure what is really needed and what isn't.

advertiserDomains is something each bidder must provide in response, it's a domain of advertisers delivering the ad.
It's being passed here: https://github.com/showheroes/viralize/blob/PLT-2267-adserver-add-dynamic-price-for-prebid-integration/viralize/web/executors/ad/auction.py#L131

[francesconistri]

Here, contrary to what we did before, we are considering "VIDEO" as a fallback

[francesconistri]

What type of object is response?
If I'm not wrong is something out of our direct control, coming from Prebid.js.
I'm wondering if it is ok to mix response and renderOptions, this might lead to some unwanted properties override.

Moreover it's not clear the contract between the renderer function and the argument it can receive.

[FilipStamenkovic]

I left like that on purpose. To be free to change/modify from the backend side whatever we want.
I didn't know what else to do. Right now we are serving appnexus renderer and we want to change that with showheroes renderer. So, to be able to control from the backend side without opening another PR in the pbjs repo.

I know it's not ideal, I know it's ugly and error-prone. But, until we don't have our renderer in place, I really don't see the other way. Then we can open a PR in pbjs and cleanup the mess that is left behind.

Another option is to hardcode appnexus renderer inside pbjs, and then 2nd PR to replace it with ours. Code would be a lot cleaner. Let me know what you think?

[francesconistri]

I was just looking at the ANOutstreamVideo.renderAd arguments and how other bidders are using it (simplest one).

Maybe it's better to explicitly return the object passed to the renderer directly from the server response

function outstreamRender(response, renderConfig) {
  response.renderer.push(() => {
    const func = deepAccess(window, renderConfig.renderFunc);
    if (!isFn(func)) {
      return;
    }
    func(renderConfig.renderOptions);
  });
}

This means that adserver must be changed with:

if media_type == settings.PREBID_FORMAT_OUTSTREAM:
    openrtb_bid.ext["rendererConfig"] = {
        "rendererUrl": RENDERER_URL,
        "renderFunc": "ANOutstreamVideo.renderAd",
        "renderOptions": {
            "showBigPlayButton": False,
            "showProgressBar": "bar",
            "skippable": False,
            "targetId": adunit_code,
            "adResponse": {
                "content": winning_xml
            }
        },
    }

[francesconistri]

As discussed in private conversation, we'd like to use (if possible) converter.fromORTB by following the docs

[FilipStamenkovic]

It is possible, but unfortunately, we have many customizations that I would have to extend the code and I wouldn't get any specific benefit. We have:

• extra param (that I think it's going to be needed for the player later on)
• callbacks that are fired when our bidder wins the auction
• renderer specific items

[FilipStamenkovic]

aaah, I jumped the gun and responded before turning on my brain. Actually using the fromORTB can remove 20 lines of code, which is great. I'll update the code

[francesconistri]

This line looks suspicious, maybe you wanted to write:

if (bidResponse.mediaType === VIDEO && bidRequest.mediaTypes.video.context === 'outstream') {

[francesconistri]

I was just looking at the ANOutstreamVideo.renderAd arguments and how other bidders are using it (simplest one).

Maybe it's better to explicitly return the object passed to the renderer directly from the server response

function outstreamRender(response, renderConfig) {
  response.renderer.push(() => {
    const func = deepAccess(window, renderConfig.renderFunc);
    if (!isFn(func)) {
      return;
    }
    func(renderConfig.renderOptions);
  });
}

This means that adserver must be changed with:

if media_type == settings.PREBID_FORMAT_OUTSTREAM:
    openrtb_bid.ext["rendererConfig"] = {
        "rendererUrl": RENDERER_URL,
        "renderFunc": "ANOutstreamVideo.renderAd",
        "renderOptions": {
            "showBigPlayButton": False,
            "showProgressBar": "bar",
            "skippable": False,
            "targetId": adunit_code,
            "adResponse": {
                "content": winning_xml
            }
        },
    }

[francesconistri]

Remember to change the adserver implementation about the video context

[musikele]

LGTM

[FilipStamenkovic]

Official PR opened.