googleads / google-ads-php

Google Ads API Client Library for PHP
https://developers.google.com/google-ads/api/docs/client-libs/php
Apache License 2.0
280 stars 260 forks source link

Missing request Id when we upload offline conversions #1005

Closed rilwanfit closed 2 months ago

rilwanfit commented 4 months ago

Your question: We are currently migrating our project from Google Ads API V15 to V16 with the release of Google Ads PHP 22.1.0.

In our application, we used to log the RequestID whenever we sent offline conversions to Google Ads API. (both success calls as well as failed calls)

We used the following code when we used V15.

use Google\Ads\GoogleAds\V15\Services\ConversionUploadServiceClient;

 /** @var UploadClickConversionsResponse $response */
[$response,$status] = $conversionUploadService->uploadClickConversions( 
    $customerId,
    $conversions,
    true,
    ['withResponseMetadata' => true],
);

The $status is the GoogleAdsResponseMetadata where we can get the request-id.

Problem

With V16 the above code is NOT working anymore. So we have updated the code to the following

use Google\Ads\GoogleAds\V16\Services\Client\ConversionUploadServiceClient;

$response = $conversionUploadService->uploadClickConversions(
        UploadClickConversionsRequest::build($customerId, $conversions, true),
 );

With this code, we missed the entire GoogleAdsResponseMetadata object.

How can we get the request Id with new version (V16)? (both success calls as well as failed calls)

fiboknacky commented 4 months ago

It may be related to the change of new generated code and google/gax. Let me ask the owners of the repository and get back here.

fiboknacky commented 4 months ago

BTW, what google/gax did you install in your project?

rilwanfit commented 4 months ago

We are using "googleads/google-ads-php": "^v22.1.0" and google/gax (^1.19.1) as an internal dependency

bshaffer commented 4 months ago

Without looking too deeply (I'm not at a laptop), the first example contains ['withResponseMetadata' => true] and the second does not. Have you tried removing this line?

bshaffer commented 4 months ago

Hmm, after a second thought, I think the issue is that the API has changed, and the second way you have is the new (and correct) way to make the API call, which would mean this is working as designed.

fiboknacky commented 4 months ago

I don't think so. The API specification itself hasn't changed.

We have GoogleAdsGapicTrait that has been used for a long time with the GAPIC v1 classes.

modifyUnaryCallable and modifyStreamingCallable are the methods heavily used to inject what we need such as GoogleAdsException and to return the metadata like shown above.

The GAPIC v1 classes can call these functions successfully, but somehow for GAPIC v2 classes (like GoogleAdsServiceClient), it looks like there is something changed underneath that makes the result of a callable not propagated back.

Has anything in gax-php changed that would affect how the middleware works for this specific case?

bshaffer commented 4 months ago

@fiboknacky If I could get a repro test going for this, we could roll back versions of GAX until we land on a version where it works - this would go a long way in helping diagnose this issue

fiboknacky commented 3 months ago

FYI @rilwanfit

Brent (@bshaffer) and I synced offline and we found the root cause. Brent has implemented a new solution (#1013), which was merged last week. I'll release a new client library version soon this week at earliest.

fiboknacky commented 2 months ago

New version of the client library was released.

dabontv commented 1 month ago

Is there an example to get metadata in the new update? It seems the api docs are still outdated https://developers.google.com/google-ads/api/docs/client-libs/php/response-metadata

fiboknacky commented 1 month ago

We've just updated the doc. Sorry for your inconvenience.

dabontv commented 1 month ago

Using v17 example:

        $conversionUploadServiceClient = $googleAdsClient->getConversionUploadServiceClient();
        $response = $conversionUploadServiceClient->uploadClickConversions(
            UploadClickConversionsRequest::build($customerId, $clickConversion, true),
            ['withResponseMetadata' => true]
        );

$conversionUploadServiceClient->getResponseMetadata(); will return null

fiboknacky commented 1 month ago

I cannot reproduce your issue. It works as expected on my machine. Could you share the whole code? And what's the result? Can you still get your log and request ID in there?