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

fix(ADJUtil): iad-adgroup-name == "AdGroupName" #523

Closed
wants to merge 1 commit into from

Conversation

buschtoens
Copy link
Contributor

@buschtoens buschtoens commented Mar 30, 2021

Alternative PR to #524.

After debugging for many hours over the course of a few days, I think I uncovered a typo bug.

Observed Behavior

image
image
image

Adjust keeps attributing Apple Search Ads with broken / dummy attribution data, even though:

  • an Apple Search Ad was never clicked or at least definitely not in the last 7 days
  • there is "correct" attribution data in the tracker deep link, which is discarded.

As you can clearly see from the screenshots, the...

  • CampaignName (1234567890) (iad-campaign-name, iad-campaign-id)
  • AdGroupName (1234567890) (iad-adgroup-name, iad-adgroup-id)

...are obviously invalid dummy data.

I have no clue which event unknown (12323222) is supposed to represent, but I can see that 12323222 is the iad-keyword-id. Furthermore, it also appears in this log message from #509 (comment) lamenting a related issue, making it very likely to be some form of dummy data.

Cause

ios_sdk/Adjust/ADJUtil.m

Lines 914 to 948 in cfbbd18

+ (BOOL)checkAttributionDetails:(NSDictionary *)attributionDetails {
if ([ADJUtil isNull:attributionDetails]) {
return NO;
}
NSDictionary *details = [attributionDetails objectForKey:@"Version3.1"];
if ([ADJUtil isNull:details]) {
return YES;
}
// Common fields for both iAd3 and Apple Search Ads
if (![ADJUtil contains:details key:@"iad-org-name" value:@"OrgName"] ||
![ADJUtil contains:details key:@"iad-campaign-id" value:@"1234567890"] ||
![ADJUtil contains:details key:@"iad-campaign-name" value:@"CampaignName"] ||
![ADJUtil contains:details key:@"iad-lineitem-id" value:@"1234567890"] ||
![ADJUtil contains:details key:@"iad-lineitem-name" value:@"LineName"]) {
[ADJAdjustFactory.logger debug:@"iAd attribution details has dummy common fields for both iAd3 and Apple Search Ads"];
return YES;
}
// Apple Search Ads fields
if ([ADJUtil contains:details key:@"iad-adgroup-id" value:@"1234567890"] &&
[ADJUtil contains:details key:@"iad-adgroup-name" value:@"AdgroupName"] &&
[ADJUtil contains:details key:@"iad-keyword" value:@"Keyword"]) {
[ADJAdjustFactory.logger debug:@"iAd attribution details has dummy Apple Search Ads fields"];
return NO;
}
// iAd3 fields
if ([ADJUtil contains:details key:@"iad-adgroup-id" value:@"1234567890"] &&
[ADJUtil contains:details key:@"iad-creative-name" value:@"CreativeName"]) {
[ADJAdjustFactory.logger debug:@"iAd attribution details has dummy iAd3 fields"];
return NO;
}
return YES;
}

I've placed a breakpoint in checkAttributionDetails and after a lot of back and forth found, that the issue is in L935:

ios_sdk/Adjust/ADJUtil.m

Lines 933 to 939 in cfbbd18

// Apple Search Ads fields
if ([ADJUtil contains:details key:@"iad-adgroup-id" value:@"1234567890"] &&
[ADJUtil contains:details key:@"iad-adgroup-name" value:@"AdgroupName"] &&
[ADJUtil contains:details key:@"iad-keyword" value:@"Keyword"]) {
[ADJAdjustFactory.logger debug:@"iAd attribution details has dummy Apple Search Ads fields"];
return NO;
}

This compares the value of iad-adgroup-name to AdgroupName. The intention is to reject attribution details from iAd that are just dummy data. It has a typo. The actual dummy data doesn't use AdgroupName with a lower-case g, but AdGroupName with an upper-case G.

# (lldb) po details
{
    "iad-adgroup-id" = 1234567890;
    "iad-adgroup-name" = AdGroupName;
    "iad-attribution" = true;
    "iad-campaign-id" = 1234567890;
    "iad-campaign-name" = CampaignName;
    "iad-click-date" = "2021-03-30T02:54:34Z";
    "iad-conversion-date" = "2021-03-30T02:54:34Z";
    "iad-conversion-type" = Download;
    "iad-country-or-region" = US;
    "iad-creativeset-id" = 1234567890;
    "iad-creativeset-name" = CreativeSetName;
    "iad-keyword" = Keyword;
    "iad-keyword-id" = 12323222;
    "iad-keyword-matchtype" = Broad;
    "iad-lineitem-id" = 1234567890;
    "iad-lineitem-name" = LineName;
    "iad-org-id" = 1234567890;
    "iad-org-name" = OrgName;
    "iad-purchase-date" = "2021-03-30T02:54:34Z";
}

Searching through the issue tracker for AdGroupName brings up these issues with log messages, in which you can see, that the capitalization is AdGroupName with an upper-case G, and not AdgroupName: #445, #475, #509

Fix

From my POV the fix is simply changing the capitalization, as done in this PR.

However, this begs the question: How did this ever work before?

As always, the Apple Developer docs around this are "thin" to say the least... I could imagine, that this actually used to be AdgroupName with a lower-case g and only got changed recently. The three issues I linked are all relatively new. So either this is really because it only got changed a few months ago, or there's now just a higher interest in all of this, e.g. due to ATT & iOS 14.5 or whatever...

In my testing thus far I haven't run into AdgroupName at all, but I've only debugged this bit on iOS 14.4.2 yet and currently have no access to devices with a lower iOS version. My iOS 13.7 simulator also uses AdGroupName.

So, if you think that we should not replace the capitalization, but check for both, I have an alternative PR here: #524

@@ -932,7 +932,7 @@ + (BOOL)checkAttributionDetails:(NSDictionary *)attributionDetails {
}
// Apple Search Ads fields
if ([ADJUtil contains:details key:@"iad-adgroup-id" value:@"1234567890"] &&
[ADJUtil contains:details key:@"iad-adgroup-name" value:@"AdgroupName"] &&
[ADJUtil contains:details key:@"iad-adgroup-name" value:@"AdGroupName"] &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative: #524 Checking for both capitalizations AdGroupName and AdgroupName.

@uerceg
Copy link
Contributor

uerceg commented Mar 30, 2021

HI @buschtoens

Thanks for the PR! Story behind this topic is that we have noticed that Apple has tweaked the dummy iAd.framework attribution payload in the past and with that made filtering attempts we added in the SDK to fail to catch new stuff. Fighting this on SDK side has pretty limited effect for one simple reason - people don't update SDKs as often and even if we release new version, clients might pick it up months after the release. Nevertheless, doesn't mean that trying to filter as much as possible on SDK level is pointless and with that in mind - thank you one more time for the PR, we will try to make it part of our next iOS SDK update.

How this is really fought against is on backend side where backend should filter this and don't do any attribution based on these values. I'm escalating this with our backend team to be checked and will get back to you as soon as I have some news on the topic.

One thing I'd need from you is your app token so that we can check server side logs. You can find my email in my GiHub profile page, so feel free to send it in there.

In case you have any questions in the meantime, feel free to ask.

Cheers

@uerceg
Copy link
Contributor

uerceg commented Aug 9, 2021

Replace with #560. Thanks one more time for the contribution!

@uerceg uerceg closed this Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants