Campaign BiddingStrategy MAXIMIZE_CONVERSIONS Not Detected In FieldMasks Utility On Mutate

PeterLavetsky commented 4 years ago

Hi All,

This bug report stems from a conversation on the API board : https://groups.google.com/forum/#!topic/adwords-api/GB4sReLzky4

If a Campaign has an existing bidding strategy, updating the Campaign to a bidding strategy of MAXIMIZE_CONVERSIONS does not persist through a mutate transaction. There are no partial failures and the mutate appears to go through fine. On query of the Campaign, it still has the old bidding strategy.

The problem appears to stem from the FieldMasks.allSetFieldsOf utility not detecting the automatic strategy / no parameter object that comes with MaximizeConversions.newBuilder().build()

final com.google.ads.googleads.v3.resources.Campaign.Builder googleCampaign = com.google.ads.googleads.v3.resources.Campaign.newBuilder();

case MAXIMIZE_CONVERSIONS:

googleCampaign.setBiddingStrategyType( BiddingStrategyTypeEnum.BiddingStrategyType.MAXIMIZE_CONVERSIONS );
googleCampaign.setMaximizeConversions( MaximizeConversions.newBuilder().build() );
break;

CampaignOperation.newBuilder() .setUpdate( googleCampaign ) .setUpdateMask( FieldMasks.allSetFieldsOf( googleCampaign ) );

operations { update { ... resource_name: "customers/ABC/campaigns/XYC” ... bidding_strategy_type: MAXIMIZE_CONVERSIONS maximize_conversions { } ... } update_mask { paths: "resource_name" paths: "name" paths: "status" paths: "tracking_url_template" paths: "network_settings.target_google_search" paths: "network_settings.target_search_network" paths: "network_settings.target_content_network" paths: "network_settings.target_partner_search_network" paths: "geo_target_type_setting.positive_geo_target_type" paths: "geo_target_type_setting.negative_geo_target_type" paths: "campaign_budget" paths: "bidding_strategy_type" paths: "end_date" } }

Modifying the FieldMask code to something like this does persist the updated bidding strategy on the mutate transaction:

FieldMasks.allSetFieldsOf( campaign ) .toBuilder() .addPaths( "maximize_conversions" ) .build();

Thanks, Pete

Which version of the client library are you using? com.google.api-ads:google-ads:5.0.0

Which version of Java are you using? openjdk version "1.8.0_201" OpenJDK Runtime Environment (build 1.8.0_201-b09) OpenJDK 64-Bit Server VM (build 25.201-b09, mixed mode)

Which operating system (Linux, Windows, ...) and version? Linux amzn1.x86_64 x86_64 x86_64 x86_64 GNU/Linux

Expected result The mutated campaign will have a bidding strategy of MAXIMIZE_CONVERSIONS

Actual result The mutated campaign does not have its bidding strategy updated

jradcliff commented 4 years ago

Hi,

Thanks for pointing this out. It looks like FieldMasks doesn't handle this because the call to descriptor.getFields() here returns an empty collection. We'll take a look to see how we can handle the case of a MESSAGE type attribute where the message does not have any fields.

Thanks, Josh, Google Ads API Team

nwbirnie commented 4 years ago

Hey @jradcliff, did you get a chance to look into this one?

PeterLavetsky commented 3 years ago

Hi All,

Re-upping this because after moving to v6 of the Ads API it looks like we're not able to change a campaign to MAXIMIZE_CONVERSIONS bidding strategy.

Recall earlier that we had this workaround:

final FieldMask.Builder fieldMaskBuilder = FieldMasks.allSetFieldsOf( campaign ).toBuilder();

if( campaign.hasMaximizeConversions() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversions" ) ) {
    fieldMaskBuilder.addPaths( "maximize_conversions" );
}

operationBuilder
.setUpdate( campaign )
.setUpdateMask( fieldMaskBuilder.build() );

This worked on v5.

Now on v6 we get this error:

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=JB4URFpj2gC_MCU8PzsqGw,date=Fri, 13 Nov 2020 21:08:56 GMT,alt-svc=h3-Q050=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
}
partial_failure_error {
  code: 3
  message: "The field mask updated a field with subfields: \'maximize_conversions\'., at operations[0]"
  details {
    type_url: "type.googleapis.com/google.ads.googleads.v6.errors.GoogleAdsFailure"
    value: "\n^\n\002@\003\022FThe field mask updated a field with subfields: \'maximize_conversions\'.\"\020\022\016\n\noperations\030\000"
  }
}

If we remove the workaround code and just attempt to let the FieldMask utility do its job we get back to square one of this bug, where the MAXIMIZE_CONVERSIONS is sent through but the update_mask does not detect it and the bidding scheme does not change.

Snippet, with the exact same campaign as above but without the fieldMaskBuilder massaging from above:

bidding_strategy_type: MAXIMIZE_CONVERSIONS
maximize_conversions {
}

}
update_mask {
  paths: "resource_name"
  paths: "name"
  paths: "status"
  paths: "url_custom_parameters"
  paths: "network_settings.target_google_search"
  paths: "network_settings.target_search_network"
  paths: "network_settings.target_content_network"
  paths: "network_settings.target_partner_search_network"
  paths: "geo_target_type_setting.positive_geo_target_type"
  paths: "geo_target_type_setting.negative_geo_target_type"
  paths: "campaign_budget"
  paths: "bidding_strategy_type"
  paths: "end_date"
  paths: "final_url_suffix"
}

I'm not sure if this release note factors in: https://developers.google.com/google-ads/api/docs/release-notes#bidding-v6

Do we need to change the path value that we're manually massaging to something other than maximize_conversions?

Please advise, as again, we're unable to change the bidding scheme to MAXIMIZE_CONVERSIONS with the code we have.

Thanks Pete

jradcliff commented 3 years ago

Hi Pete,

I'm working with our bidding team to figure out how to achieve this in v6. I tried various approaches today but was unsuccessful.

I'll provide an update as soon as I have more info.

Thanks, Josh

jradcliff commented 3 years ago

Hi Pete,

The bidding team is still investigating, but one workaround I found was to create a portfolio (shared) bidding strategy of type MAXIMIZE_CONVERSIONS, then submit an update that sets the campaign's bidding_strategy to the resource name of the portfolio strategy:

operations {
  update {
    resource_name: "customers/<CUSTOMER_ID>/campaigns/<CAMPAIGN_ID>"
    bidding_strategy: "customers/<CUSTOMER_ID>/biddingStrategies/<STRATEGY_ID>"
  }
  update_mask {
    paths: "bidding_strategy"
  }
}

I realize this is not an ideal workaround since it will require creating the portfolio bidding strategy in a separate step, but I thought it might be useful while you're waiting on a fix for this issue.

Thanks, Josh

jradcliff commented 3 years ago

Hi Pete,

Good news! The fix for v6 is now live, so you will be able to switch to MAXIMIZE_CONVERSIONS by specifying an update_mask of maximize_conversions, the same as with v5. Please give it a try and let me know if it gives you any trouble.

Thanks, Josh

PeterLavetsky commented 3 years ago

Thanks for the follow up Josh ... confirmed it's working on our end!

jradcliff commented 3 years ago

Great, thanks for confirming. Appreciate your patience as we got this one resolved.

PeterLavetsky commented 3 years ago

@jradcliff it looks like this behavior has again changed behind the scenes within the last couple of hours

With no deploys in the meantime we've started seeing this error again when mutating campaigns with MAXIMIZE_CONVERSIONS:

  message: "The field mask updated a field with subfields: \'maximize_conversions\'., at operations[0]"

Request
-------
MethodName: google.ads.googleads.v6.services.CampaignService/MutateCampaigns
Endpoint: googleads.googleapis.com:443
Headers: {developer-token=REDACTED, login-customer-id=5741853629, x-goog-api-client=gl-java/1.8.0_161 gapic/ gax/1.60.1 grpc/1.32.2}
Body: customer_id: "8477653270"
operations {
  update {
    resource_name: "customers/8477653270/campaigns/8719144693"
    status: ENABLED
    bidding_strategy_type: MAXIMIZE_CONVERSIONS
    maximize_conversions {
    }
  }
  update_mask {
    paths: "resource_name"
    paths: "name"
    paths: "status"
    paths: "tracking_url_template"
    paths: "url_custom_parameters"
    paths: "network_settings.target_google_search"
    paths: "network_settings.target_search_network"
    paths: "network_settings.target_content_network"
    paths: "network_settings.target_partner_search_network"
    paths: "targeting_setting.target_restrictions"
    paths: "geo_target_type_setting.positive_geo_target_type"
    paths: "geo_target_type_setting.negative_geo_target_type"
    paths: "campaign_budget"
    paths: "bidding_strategy_type"
    paths: "end_date"
    paths: "final_url_suffix"
    paths: "maximize_conversions"
  }
}
partial_failure: true

I've removed some of the irrelevant attributes

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=pI8O-UxwKqKhHj7klaNcEw,date=Wed, 17 Mar 2021 01:23:27 GMT,alt-svc=h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
}
partial_failure_error {
  code: 3
  message: "The field mask updated a field with subfields: \'maximize_conversions\'., at operations[0]"
  details {
    type_url: "type.googleapis.com/google.ads.googleads.v6.errors.GoogleAdsFailure"
    value: "\n^\n\002@\003\022FThe field mask updated a field with subfields: \'maximize_conversions\'.\"\020\022\016\n\noperations\030\000"
  }
}

Failure message: null
Status: Status{code=OK, description=null, cause=null}.
2021-03-16 21:23:28,964 DEBUG RequestLogger - SUCCESS REQUEST DETAIL.

This behavior looks to have changed around 5PM EST 2021-03-09

We're on com.google.api-ads:google-ads:11.0.0

Pete

nwbirnie commented 3 years ago

Looks like we should have a regression test in place here to stop this recurring.

@jradcliff do you have cycles to drive this? I can take a look if not.

chayamor commented 3 years ago

