Support Collecting Arbitrary ORTB Parameters #941

jsligh · 2023-12-13T16:57:34Z

Closes #877

…or deinit.

…o do if other parameters are there.

PrebidMobile/PrebidMobileRendering/Prebid/PBMCore/AdUnitConfig.swift

PrebidMobile/AdUnits/AdUnit.swift

YuriyVelichkoPI · 2023-12-15T14:50:45Z

PrebidMobile/PrebidMobileRendering/Prebid/PBMCore/PBMPrebidParameterBuilder.m

@@ -178,6 +178,7 @@ - (void)buildBidRequest:(nonnull PBMORTBBidRequest *)bidRequest {
        nextImp.extPrebid.storedAuctionResponse = Prebid.shared.storedAuctionResponse;
        nextImp.extPrebid.isRewardedInventory = self.adConfiguration.adConfiguration.isOptIn;
        nextImp.extGPID = self.adConfiguration.gpid;
+        nextImp.extOrtbObject = self.adConfiguration.ortbObject;


Hi @jsligh , @bretg !

I would like to clarify the scope of the implementation to provide the proper review.

For now, I see support only for the imp.ext field. So, the publisher can set the value of this field providing the respective JSON.

However, from my understanding of the feature request, this new API should provide the ability to set any part of the request with the given objects.

For instance, the publisher should be able to set the whole imp object like:

adUnit.setOrtb(" { "imp": [ { "id": "1", "bidfloor": 0.03, "banner": { "h": 250, "w": 300, "pos": 0 } } ] } ")

Or set the app info like:

adUnit.setOrtb(" { "app": { "id": "agltb3B1Yi1pbmNyDAsSA0FwcBiJkfIUDA", "name": "Yahoo Weather", "cat": [ "IAB15", "IAB15-10" ], "ver": "1.0.2", "bundle": "12345", "storeurl": "https://itunes.apple.com/id628677149", "publisher": { "id": "agltb3B1Yi1pbmNyDAsSA0FwcBiJkfTUCV", "name": "yahoo", "domain": "www.yahoo.com" } }, })

Note: these examples are copy-pasts from the Opertb doc just for demo purposes. Don't treat them as an implementation spec.

So, if the request is about the second variant, the implementation should be extended.

You're right @YuriyVelichkoPI that the goal is for the SDK to be able to supply any arbitrary ORTB parameter.

My suggestion in the original issue was to break this into two parts: the "global" part and the "imp-level" part:

addOrtbGlobal('{"ext":{"prebid":{"debug":1,"trace":"verbose"}}}'); adUnit = BannerAdUnit(configId: "my-stored-request", size: adSize, "{instl:1, ext:{ gpid: "222", data: { foo: "bar" }})

I don't see them every wanting to supply the entire imp object, but they need to be able to add any field under imp, not just in imp.ext.

Thanks for the clarification! I will set it to do fields for the whole imp object, not just imp.ext. @bretg should I also support arbitrary parameters in ext as well? because then we'll have to keep track of two separate objects.

The global ext object (and any other global params) would be set in a new function, e.g. addOrtbGlobal

… bidrequest

…meters

…or deinit.

…me-nil' of https://github.com/prebid/prebid-mobile-ios into 908-stop-viewabilitytimer-when-viewfortracking-has-become-nil

YuriyVelichkoPI · 2024-01-11T17:22:17Z

PrebidMobileTests/RenderingTests/Tests/ParameterBuilderTests/PrebidParameterBuilderTest.swift

+        let adUnit = AdUnit(configId: "test", size: CGSize.zero, adFormats: [.banner])
+        adUnit.setGPID(gpid)
+        adUnit.setGlobalOrtbObject(globalOrtb)
+        adUnit.setImpOrtbObject(impOrtb)


Hi @jsligh, @bretg !

The current implementation looks good, and it can address the needs described in the ticket. But also, it seems too fragile and publisher-dependent in the long-term perspective.

For example, by providing an API to set the Global object and Imp object independently sooner or later, we will get a situation when the publisher mixes these approaches, and the final request will have the wrong structure.

Also, correct me if I am wrong, but from what I see in the PBMORTBAbstractTest.swift, if a publisher provides, for example, the same ext object via the ortbObject and via AdUnit API, the filed will be duplicated in the final bid request. Instead, the data provided in ortbObject should override any other similar data. In the same time, we can't ignore data received from API fields because they can be set by 3p services, like identity providers, so the SDK update may lead to losing data in the bid request silently.

Also, there are no changes for Rendering and Mediation API in this PR.

As I told you from the very beginning, it is an extremely complex task that assumes a lot of design work first. So, instead of searching for implementation gaps, let's design the implementation spec first, and if it covers all the objectives, it is robust for integration and unambiguous for utilization - we'll update the PR accordingly.

I can provide a draft version early next week so we can discuss it.

cc: @alexsavelyev , @mmullin

Thanks for the thoughts Yuriy. My take is based on the experience we've had with Prebid.js:

ORTB is a standard. Developers have to understand it anyhow in order to debug.

Things in our industry change too quickly for an API to keep up

Therefore, instead of trying to design a new API function everytime there's a new feature, letting developers have direct access to ORTB is better in every way: smaller API footprint, more robust to continuing evolution, developers have less to learn.

I agree that the "rendering API" also needs this functionality, though as we discussed earlier today, I frankly have issues with a core assumption of that API -- that Prebid can always choose the winner. I'm going to open a separate issue to discuss that.

I agree with all points and statements except:

Developers have to understand it anyhow in order to debug.

With full respect, but from all our experience in mobile development on both sides - publishers and the ad tech industry, I should say that developers of mobile apps don't have a knowledge of ORTB.

Open RTB, as usual, is used in s2s cases. In the mobile world, the ad request (SDK --> SSP) has a custom format depending on each particular implementation (GMA SDK, AppLovin SDK, ironSource SDK). App developers, in their own turn, work only with the API of each particular SDK that provides a certain set of parameters to tune. This API and integration documentation is the final frontier of their knowledge of ad tech and what is happening behind the ad requests to the SSP (or other platforms).

Of course, with exceptions when publishers have an AdOps department that directly works on monetization. These guys have a great understanding of the ecosystem.

So, in order to prevent logical errors, it makes sense to restrict the ability to shoot themselves in the foot as much as possible.

@bretg @jsligh please review the proposal. For now, it is in the doc so you can easily edit it. Once everything is aligned we can copy the text here:

https://docs.google.com/document/d/1cRWDNO6i7wLyQdQA9ZzmK0QumIr2vvC40r5GhHhOSz4/edit?usp=sharing

Sorry if the intro sections are too long. I tried to highlight the context and pitfalls that impact the proposed design.

cc: @alexsavelyev @mmullin

…or deinit.

…me-nil' of https://github.com/prebid/prebid-mobile-ios into 908-stop-viewabilitytimer-when-viewfortracking-has-become-nil

… timer to fire every 0.1ms instead of 4 times/second. This hopefully will fix them.

…at the correct time!

…ewfortracking-has-become-nil Stop ViewabilityTimer when ViewForTracking has become nil

Changes for 2.2.0 release

Add carthage change

Update README.md

…tps://github.com/prebid/prebid-mobile-ios into 877-support-collecting-arbitrary-ortb-parameters

YuriyVelichkoPI

Except for the requested changes, I want to ask you to redo the branch and make it clean from changes from the master. For now, I see which changes are related to the parent task and which were merged from the master. But later whey can confuse.

The best way to keep the history clear is to follow the trunk-based development methodology: https://trunkbaseddevelopment.com/ as we did in mobile repositories.

It will help any collaborator to see the clear history that is especially important for the open source project.

YuriyVelichkoPI · 2024-02-13T14:16:48Z

PrebidMobile/AdUnits/AdUnit.swift

@@ -387,6 +387,16 @@ public class AdUnit: NSObject, DispatcherDelegate {
        return adUnitConfig.gpid
    }

+    // MARK: Global ORTBObject
+
+    public func setOrtbConfig(_ ortbObject: [String: Any]?) {


The custom request should be set as a String.

Publishers should just put the JSON string as a configuration option and not a dictionary.

YuriyVelichkoPI · 2024-02-13T14:17:30Z

PrebidMobile/PrebidMobileRendering/ORTB/PBMORTBBidRequest.m

@@ -68,6 +68,58 @@ - (nonnull PBMJsonDictionary *)toJsonDictionary {
    ext[@"prebid"] = [[self.extPrebid toJsonDictionary] nullIfEmpty];
    ret[@"ext"] = [[ext pbmCopyWithoutEmptyVals] nullIfEmpty];

+    //remove "protected" fields from ortbObject then do a merge but merge ret into the ortbObject (addEntriesFromDictionary)
+    NSMutableDictionary *o = [self.arbitraryJsonConfig mutableCopy];


We should avoid such names for variables.

YuriyVelichkoPI · 2024-02-13T14:20:05Z

PrebidMobile/PrebidMobileRendering/ORTB/PBMORTBBidRequest.m

+                    [ret setObject:[dictionary2 objectForKey: key] forKey:key];
+                }
+            }
+            //not sure what to do if array of objects


Not sure about this comment. Does it mean that the algorithm doesn't merge part of the data?

YuriyVelichkoPI · 2024-02-13T14:29:14Z

PrebidMobile/PrebidMobileRendering/ORTB/PBMORTBBidRequest.m

+    return ret;
+}
+
+- (nonnull PBMMutableJsonDictionary *)mergeDictionaries:(NSMutableDictionary*)dictionary1 joiningArgument2:(NSMutableDictionary*)dictionary2


In general, we can merge dictionaries. This path is faster to implement. However, it makes the solution restricted to such things as

RTB protocol validation (proper structure, objects, and types)

Warnings for objects prohibited from merging.

Upgrading protocol version. See the item 1 - the validation issue.

It is not critical, but I would still merge serialized objects. It is a more reliable approach from the support perspective.

YuriyVelichkoPI · 2024-02-13T14:31:23Z

PrebidMobileTests/RenderingTests/Tests/PBMORTBAbstractTest.swift

+
+        pbmORTBBidRequest.ortbObject = ["arbitraryparamkey1": "arbitraryparamvalue1", "tmax": 3000, "id": "1231234", "imp": ["hi": "hello"]]
+
+        codeAndDecode(abstract: pbmORTBBidRequest, expectedString: "{\"arbitraryparamkey1\":\"arbitraryparamvalue1\",\"id\":\"\(uuid)\",\"imp\":[{\"clickbrowser\":0,\"ext\":{\"dlp\":1},\"instl\":0,\"secure\":0}],\"test\":2,\"tmax\":2000}")


We need to cover more use cases here, including incorrect JSONs. Not overriding of reserved objects. Also the custom parameter should be a String but not Dictionary.

jsligh added 5 commits November 1, 2023 12:10

Makes unregister function public.

5c27578

checking for nil and invalidating timer but may have to dive deeper f…

0897cc5

…or deinit.

Added in multiple places just need to figure out the merging / what t…

e3b7521

…o do if other parameters are there.

Loading and sending arbitrary values done.

dad6d27

Finished writing tests.

c85bf65

jsligh requested review from YuriyVelichkoPI and OlenaPostindustria December 13, 2023 16:57

jsligh self-assigned this Dec 13, 2023

jsligh linked an issue Dec 13, 2023 that may be closed by this pull request

Support collecting arbitrary ORTB parameters #877

Closed

Update GAM version

c3c62be

YuriyVelichkoPI requested changes Dec 15, 2023

View reviewed changes

jsligh added 10 commits December 15, 2023 12:05

whoops, removing this.

65fa06b

Finished imp and imp.ext levels. Need to do global ext.

8ec26b3

Support all the way through.

2f3f6ba

I believe this covers the implementation for the global ext object in…

a642064

… bidrequest

Added globally.

86cc5df

Merge branch 'master' into 877-support-collecting-arbitrary-ortb-para…

e52f234

…meters

fixes

79fde02

more syntax

b029432

Added back imp-level and tests.

d7ab955

Remove from test.

473684e

jsligh marked this pull request as draft January 4, 2024 19:21

jsligh added 4 commits January 4, 2024 17:22

Fixed test

5579256

Makes unregister function public.

452b4e6

checking for nil and invalidating timer but may have to dive deeper f…

8a87df7

…or deinit.

Merge branch '908-stop-viewabilitytimer-when-viewfortracking-has-beco…

c44632a

…me-nil' of https://github.com/prebid/prebid-mobile-ios into 908-stop-viewabilitytimer-when-viewfortracking-has-become-nil

YuriyVelichkoPI reviewed Jan 11, 2024

View reviewed changes

jsligh added 3 commits January 11, 2024 11:33

Makes unregister function public.

036bdbb

checking for nil and invalidating timer but may have to dive deeper f…

27747b5

…or deinit.

Merge branch '908-stop-viewabilitytimer-when-viewfortracking-has-beco…

491bca5

…me-nil' of https://github.com/prebid/prebid-mobile-ios into 908-stop-viewabilitytimer-when-viewfortracking-has-become-nil

jsligh and others added 11 commits January 16, 2024 10:59

We believe there may be some type casting issues that are causing the…

0280711

… timer to fire every 0.1ms instead of 4 times/second. This hopefully will fix them.

Needed to change the check bounds to make sure isIABViewable is true …

e8c17c0

…at the correct time!

removed previous changes.

428e9a5

Proposed changes.

a9d03e4

Merge pull request #949 from prebid/908-stop-viewabilitytimer-when-vi…

88cf61d

…ewfortracking-has-become-nil Stop ViewabilityTimer when ViewForTracking has become nil

changes for 2.2.0 release

8b7b66e

Merge pull request #952 from prebid/prerelease-2.2.0

7034d23

Changes for 2.2.0 release

Update README.md

be74795

Add carthage change

Merge pull request #953 from prebid/jsligh-patch-1

46f53b7

Update README.md

Merge branch '877-support-collecting-arbitrary-ortb-parameters' of ht…

faacf22

…tps://github.com/prebid/prebid-mobile-ios into 877-support-collecting-arbitrary-ortb-parameters

Pretty Sure my merging logic is correct.

843ad3a

YuriyVelichkoPI requested changes Feb 13, 2024

View reviewed changes

jsligh mentioned this pull request Feb 15, 2024

877 Supporting Arbitrary ORTB Params #956

Merged

jsligh closed this Feb 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Collecting Arbitrary ORTB Parameters #941

Support Collecting Arbitrary ORTB Parameters #941

jsligh commented Dec 13, 2023

YuriyVelichkoPI Dec 15, 2023 •

edited

Loading

bretg Dec 15, 2023

jsligh Dec 15, 2023 •

edited

Loading

bretg Dec 15, 2023

YuriyVelichkoPI Jan 11, 2024 •

edited

Loading

bretg Jan 11, 2024

YuriyVelichkoPI Jan 11, 2024 •

edited

Loading

YuriyVelichkoPI Jan 15, 2024

YuriyVelichkoPI left a comment

YuriyVelichkoPI Feb 13, 2024

YuriyVelichkoPI Feb 13, 2024

YuriyVelichkoPI Feb 13, 2024

YuriyVelichkoPI Feb 13, 2024

YuriyVelichkoPI Feb 13, 2024


		pbmORTBBidRequest.ortbObject = ["arbitraryparamkey1": "arbitraryparamvalue1", "tmax": 3000, "id": "1231234", "imp": ["hi": "hello"]]

		codeAndDecode(abstract: pbmORTBBidRequest, expectedString: "{\"arbitraryparamkey1\":\"arbitraryparamvalue1\",\"id\":\"\(uuid)\",\"imp\":[{\"clickbrowser\":0,\"ext\":{\"dlp\":1},\"instl\":0,\"secure\":0}],\"test\":2,\"tmax\":2000}")

Support Collecting Arbitrary ORTB Parameters #941

Support Collecting Arbitrary ORTB Parameters #941

Conversation

jsligh commented Dec 13, 2023

YuriyVelichkoPI Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsligh Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YuriyVelichkoPI Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YuriyVelichkoPI Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YuriyVelichkoPI left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YuriyVelichkoPI Dec 15, 2023 •

edited

Loading

jsligh Dec 15, 2023 •

edited

Loading

YuriyVelichkoPI Jan 11, 2024 •

edited

Loading

YuriyVelichkoPI Jan 11, 2024 •

edited

Loading