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 Analytics Adapter: added support for Bidder alias #7153

Merged
merged 6 commits into from
Jul 13, 2021

Conversation

pm-azhar-mulla
Copy link
Contributor

Type of change

  • Feature

Description of change

Added support for Bidder alias in pubmaticAnalyticsAdapter.
Added a new field "bc" to log the bidder code for adapter.

@aleksatr
Copy link
Contributor

aleksatr commented Jul 8, 2021

Hi @pm-azhar-mulla , I just noticed that these changes will work only if the bidder alias is all lowercase letters:
image

I've tried the following example:

var adUnits = [{
    code: 'div-gpt-ad-1460505748561-0',
    mediaTypes: {
        banner: {
            sizes: [[300, 250], [300,600]],
        }
    },
    // Replace this object to test a new Adapter!
    bids: [{
        bidder: 'bRealTime1',
        params: {
            placementId: 13144370
        }
    }]

}];

...

pbjs.que.push(function() {
    pbjs.aliasBidder('appnexus', 'bRealTime1');

    pbjs.addAdUnits(adUnits);

    pbjs.enableAnalytics({
        provider: 'pubmatic',
        options: {
            "publisherId": 12345 // please contact PubMatic to get a publisherId for yourself
        }
    });

    pbjs.requestBids({
        bidsBackHandler: sendAdserverRequest,
        timeout: PREBID_TIMEOUT
    });
});

...

And what I see get sent to your endpoint is ... "pn":"brealtime1","bc":"brealtime1" ...

@pm-azhar-mulla
Copy link
Contributor Author

Hi @pm-azhar-mulla , I just noticed that these changes will work only if the bidder alias is all lowercase letters:
image

I've tried the following example:

var adUnits = [{
    code: 'div-gpt-ad-1460505748561-0',
    mediaTypes: {
        banner: {
            sizes: [[300, 250], [300,600]],
        }
    },
    // Replace this object to test a new Adapter!
    bids: [{
        bidder: 'bRealTime1',
        params: {
            placementId: 13144370
        }
    }]

}];

...

pbjs.que.push(function() {
    pbjs.aliasBidder('appnexus', 'bRealTime1');

    pbjs.addAdUnits(adUnits);

    pbjs.enableAnalytics({
        provider: 'pubmatic',
        options: {
            "publisherId": 12345 // please contact PubMatic to get a publisherId for yourself
        }
    });

    pbjs.requestBids({
        bidsBackHandler: sendAdserverRequest,
        timeout: PREBID_TIMEOUT
    });
});

...

And what I see get sent to your endpoint is ... "pn":"brealtime1","bc":"brealtime1" ...

Removed the code which was converting bidder to lowercase, @pm-harshad-mane you remember any reason why were converting bidder to lowercase?

@pm-azhar-mulla
Copy link
Contributor Author

Can someone please re-run the test jobs (CI)

@ChrisHuie ChrisHuie changed the title [PubMatic] Added support for Bidder alias in pubmaticAnalyticsAdapter PubMatic Analytics Adapter: added support for Bidder alias Jul 12, 2021
@ChrisHuie ChrisHuie self-requested a review July 12, 2021 07:15
@aleksatr aleksatr added the minor label Jul 12, 2021
@aleksatr
Copy link
Contributor

@pm-azhar-mulla if you can confirm that we can move forward with the removal of lowercase conversion (for bidder name), I can merge this.

@pm-harshad-mane
Copy link
Contributor

Hello @aleksatr , @pm-azhar-mulla
Please go ahead with the change.

@aleksatr aleksatr added the LGTM label Jul 13, 2021
@aleksatr aleksatr merged commit 3cfbf9d into prebid:master Jul 13, 2021
prebidtappx pushed a commit to prebidtappx/Prebid.js that referenced this pull request Nov 15, 2021
* Changed net revenue to True

* Added support for bidderAlias

* Removed the code converting bidder to lowercase

Co-authored-by: Azhar <azhar@L1119.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants