-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
51Degrees RTD submodule: initial commit #11414
51Degrees RTD submodule: initial commit #11414
Conversation
…g additional information and resources. Added an exception if the resourceKey parameter is not updated from the example. Added a customer notices section to the readme. Used User Agent Client Hint (UA-CH) consistently in the documentation.
['SmartWatch', ORTB_DEVICE_TYPE.CONNECTED_DEVICE], | ||
['Tablet', ORTB_DEVICE_TYPE.TABLET], | ||
['Tv', ORTB_DEVICE_TYPE.CONNECTED_TV], | ||
['Vehicle Display', ORTB_DEVICE_TYPE.PERSONAL_COMPUTER] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully not too many drivers viewing ads ;)
logMessage('On-premise JS URL: ', onPremiseJSUrl); | ||
|
||
// Get 51Degrees JS URL, which is either cloud or on-premise | ||
const scriptURL = get51DegreesJSURL(resourceKey ? {resourceKey} : {onPremiseJSUrl}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're using prebid to inject, we prefer you lock versions and pr the project when the script changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script is generated by the 51Degrees Cloud API that is version-locked through its URL: https://cloud.51degrees.com/api/v4/<your_resource_Key>.js
, however the script itself is generated based on resource-key and properties user selects for it - so we would not be able to store its content as part of this repo. The javascript template and device detection library used by the cloud are open source (as mentioned in our docs):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you dm me a copy of something running; I want to check for example, you don't gather bids or identifiers from other companies and send them to your endpoint, or other obvious rules violations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few remarks in line
} | ||
|
||
/** | ||
* Check if meta[http-equiv="Delegate-CH"] tag is present in the document head and points to 51Degrees cloud |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on here, why not just get the high entropy hints in js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. Agreed that in general it's a good idea to use getHighEntropyValues
API. The injected script actually uses it as a fallback in the case when navigator did not send User-Agent Client Hints as part of the first request. Having the navigator send the User-Agent Client Hints by itself seems to be a bit more performant and also transparent to the user that they give permission to a 3rd party to process those as prescribed by the spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW - PBJS-core will add high entropy device.sua if the publisher sets some config...
pbjs.setConfig({
firstPartyData: {
uaHints: ['platform', 'architecture', 'model', 'platformVersion', 'fullVersionList']
}
});
This may be something you want to suggest in your doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for raising this, @bretg. A lengthy comment ahead, but we hope it will become a basis for more documentation. Breaking it down below.
PBS modules
The First Party Data Enrichment module with uaHints mentioned above is very relevant for 51D Prebid Server Modules, and we'll actually add it to the server modules docs, because 51D PBS modules are more precise when device.sua
is present in the request.
Prebid.js module
51D Prebid.js module injects a dynamically generated script based on the evidence available. It also falls back to the GetHighEntropyValues API, but does not rely on it as a first (or only) choice route - please see the illustrative cases below. Albeit it seems easier, GHEV API is not supported by all browsers (so the decision to call it should be conditional) and also even in Chrome this API will likely be a subject to the Privacy Budget in the future.
In summary we use Delegate-CH
http-equiv as the preferred method of obtaining the necessary evidence because it is the fastest method, and future proof. Delegate-CH
enables the Chromium-based browser to send the User-Agent Client Hints to a 3rd party right away on the very first request unlike Accept-CH
+ Permissions-Policy
http response headers which would let it send them on subsequent requests. Hence this check in the code that outputs the warning in debug mode.
Illustrative Cases
-
if the device is iPhone/iPad then there is no point checking for or calling GetHighEntropyValues at the moment because iOS does not support this API. However this might change in the future.
Platforms like iOS require additional techniques to identify the model which are not covered via a single API call, and change from version to version of the operating system and browser rendering engine. When used with iOS 51Degrees resolves the iPhone/iPad model groups using these techniques. That is one of the benefits the module brings to the Prebid community as most solutions do not resolve iPhone/iPad model groups. More on Apple Device Detection here. -
if the browser is Brave on Android then there is similarly no point requesting GHEV as the API is not supported.
-
if the browser is Chrome then the
Delegate-CH
if enabled by the publisher would enable the browser to provide the necessary evidence. However if this is not implemented - then the dynamic script would fall back to GHEV which is slower.
As the code for these features changes frequently the relevant snippets are in the data file rather than the core source code. As the web diverges the ability to dynamically adapt is going to be very important. Reducing the depency on Javascript and maximising server side stable controlled environments is part of the 51Degrees approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jwrosewell in general it is bad form to mark reviewer comments as resolved. We let reviewers decide if their comments are resolved. Of course, if the item is fix this typo or use this alternate function, the author can indicate they have completed. Here, you are going a bit far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolving to make easier to find for future readers.
]; | ||
|
||
pbjs.setConfig({ | ||
debug: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a tiny bit dangerous in an example; i would add a comment here to tell devs to be careful not to run debug in production
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM. please add the tiny warning about debug to your example, please add the external script disclosure prominently on yours docs PR. Please add the long discussion on entropy hints vs headers to your documentation somewhere reasonably accessible.
as per review comment: - added a comment warning that debug flag should not be used in production - improved example page a bit with the testing/debugging guidance
addressing review comment: prebid/Prebid.js#11414 (review)
addressing review comment: prebid/Prebid.js#11414 (review)
addressing review comment: prebid#11414 (review) with a discussion on why Delegate-CH is more prefereable than GetHighEntropyValues API
…iew-comments 51d: addressing latest review comments
* 51Degrees RTD provider * Amended comments and documentation to improve references for obtaining additional information and resources. Added an exception if the resourceKey parameter is not updated from the example. Added a customer notices section to the readme. Used User Agent Client Hint (UA-CH) consistently in the documentation. * Modified the maintainer email address. * Change outdated URL * Adjust code to work on legacy browsers * Refactor a test of the `inject` method * Replace URL in a test method * 51Degrees RTD provider: remove redundant parameter from the example * 51Degrees RTD provider: update gpt.js URL in the example file * 51Degrees RTD provider: add schema to the `is51DegreesMetaPresent` method's URL * 51Degrees RTD provider: refactor `51Degrees` script injection method * 51Degrees RTD provider: show enriched device data in the example page * 51Degrees RTD provider: provide additional explanation for checking meta tags * 51Degrees RTD provider: update string for meta tag check * 51Degrees RTD provider: improve tests, reach 100% tests coverage * 51d example: debug warning and guidance as per review comment: - added a comment warning that debug flag should not be used in production - improved example page a bit with the testing/debugging guidance * 51d doc: add GetHighEntropyValues vs. Delegate-CH addressing review comment: prebid#11414 (review) with a discussion on why Delegate-CH is more prefereable than GetHighEntropyValues API * 51d: fix minor doc omissions --------- Co-authored-by: Bohdan V <25197509+BohdanVV@users.noreply.github.com> Co-authored-by: Eugene Dorfman <eugene.dorfman@gmail.com>
* 51Degrees RTD Provider documentation * 51Degrees RTD Provider documentation amendments * Fix markdown to comply with style rules. * Fix params table in 51DegreesRtdProvider.md * 51d: add a disclaimer on external script addressing review comment: prebid/Prebid.js#11414 (review) * 51d: add section on GetHigheEntropyValues API addressing review comment: prebid/Prebid.js#11414 (review) * 51d: slightly updated disclosure warning * 51d: fix minor omissions * 51d: slight disclosure wording improvement * 51d: remove hardcoded API limits, as they change --------- Co-authored-by: Bohdan V <25197509+BohdanVV@users.noreply.github.com> Co-authored-by: Eugene Dorfman <eugene.dorfman@gmail.com>
* 51Degrees RTD provider * Amended comments and documentation to improve references for obtaining additional information and resources. Added an exception if the resourceKey parameter is not updated from the example. Added a customer notices section to the readme. Used User Agent Client Hint (UA-CH) consistently in the documentation. * Modified the maintainer email address. * Change outdated URL * Adjust code to work on legacy browsers * Refactor a test of the `inject` method * Replace URL in a test method * 51Degrees RTD provider: remove redundant parameter from the example * 51Degrees RTD provider: update gpt.js URL in the example file * 51Degrees RTD provider: add schema to the `is51DegreesMetaPresent` method's URL * 51Degrees RTD provider: refactor `51Degrees` script injection method * 51Degrees RTD provider: show enriched device data in the example page * 51Degrees RTD provider: provide additional explanation for checking meta tags * 51Degrees RTD provider: update string for meta tag check * 51Degrees RTD provider: improve tests, reach 100% tests coverage * 51d example: debug warning and guidance as per review comment: - added a comment warning that debug flag should not be used in production - improved example page a bit with the testing/debugging guidance * 51d doc: add GetHighEntropyValues vs. Delegate-CH addressing review comment: prebid#11414 (review) with a discussion on why Delegate-CH is more prefereable than GetHighEntropyValues API * 51d: fix minor doc omissions --------- Co-authored-by: Bohdan V <25197509+BohdanVV@users.noreply.github.com> Co-authored-by: Eugene Dorfman <eugene.dorfman@gmail.com>
Type of change
Description of change
51Degrees module enriches an OpenRTB request with 51Degrees Device Data.
51Degrees module sets the following fields of the device object:
make
,model
,os
,osv
,h
,w
,ppi
,pixelratio
- interested bidder adapters may use these fields as needed. In addition the module setsdevice.ext.fiftyonedegrees_deviceId
to a permanent device ID which can be rapidly looked up in on premise data exposing over 250 properties including the device age, chip set, codec support, and price, operating system and app/browser versions, age, and embedded features.The module supports on premise and cloud device detection services with free options for both.
Integration details are specified in the
modules/51DegreesRtdProvider.md
.Maintainer contacts:
support@51degrees.com
Other details
The features provided by this module have been requested previously as part of: #8217