when we are remove the mask field paths (as a workaround) - or send maximize_conversions.target_cpa we are not enabled to update campaign from Target cpa for example to maximize conversions,

Upload

Request MethodName: google.ads.googleads.v6.services.GoogleAdsService/Mutate Endpoint: googleads.googleapis.com:443 Headers: {developer-token=REDACTED, login-customer-id=5672576074, x-goog-api-client=gl-java/11.0.4 gapic/ gax/1.57.0 grpc/} Body: customer_id: "4222912717" mutate_operations { campaign_operation { update { resource_name: "customers/4222912717/campaigns/12563908925" status: ENABLED network_settings { target_google_search: true target_search_network: false target_content_network: false } maximize_conversions { } name: "camp20.0388298286882262641615989733957" campaign_budget: "customers/4222912717/campaignBudgets/8363372096" } update_mask { paths: "resource_name" paths: "name" paths: "status" paths: "network_settings.target_google_search" paths: "network_settings.target_search_network" paths: "network_settings.target_content_network" paths: "campaign_budget" paths: "tracking_url_template" paths: "final_url_suffix" paths: "url_custom_parameters" } } } partial_failure: true

Response

Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=qgA7ml9sdQrPiJOcqg9ZbA,date=Wed, 17 Mar 2021 14:02:31 GMT,alt-svc=h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43") Body: mutate_operation_responses { campaign_result { resource_name: "customers/4222912717/campaigns/12563908925" } }

Download query: "SELECT campaign.id, campaign.bidding_strategy_type, campaign.bidding_strategy, campaign.maximize_conversions.target_cpa, campaign.target_cpa.cpc_bid_ceiling_micros, campaign.target_cpa.target_cpa_micros, campaign.target_cpa.cpc_bid_floor_micros, campaign.target_spend.cpc_bid_ceiling_micros, campaign.maximize_conversion_value.target_roas, campaign.manual_cpc.enhanced_cpc_enabled, campaign.target_impression_share.location, campaign.target_impression_share.location_fraction_micros, campaign.target_impression_share.cpc_bid_ceiling_micros, campaign.targeting_setting.target_restrictions, campaign.tracking_url_template, campaign.final_url_suffix, campaign.url_custom_parameters FROM campaign WHERE campaign.id IN (12563908925, 12563908925)" page_size: 1000

Response

Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=ggE0O6c7bhk5tNQSasCEjQ,date=Wed, 17 Mar 2021 14:08:05 GMT,alt-svc=h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43") Body: results { campaign { resource_name: "customers/4222912717/campaigns/12563908925" bidding_strategy_type: TARGET_CPA target_cpa { target_cpa_micros: 5000000 } id: 12563908925 } }

----------Or another example when adding to the paths

Upload

Request

MethodName: google.ads.googleads.v6.services.GoogleAdsService/Mutate Endpoint: googleads.googleapis.com:443 Headers: {developer-token=REDACTED, login-customer-id=5672576074, x-goog-api-client=gl-java/11.0.4 gapic/ gax/1.57.0 grpc/} Body: customer_id: "4222912717" mutate_operations { campaign_operation { update { resource_name: "customers/4222912717/campaigns/12563479903" status: ENABLED network_settings { target_google_search: true target_search_network: false target_content_network: false } maximize_conversions { } name: "camp20.66590550303884191615988965202" campaign_budget: "customers/4222912717/campaignBudgets/8363327915" } update_mask { paths: "resource_name" paths: "name" paths: "status" paths: "network_settings.target_google_search" paths: "network_settings.target_search_network" paths: "network_settings.target_content_network" paths: "campaign_budget" paths: "tracking_url_template" paths: "final_url_suffix" paths: "url_custom_parameters" paths: "maximize_conversions.target_cpa" } } } partial_failure: true

Response

Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=Tj4JpzBns3dH4Owp1zpGPg,date=Wed, 17 Mar 2021 13:49:44 GMT,alt-svc=h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43") Body: mutate_operation_responses { campaign_result { resource_name: "customers/4222912717/campaigns/12563479903" } }

Download: Request

MethodName: google.ads.googleads.v6.services.GoogleAdsService/Search Endpoint: googleads.googleapis.com:443 Headers: {developer-token=REDACTED, login-customer-id=5672576074, x-goog-api-client=gl-java/11.0.4 gapic/ gax/1.57.0 grpc/} Body: customer_id: "4222912717" query: "SELECT campaign.id, campaign.bidding_strategy_type, campaign.bidding_strategy, campaign.maximize_conversions.target_cpa, campaign.target_cpa.cpc_bid_ceiling_micros, campaign.target_cpa.target_cpa_micros, campaign.target_cpa.cpc_bid_floor_micros, campaign.target_spend.cpc_bid_ceiling_micros, campaign.maximize_conversion_value.target_roas, campaign.manual_cpc.enhanced_cpc_enabled, campaign.target_impression_share.location, campaign.target_impression_share.location_fraction_micros, campaign.target_impression_share.cpc_bid_ceiling_micros, campaign.targeting_setting.target_restrictions, campaign.tracking_url_template, campaign.final_url_suffix, campaign.url_custom_parameters FROM campaign WHERE campaign.id IN (12563479903, 12563479903)" page_size: 1000

Response

Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=DY9sBE2p2CmXWLwvs6N0Gg,date=Wed, 17 Mar 2021 13:50:10 GMT,alt-svc=h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43") Body: results { campaign { resource_name: "customers/4222912717/campaigns/12563479903" bidding_strategy_type: TARGET_CPA target_cpa { target_cpa_micros: 5000000 } id: 12563479903 } }

jradcliff commented 3 years ago

I'm investigating and following up to see what changed. Thanks for letting us know this resurfaced.

-Josh

jradcliff commented 3 years ago

We deployed a fix for this and I've confirmed that this is now working again as expected. Specifically, to switch to a MAXIMIZE_CONVERSIONS bidding strategy, please use a field mask of maximize_conversions, the same as with the previous fix.

Thanks, Josh

PeterLavetsky commented 3 years ago

Thanks @jradcliff ... confirmed on our end

Pete

PeterLavetsky commented 2 years ago

I believe this bug has resurfaced. Was there recently any changes to backend google code?

request-id=xHj8gfVBxoMT7nd4OWoWiA,date=Thu, 30 Sep 2021 15:09:41 GMT

Request
-------
MethodName: google.ads.googleads.v8.services.CampaignService/MutateCampaigns
Endpoint: googleads.googleapis.com:443
Headers: {developer-token=REDACTED, login-customer-id=ABC, x-goog-api-client=gl-java/1.8.0_161 gapic/ gax/2.5.0 grpc/1.40.1}
Body: customer_id: "ABC"
operations {
  update {
    resource_name: "customers/ABC/campaigns/14430520444"
    bidding_strategy_type: MAXIMIZE_CONVERSIONS
    maximize_conversions {
    }

      update_mask {
    paths: "resource_name"
    paths: "bidding_strategy_type"
  }

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=xHj8gfVBxoMT7nd4OWoWiA,date=Thu, 30 Sep 2021 15:09:41 GMT,alt-svc=h3=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
  resource_name: "customers/ABC/campaigns/14430520444"
}

Executing a SearchStream for the campaign after the mutate returns:

    bidding_strategy_type: TARGET_SPEND
    target_spend {
      cpc_bid_ceiling_micros: 5000000
    }

Thanks Pete

nwbirnie commented 2 years ago

I think the update mask is incorrect here, it should include maximize_conversions. The bidding_strategy_type is a read-only field so it will be ignored on update.

Are you using the FieldMasks utility here? If so which method are you calling?

PeterLavetsky commented 2 years ago

final FieldMask.Builder fieldMaskBuilder = FieldMasks.allSetFieldsOf( campaign ).toBuilder();

The following code is commented out because we've had to workaround this issue before, but this has been working since the June release of v15.0.0 I believe:

/*
                if( campaign.hasMaximizeConversions() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversions" ) ) {
                    fieldMaskBuilder.addPaths( "maximize_conversions" );
                }
                 */

Our code worked yesterday, or at the very least within the last week. Today it doesn't work.

I can uncomment the item above and I'd presume it'll work, but a permanent fix would be desirable.

Pete

nwbirnie commented 2 years ago

It looks like there's probably a bug in the FieldMasks implementation. I'll work on a patch just now, should be fixed in a day or 2.

PeterLavetsky commented 2 years ago

@nwbirnie @jradcliff When I uncommented the following code:

if( campaign.hasMaximizeConversions() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversions" ) ) {
                    fieldMaskBuilder.addPaths( "maximize_conversions" );
                }

this error which was also previously discussed in this thread showed up again:

    bidding_strategy_type: MAXIMIZE_CONVERSIONS
    maximize_conversions {
    }

   update_mask {
    paths: "bidding_strategy_type"
    paths: "maximize_conversions"
  }

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=keDPnL40StqsJlgtRT5nDA,date=Thu, 30 Sep 2021 17:20:18 GMT,alt-svc=h3=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
}
partial_failure_error {
  code: 3
  message: "The field mask updated a field with subfields: \'maximize_conversions\'., at operations[0]"
  details {
    type_url: "type.googleapis.com/google.ads.googleads.v8.errors.GoogleAdsFailure"
    value: "\n^\n\002@\003\022FThe field mask updated a field with subfields: \'maximize_conversions\'.\"\020\022\016\n\noperations\030\000"
  }
}

https://github.com/googleads/google-ads-java/issues/272#issuecomment-801024456

So we're now back to where we were in March with no ability / workaround to change a Campaign to MAXIMIZE_CONVERSIONS

Pete

jradcliff commented 2 years ago

Hi Pete,

Did you recently switch from using v7 of the API to using v8?

In v7, the target_cpa field was read-only, so it should be excluded from the field mask: maximize_conversions.
In v8 it is mutable, so it should be included in the field mask: maximize_conversions.target_cpa.

I just verified this by switching a campaign from manual CPC to maximize conversions using both versions.

