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

Per Google, GCLID, GBRAID and WBRAID can't be used at the same time. #2365

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

seg-leonelsanches
Copy link
Contributor

Per Google:

"Considering that for iOS, gclid is sent if your user has given consent on iOS, and gbraid if the user has not given consent, it makes no sense to send Advanced Conversions email data requesting consent if gbraid is sent at the same time which implies no consent."

Our integration allows a customer to send all the three parameters together, resulting in cryptic errors in the destination.

Testing

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [Segmenters] Tested in the staging environment

Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.57%. Comparing base (5a5f52b) to head (cdd0e0f).
Report is 114 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2365       +/-   ##
===========================================
+ Coverage   33.18%   78.57%   +45.38%     
===========================================
  Files          14     1027     +1013     
  Lines         693    18125    +17432     
  Branches      109     3413     +3304     
===========================================
+ Hits          230    14242    +14012     
- Misses        463     2748     +2285     
- Partials        0     1135     +1135     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

…conversions/uploadClickConversion/index.ts

Co-authored-by: Joe Ayoub <[email protected]>
@joe-ayoub-segment
Copy link
Contributor

hi @brennan - got a small PR for a change to 2 Actions in Google Enhanced Conversions, raised by @seg-leonelsanches from the SA team.

Could you triage please?

@brennan
Copy link
Contributor

brennan commented Oct 4, 2024

Hi @seg-leonelsanches, thanks for raising this PR. Please provide:

  • A link to Google's documentation supporting the change.
  • A link to Google's documentation related to the errors customers are seeing (including screenshots of the errors).
  • A description of all possible side effects on existing customers (as this change will result in Segment discarding customer events that we previous sent to Google's API).

@lukebussey
Copy link

@brennan This has been problematic when using the Google Ads destination.

You can only send one of gclid, gbraid, or wbraid. But Google will append one or more on the ad final URL.

https://developers.google.com/google-ads/api/docs/conversions/upload-clicks#set_the_required_fields_of_each_conversion_operation

Google's code example shows the following logic:

# Sets the single specified ID field.
    if gclid:
        click_conversion.gclid = gclid
    elif gbraid:
        click_conversion.gbraid = gbraid
    else:
        click_conversion.wbraid = wbraid

This is the error you get from the mapping event tester:

Screenshot 2024-10-05 at 12 25 14 PM

There's no impact in adding this change, customers events are already failing if more than one parameter is being sent.

In addition, this PR doesn't address another issue where if a gbraid or wbraid parameter are sent, Google will reject the payload if it contains an email address and/or phone number.

Error messages:
https://developers.google.com/google-ads/api/docs/conversions/upload-clicks#debug_common_errors

Error Message
ConversionUploadError.GBRAID_WBRAID_BOTH_SET The ClickConversion has a value set for both gbraid and wbraid. Update the conversion to use only one click ID, and make sure you are not combining multiple clicks into the same conversion. Each click has only one click ID.
FieldError.VALUE_MUST_BE_UNSET Check the location of the GoogleAdsError to determine which of the following issues led to the error.

The ClickConversion has a value set for gclid as well as at least one of gbraid or wbraid. Update the conversion to use only one click ID, and make sure you are not combining multiple clicks into the same conversion. Each click has only one click ID.

The ClickConversion has a value set for either gbraid or wbraid and has a value for custom_variables. Google Ads does not support custom variables for a conversion with a gbraid or wbraid click ID. Unset the custom_variables field of the conversion.

@seg-leonelsanches
Copy link
Contributor Author

Thank you, @brennan and @lukebussey, for your comments. Today I had the opportunity to meet with the Google team, validating all the information contained here, adding answers to this PR's raised questions:

A link to Google's documentation supporting the change.

Links:

    """Creates a click conversion with a default currency of USD.

    Args:
        ...
        gclid: The Google Click Identifier ID. If set, the wbraid and gbraid
            parameters must be None.
        ...
        gbraid: The GBRAID for the iOS app conversion. If set, the gclid and
            wbraid parameters must be None.
        wbraid: The WBRAID for the iOS app conversion. If set, the gclid and
            gbraid parameters must be None.
        ...
    """

A link to Google's documentation related to the errors customers are seeing (including screenshots of the errors).

That's again https://developers.google.com/google-ads/api/docs/conversions/upload-clicks#debug_common_errors. Below I'm adding the error screenshots:

image image

A description of all possible side effects on existing customers (as this change will result in Segment discarding customer events that we previous sent to Google's API).

The events will be discarded anyway. What changes for us is the opportunity to add better explanatory error messages, as well as updating our documentation to inform our customers of these validations.

@lukebussey, I'll implement the extra suggestion you gave, and then this PR can be re-evaluated by our team. I'll keep everybody posted here.

- Updating unit tests and fields documentation.
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.

4 participants