Add Geoedge RTD provider submodule

[GeoEdge-r-and-d]

• Add Geoedge RTD provider submodule according to RTD phase 3
• Add tests
• Add docs
• Add integration example
• Add as child module of RTD for easier builds

Type of change

• [x ] Feature

Description of change

Added new Geoedge RTD provider submodule to allow publishers to integrate Geoedge real time solution with Prebid.js.

Other information

#4610 #5519 @omerBrowsi @Asafsham

[bretg]

@GeoEdge-r-and-d - sorry for the delay here. We just released a major revision to the Real-Time-Data infrastructure. You need to read https://docs.prebid.org/dev-docs/add-rtd-submodule.html and adjust your implementation.

What your module is actually trying to do is not clear to me from the description, but here's what I gather:

• you're not obtaining any data from an endpoint
• you're forcing bidders to load functions from your library. We cannot accept this design.
• all you really want to do is wrap some bidder creatives

If I have this right, you need to change your design:

• just use the onBidResponseEvent listener to sniff for bidresponses and modify them.
• done. no need to force bidders to import your code or do anything special.

[GeoEdge-r-and-d]

@bretg Thank you very much for reviewing and providing this feedback!
You absolutely got it right, we adjusted our implementation accordingly. We only use the onBidResponseEvent now. Thanks again!

[bretg]

Also please create a docs pull request as described in https://docs.prebid.org/dev-docs/add-rtd-submodule.html#step-5-website-pull-request

I made a round of revisions to that page so hopefully future sub-module authors will learn from this interaction. Would certainly appreciate your feedback.

[GeoEdge-r-and-d]

@bretg Thank you very much! We changed the code to match your suggestions and the new RTD submodule doc. We also submitted a docs PR as described.

[bretg]

Ok, structure of the module is looking better. Will assign someone to look deeper at the javascript. Now we have to talk about this external code. In order to protect publisher pages, Global Module Rule #1 is No External Code. Looking at wrapper.html, it's pretty small, but it loads another file: https://rumcdn.geoedge.be/$%7Bkey%7D/grumi.js.

You're going to have to figure out how to live within this Module Rule, basically, all code that's loaded must be inspectable, disclosed, and locked to a specific version so we know when you've changed it. Since the whole point of GeoEdge is to help protect publishers, I suspect you'll understand why this rule exists. :-)

If wrapper.html may be more-or-less static, you might consider including that right in this code. grumi.js probaby depends on the publisher, so for that you're going to need to A) open source it and B) lock it to a version, e.g. grumi-v1.js

If grumi.js needs data (e.g. malware patterns), you might consider:

• separate the code from the data. Loading data files is fine.
• make grumi.js as static as possible and put your application logic server-side

[bretg]

@Fawke - could you take a look at the .js and unit tests? I'll work with them on the external code. Thanks!

[GeoEdge-r-and-d]

@bretg since open sourcing grumi.js isn't an option for us and we value your and @Fawke's time, please hold on the review process for now. We need to discuss potential solutions internally, and will update here if we come up with something viable. Thanks again!

[bretg]

The rules committee has approved the use of external code here with the condition of a prominent disclosure on the documentation.

prebid/prebid.github.io#2490

    - A Real-Time Data module may load external code if it requires publisher registration and there's a prominent disclosure on the module documentation. The idea is that a publisher will not include the module if they don't approve of the external code, and since they've registered for the service, they must approve. The text of the disclosure may differ if the vendor allows Prebid to do regular reviews of a strictly versioned proprietary library.

I've added a draft to the docs PR prebid/prebid.github.io#2474

[GeoEdge-r-and-d]

good news! thanks @bretg for bringing this up to the rules committee and fixing the docs! Are there any more changes needed from our side?

[Asafsham]

@bretg Let me know if another reviewer is required here, I'll get someone from the team to review.
Thanks @GeoEdge-r-and-d for making these swift changes.

[bretg]

My review covers general structure, not detailed javascript interaction

[Fawke]

Hi @GeoEdge-r-and-d,

Overall, this looks good. Just look at my inline comments and also, I wanted to mention that you should hardcode the request that you're making for both wrapper.html and grumi.js to https.

[GeoEdge-r-and-d]

@Fawke thanks for reviewing! We changed the code to match your requests. Please let us know if anything else is needed.

[Fawke]

Hi @GeoEdge-r-and-d,

Thanks for making those changes. I think it's better if you remove the html and a separate PR documenting usages of all RTD providers would be better.

[GeoEdge-r-and-d]

Thanks @Fawke, we can immediately remove the html. As for all the rest of RTD providers, where should it be and how should it look like?

[bretg]

separate PR documenting usages of all RTD providers would be better

I think this is just the docs PR isn't it? prebid/prebid.github.io#2474

So I think we're done here.

[Fawke]

LGTM

[Fawke]

Thanks @Fawke, we can immediately remove the html. As for all the rest of RTD providers, where should it be and how should it look like?

The example html page (the one with working examples of all RTD submodules) is not required for this PR, but good to have for testing.

[GeoEdge-r-and-d]

@bretg and @Fawke, thanks for reviewing and providing feedback. We see there's a pending request for review from @ncolletti, please let us know if anything else is needed from our side. thanks!

[bretg]

Merging - our reviews are enough.