That being said, FieldMasks.allSetFieldsOf will only return the correct v8 field mask of maximize_conversions.target_cpa if target_cpa is set to a non-zero value, so there may still be an issue with that utility as @nwbirnie mentioned.

-Josh

PeterLavetsky commented 2 years ago

@jradcliff We've been on v8 since June-ish ... about a week after the library was made available

In this instance there is no non-zero value for Target CPA so that's likely the culprit that I did not double check

EDIT: Double checking on my side ... we hadn't been setting TargetCpa at all on MaxConversions so this is a recent breakage whether there was a > 0 TargetCpa value or not

nwbirnie commented 2 years ago

Can you try sending maximize_conversions.target_cpa as a workaround? I can also update the FieldMasks util to detect this case.

PeterLavetsky commented 2 years ago

operations {
  update {
    resource_name: "customers/ABC/campaigns/12657518522"
    bidding_strategy_type: MAXIMIZE_CONVERSIONS
    maximize_conversions {
    }
  }
}

update_mask {
  paths: "resource_name"
  paths: "bidding_strategy_type"
  paths: "maximize_conversions"
  paths: "maximize_conversions.target_cpa"
}

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=qsV_AocCZVTCukkXSlNaIg,date=Thu, 30 Sep 2021 18:54:20 GMT,alt-svc=h3=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
}
partial_failure_error {
  code: 3
  message: "The field mask updated a field with subfields: \'maximize_conversions\'., at operations[0]"
  details {
    type_url: "type.googleapis.com/google.ads.googleads.v8.errors.GoogleAdsFailure"
    value: "\n^\n\002@\003\022FThe field mask updated a field with subfields: \'maximize_conversions\'.\"\020\022\016\n\noperations\030\000"
  }
}

nwbirnie commented 2 years ago

Sorry I mean to remove maximize_conversions:

update_mask {
  paths: "resource_name"
  paths: "bidding_strategy_type"
  paths: "maximize_conversions.target_cpa"
}

PeterLavetsky commented 2 years ago

update_mask {
    paths: "resource_name"
    paths: "bidding_strategy_type"
    paths: "maximize_conversions.target_cpa"
 }

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=fWOre2LKHtAjNGpiPOu8tw,date=Fri, 01 Oct 2021 17:03:47 GMT,alt-svc=h3=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
  resource_name: "customers/ABC/campaigns/12665607318"
}

This mutate succeeds, however I don't know if it actually changed the bid strategy type as our users have reverted their changes back away from MAXIMIZE_CONVERSIONS because of the Google bug. I was only able to test on campaigns that already had MAXIMIZE_CONVERSIONS set at Google.

jradcliff commented 2 years ago

Hi Pete,

I had a test case handy so I tried this on my account. First, I confirmed the campaign was on MANUAL_CPC:

query: "SELECT campaign.id, campaign.bidding_strategy_type,  campaign.bidding_strategy,  
  campaign.maximize_conversions.target_cpa, campaign.manual_cpc.enhanced_cpc_enabled 
FROM campaign
WHERE campaign.id = <my_campaign_id>"

Response:

...
    bidding_strategy_type: MANUAL_CPC
    manual_cpc {
      enhanced_cpc_enabled: false
    }

Updated using v8 and maximize_conversions.target_cpa:

  update {
    resource_name: "customers/.../campaigns/12131942085"
    maximize_conversions {
      target_cpa: 1500000
    }
  }
  update_mask {
    paths: "maximize_conversions.target_cpa"
  }

Confirmed the campaign was switched to MAXIMIZE_CONVERSIONS:

  campaign {
    resource_name: "customers/.../campaigns/12131942085"
    bidding_strategy_type: MAXIMIZE_CONVERSIONS
    maximize_conversions {
      target_cpa: 1500000
    }
    id: 12131942085
  }

-Josh

PeterLavetsky commented 2 years ago

Sweet. Thank you!

PeterLavetsky commented 2 years ago

@jradcliff we just migrated to V9 / com.google.api-ads:google-ads:16.0.0 and I had to add the following in order to change a Campaign Bidding Strategy to MaximizeConversionValue:

if( campaign.hasMaximizeConversionValue() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversion_value.target_roas" ) ) {
  fieldMaskBuilder.addPaths( "maximize_conversion_value.target_roas" );
}

Pete

jradcliff commented 2 years ago

Hi @PeterLavetsky ,

That's the same behavior I'm seeing for v8 and MaximizeConversionValue. Since target_roas is not marked optional, the field masks utility cannot differentiate between you setting the value to 0 or not setting it at all. Are you saying that you had to make this change when migrating from v8 to v9, but in v8 changing the strategy to MaximizeConversionValue worked without that change?

Thanks, Josh

PeterLavetsky commented 2 years ago

Hi Josh,

Within 24 hours of us moving to V9, MANUAL_CPC -> MAXIMIZE_CONVERSION_VALUE was brought to my attention as "not working".

I'm really not sure if it worked in V8, but we were on V8 for ~3 months without the above code and no one ever said moving to MCV didn't work :-)

Pete

jradcliff commented 2 years ago

Interesting...I strongly suspect that this was the behavior in v8 and perhaps no one noticed + brought it to your attention. The only change made to the MaximizeConversionValue message in v9 was to add two more non-optional fields, which I would not expect to make a difference in this scenario.

Thanks, Josh

devchas commented 2 years ago

I suspect this resulted from the changes we made to the FieldMasks utility in v8_1.

devchas commented 2 years ago

Hi @PeterLavetsky are you still experiencing this issue? I just tried reproducing your case, and this seems to be working properly for me using v9. Would you please give this another try and let me know if it is working for you?

@jradcliff I am seeing a separate issue with the FieldMasks utility that occurs when you try to update a field with all optional sub-fields and do not specify any sub-fields. In those cases, the FieldMask utility doesn't pick up on the change. For example, if you set campaign_bidding_strategy to manual_cpc but do not set a value for enhanced_cpc_enabled, manual_cpc is not added to the paths list. Does that sounds like a bug to you?

In a variation of this case, in which the field has no sub-fields (for example with manual_cpm), this WAI.

PeterLavetsky commented 2 years ago

@devchas the issues persists, in one way or another.

I'll lay everything out in an effort to create a reproducible environment:

case MAXIMIZE_CONVERSION_VALUE:

                googleCampaign.setBiddingStrategyType( BiddingStrategyTypeEnum.BiddingStrategyType.MAXIMIZE_CONVERSION_VALUE );
                googleCampaign.setMaximizeConversionValue( getMaximizeConversionValueBiddingScheme( plan ) );
                break;

getMaximizeConversionValueBiddingScheme is simply:

return MaximizeConversionValue.newBuilder().build();

TargetRoas double value is not marked required:

https://developers.google.com/google-ads/api/fields/v9/bidding_strategy#bidding_strategy.maximize_conversion_value.target_roas

Here is the workaround we have in production code to enable us to switch from anything to MAXIMIZE_CONVERSION_VALUE:

if( campaign.hasMaximizeConversions() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversions.target_cpa" ) ) {
    fieldMaskBuilder.addPaths( "maximize_conversions.target_cpa" );
}

if( campaign.hasMaximizeConversionValue() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversion_value.target_roas" ) ) {
    fieldMaskBuilder.addPaths( "maximize_conversion_value.target_roas" );
}

When I remove that code, an attempt to change a Campaign bidding strategy from MANUAL_CPC to MAXIMIZE_CONVERSION_VALUE does not fail, but does not change the bidding strategy:

Request
-------
MethodName: google.ads.googleads.v9.services.CampaignService/MutateCampaigns
Endpoint: googleads.googleapis.com:443
Headers: {developer-token=REDACTED, login-customer-id=6026212262, x-goog-api-client=gl-java/11.0.13 gccl/task ':google-ads:jar' property 'archiveVersion' gapic/task ':google-ads:jar' property 'archiveVersion' gax/2.6.1 grpc/task ':google-ads:jar' property 'archiveVersion'}
Body: customer_id: "9007664394"
operations {
  update {
    resource_name: "customers/9007664394/campaigns/13977442261"
    status: ENABLED
    url_custom_parameters {
      key: "fcycmp"
      value: "jma_test_campaign_1"
    }
    url_custom_parameters {
      key: "fcycmpid"
      value: "447768"
    }
    url_custom_parameters {
      key: "partnername"
      value: "google"
    }
    network_settings {
      target_google_search: true
      target_search_network: false
      target_content_network: false
      target_partner_search_network: false
    }
    bidding_strategy_type: MAXIMIZE_CONVERSION_VALUE
    maximize_conversion_value {
    }
    targeting_setting {
      target_restrictions {
        targeting_dimension: AUDIENCE
        bid_only: true
      }
    }
    geo_target_type_setting {
      positive_geo_target_type: PRESENCE_OR_INTEREST
      negative_geo_target_type: PRESENCE_OR_INTEREST
    }
    name: "JMA Test Campaign 1"
    tracking_url_template: "{lpurl}?xx=0"
    campaign_budget: "customers/9007664394/campaignBudgets/9152419912"
    end_date: "20371230"
  }
  update_mask {
    paths: "resource_name"
    paths: "name"
    paths: "status"
    paths: "tracking_url_template"
    paths: "url_custom_parameters"
    paths: "network_settings.target_google_search"
    paths: "network_settings.target_search_network"
    paths: "network_settings.target_content_network"
    paths: "network_settings.target_partner_search_network"
    paths: "targeting_setting.target_restrictions"
    paths: "geo_target_type_setting.positive_geo_target_type"
    paths: "geo_target_type_setting.negative_geo_target_type"
    paths: "campaign_budget"
    paths: "bidding_strategy_type"
    paths: "end_date"
  }
}
partial_failure: true

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=Z-xLjd4eI0lMEEJ96UvyEQ,date=Fri, 10 Dec 2021 13:58:04 GMT,alt-svc=h3=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
  resource_name: "customers/9007664394/campaigns/13977442261"
}

Failure message: null
Status: Status{code=OK, description=null, cause=null}.
2021-12-10 08:58:05,405 DEBUG RequestLogger - SUCCESS REQUEST DETAIL.

The same fingerprint is present when attempting to change from anything to MAXIMIZE_CONVERSIONS when a target_cpa is not in the field mask:

case MAXIMIZE_CONVERSIONS:

                googleCampaign.setBiddingStrategyType( BiddingStrategyTypeEnum.BiddingStrategyType.MAXIMIZE_CONVERSIONS );
                googleCampaign.setMaximizeConversions( getMaximizeConversionsBiddingScheme( plan ) );
                break;

In the above code, setTargetCpa is only called if the value is greater than 0, otherwise the returned object is simply

return MaximizeConversions.newBuilder().build();

Without the workaround code:

Request
-------
MethodName: google.ads.googleads.v9.services.CampaignService/MutateCampaigns
Endpoint: googleads.googleapis.com:443
Headers: {developer-token=REDACTED, login-customer-id=6026212262, x-goog-api-client=gl-java/11.0.13 gccl/task ':google-ads:jar' property 'archiveVersion' gapic/task ':google-ads:jar' property 'archiveVersion' gax/2.6.1 grpc/task ':google-ads:jar' property 'archiveVersion'}
Body: customer_id: "9007664394"
operations {
  update {
    resource_name: "customers/9007664394/campaigns/13977442261"
    status: ENABLED
    url_custom_parameters {
      key: "fcycmp"
      value: "jma_test_campaign_1"
    }
    url_custom_parameters {
      key: "fcycmpid"
      value: "447768"
    }
    url_custom_parameters {
      key: "partnername"
      value: "google"
    }
    network_settings {
      target_google_search: true
      target_search_network: false
      target_content_network: false
      target_partner_search_network: false
    }
    bidding_strategy_type: MAXIMIZE_CONVERSIONS
    maximize_conversions {
    }
    targeting_setting {
      target_restrictions {
        targeting_dimension: AUDIENCE
        bid_only: true
      }
    }
    geo_target_type_setting {
      positive_geo_target_type: PRESENCE_OR_INTEREST
      negative_geo_target_type: PRESENCE_OR_INTEREST
    }
    name: "JMA Test Campaign 1"
    tracking_url_template: "{lpurl}?xx=0"
    campaign_budget: "customers/9007664394/campaignBudgets/9152419912"
    end_date: "20371230"
  }
  update_mask {
    paths: "resource_name"
    paths: "name"
    paths: "status"
    paths: "tracking_url_template"
    paths: "url_custom_parameters"
    paths: "network_settings.target_google_search"
    paths: "network_settings.target_search_network"
    paths: "network_settings.target_content_network"
    paths: "network_settings.target_partner_search_network"
    paths: "targeting_setting.target_restrictions"
    paths: "geo_target_type_setting.positive_geo_target_type"
    paths: "geo_target_type_setting.negative_geo_target_type"
    paths: "campaign_budget"
    paths: "bidding_strategy_type"
    paths: "end_date"
  }
}
partial_failure: true

Response
--------
Headers: Metadata(content-disposition=attachment,content-type=application/grpc,request-id=uZQ1Nooqvo5-MOpojzRx5g,date=Fri, 10 Dec 2021 14:21:16 GMT,alt-svc=h3=":443"; ma=2592000,h3-29=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43")
Body: results {
  resource_name: "customers/9007664394/campaigns/13977442261"
}

Failure message: null
Status: Status{code=OK, description=null, cause=null}.
2021-12-10 09:21:18,237 DEBUG RequestLogger - SUCCESS REQUEST DETAIL.

Again, the bidding strategy does not change.

Adding back the following workaround allows bidding strategies to change:

if( campaign.hasMaximizeConversions() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversions.target_cpa" ) ) {
    fieldMaskBuilder.addPaths( "maximize_conversions.target_cpa" );
}

if( campaign.hasMaximizeConversionValue() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversion_value.target_roas" ) ) {
    fieldMaskBuilder.addPaths( "maximize_conversion_value.target_roas" );
}

com.google.api-ads:google-ads:16.0.0

Hope that helps.

Pete

devchas commented 2 years ago

Thanks @PeterLavetsky - this is what I suspected was the case. I just wanted to confirm you were trying to update the bidding strategy without setting any of the subfields. In one of the examples above, you were setting target_roas on maximize_conversion_value, which would work.

The first thing that I should point out to avoid any confusion is that the bidding_strategy_type field is read-only, so including it in the update operation has no effect.

As for the issue at hand, this is more of an issue with the core API than the FieldMasks utility in Java. In the Google Ads API, you cannot include an field that has subfields in an UpdateMask, regardless of whether or not those subfields are required. In other words, in order to include a field of type MESSAGE in the field mask, the message must either have no subfields, such is the case with a field like manual_cpm or at least one of the fields on that message must have a value set. If you do try to perform an operation as described above, you will receive a FieldMaskError. FIELD_HAS_SUBFIELDS error from the server. The FieldMasks utility in this library simply matches that behavior. If, for example, you manually added maximize_conversion_value to the UpdateMask instead of maximize_conversion_value.target_roas, you would receive that error.

The reason that this behavior is in place is to prevent users from incidentally clearing fields. This is a conservative behavior, which is intended to protect users, despite the fact that it is not the most intuitive or user-friendly experience.

For now, the workaround that you've found is the best available solution, and the FieldMasks utility in this client library is working as intended. However, we are exploring solutions to this problem that will hopefully make this more user friendly without sacrificing the protection afforded by the current approach.

Seeing as this is WAI, I am going to close out this issue, but feel free to open it back up if you have follow-up questions or need clarification. If we do implement a change to the core API's behavior, we'll make sure the FieldMasks utility reflects that change.

PeterLavetsky commented 2 years ago

Hi @devchas and @jradcliff

We've migrated to V11 / com.google.api-ads:google-ads:19.0.0 and wanted to relay that we've again had to update the way we work with some certain field masks

Stemming from Devin's comment above:

In other words, in order to include a field of type MESSAGE in the field mask, the message must either have no subfields, such is the case with a field like [manual_cpm](https://developers.google.com/google-ads/api/reference/rpc/latest/Campaign) or at least one of the fields on that message must have a value set.

On google-ads:18.0.0 and before we could perform the following operation:

operations {
  update {
    resource_name: "customers/2207819770/biddingStrategies/8798869785"
    type: MAXIMIZE_CONVERSIONS
    id: 8798869785
    name: "Dealer-Brand Max Conversion Portfolio Strategy"
    maximize_conversions {
    }
  }
  update_mask {
    paths: "resource_name"
    paths: "id"
    paths: "name"
    paths: "type"
    paths: "maximize_conversions"
    paths: "maximize_conversions.target_cpa"
  }
}

I'm ignoring the move from target_cpa -> target_cpa_micros as that's not part of this conversation.

The key part here is that maximize_conversions and maximize_conversions.target_cpa were both part of the field mask and the mutate succeeded.

With no internal code changes, but hitting V11 services, the above mutate now fails with:

partial_failure_error {
  code: 3
  message: "The field mask updated a field with subfields: \'maximize_conversions\'., at operations[0]"
  details {
    type_url: "type.googleapis.com/google.ads.googleads.v11.errors.GoogleAdsFailure"
    value: "\n^\n\002@\003\022FThe field mask updated a field with subfields: \'maximize_conversions\'.\"\020\022\016\n\noperations\030\000"
  }
}

So I think the V11 behavior now accurately aligns with what Devin was conveying back in December.

We now have to jump through a few more hoops and do work like the following:

if( campaign.hasMaximizeConversions() && !fieldMaskBuilder.getPathsList().contains( "maximize_conversions.target_cpa_micros" ) ) {
    internalPaths.add( "maximize_conversions.target_cpa_micros" );
    internalPaths.remove( "maximize_conversions" );
}

Then iterate over internalPaths and reconstruct the FieldMask with those values.

We're past it now but my discomfort does grow each time I have to accommodate different logic / behavior in such a core area such as FieldMask / Operation.

A quick recap of this specific scenario. We're using the FieldMasks utility like so:

final FieldMask.Builder fieldMaskBuilder = FieldMasks.allSetFieldsOf( campaign ).toBuilder();

This campaign has a MaximizeConversions bidding strategy that is constructed like:

MaximizeConversions.newBuilder().build()

This bidding strategy does not have a target_cpa_micros value set ( and it's not required )

By adding maximize_conversions.target_cpa_micros to the field_mask, with no value set, and removing maximize_conversions from the field mask ( new behavior ) we can push this through.

This behavior does seem to differ from Devin's statement of:

or at least one of the fields on that message must have a value set.

... as neither maximize_conversions nor maximize_conversions.target_cpa_micros has a value set.

I've had to make similar brand new code adjustments for V11 to BiddingStrategy mutates for maximize_conversions and target_spend

If there's updated guidance for V11 to construct our field_masks please let us know ... we're always willing to turn some dials with you to get to the simplest solution.

Thanks Pete

devchas commented 2 years ago

Hey @PeterLavetsky thanks for flagging this, and apologies for the moving target on this one. The FieldMask utils can be a bit tricky, but I'm hopeful we'll keep the logic in the most recent release.

The TLDR fix is: remove the maximize_conversions field on your campaign object and add maximize_conversions.target_cpa_micros to your field mask manually. Given the complexity of this topic, we also released this guide to help explain how to handle this.

Now for a bit of context so that you can understand why you're running into this error now. In previous versions of the client library, if the FieldMasks util encountered an empty message, like the empty maximize_conversions on your campaign, it would throw it away entirely. This was, in some respect, a bug because the request would succeed, but the associated operation wouldn't be performed. This was, in our opinion, worse than returning the error you're receiving because it was effectively failing silently.

In this release, if there is an empty message on the object, we'll now just add the path of that message field to the field mask. However, the Google Ads server will reject an empty message in the field mask if that given field has subfields defined on that object - no longer failing silently. To get around this, you need to exclude that field from the object you are updating and only add selected fields to the field mask manually.

I hope this helps! Please feel free to reach out with any other questions.

googleads / google-ads-java