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

querying video_quartile_25_rate causes google.protobuf.internal.well_known_types.Error: Fail to print FieldMask to Json string #30

Closed
apurvis opened this issue Jan 31, 2019 · 10 comments
Assignees
Labels
bug Something isn't working P1 Breaking issue affecting some users or workflows

Comments

@apurvis
Copy link

apurvis commented Jan 31, 2019

This issue arose trying to upgrade to package 0.6/API 0.7. Trying to query metrics.video_quartile_25_rate gives google.protobuf.internal.well_known_types.Error: Fail to print FieldMask to Json string: The character after a "_" must be a lowercase letter in path name metrics.video_quartile_25_rate.

full stack trace:

File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/page_iterator.py", line 199, in _items_iter
   for page in self._page_iter(increment=False):
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/page_iterator.py", line 230, in _page_iter
   page = self._next_page()
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/page_iterator.py", line 512, in _next_page
   response = self._method(self._request)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/gapic_v1/method.py", line 139, in __call__
   return wrapped_func(*args, **kwargs)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/retry.py", line 260, in retry_wrapped_func
   on_error=on_error,
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/retry.py", line 177, in retry_target
   return target()
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/timeout.py", line 206, in func_with_timeout
   return func(*args, **kwargs)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/google/api_core/grpc_helpers.py", line 59, in error_remapped_callable
   return callable_(*args, **kwargs)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/grpc/_interceptor.py", line 201, in __call__
   credentials=credentials)
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/grpc/_interceptor.py", line 226, in _with_call
   return call.result(), call
 File "/Users/oogabooga/repos/airflow-workflows/.venv/lib/python2.7/site-packages/grpc/_interceptor.py", line 116, in result
   raise self._exception
google.protobuf.internal.well_known_types.Error: Fail to print FieldMask to Json string: The character after a "_" must be a lowercase letter in path name metrics.video_quartile_25_rate.
@BenRKarl
Copy link
Contributor

BenRKarl commented Feb 4, 2019

Hi @apurvis - thanks for bringing this issue up! It looks like a problem with how the message is serialized to JSON. I'm looking into it now and will respond here once I have more information, and if there's a necessary code change I'll post a link to the PR here. As a temporary workaround - since the error is originating from the logging interceptor - if you set your logging level to ERROR the logging process will be skipped and these errors won't be triggered.

@BenRKarl
Copy link
Contributor

BenRKarl commented Feb 8, 2019

@apurvis just an update on this - the issue is stemming from the protobuf library, specifically the ToJsonString method for FieldMasks. The method converts all strings in the FieldMask's paths list from snake case to camel case, but rejects digits that follow underscores (i.e. _2). See this line. I don't see why digits should prevent this conversion, so I've proposed a change to the protobuf team. Depending on how long that takes I may post a temporary fix to patch that method, but I would much prefer the change come from their library.

Apologies again for the inconvenience, but really do appreciate you pointing this out!

@rubberviscous
Copy link

@BenRKarl
Is the ERROR error level still the preferred workaround? If so, what's the proper way to set this up? I tried passing a log dictionary to GoogleAdsClient class. But I'm still getting the Fail to print FieldMask to Json string exception. I've included my implementation below, is this the correct approach?

log_config = {
    'version': 1,
    'formatters': {
        'standard': {
            'format': '%(asctime)s [%(levelname)s] %(name)s: %(message)s'
        },
    },
    'handlers': {
        'default': {
            'level': 'ERROR',
            'formatter': 'standard',
            'class': 'logging.StreamHandler',
            'stream': 'ext://sys.stdout',  # Default is stderr
        },
    },
    'loggers': {
        'default': {
            'level': 'ERROR',
            'handlers': ['default']
        }
    }
}
client = GoogleAdsClient(
            credentials=credentials,
            developer_token=developer_token,
            login_customer_id=login_customer_id,
            logging_config=log_config
        )

@BenRKarl
Copy link
Contributor

BenRKarl commented Apr 9, 2019

@rubberviscous what's the request you're making? If you're getting an API error then setting the level to ERROR may not be an effective workaround. If that's the case you may need to turn logging off entirely.

For background the protobuf utility we use here to convert protobuf message to JSON can not parse fields with a digit following an underscore, i.e. test_2_field. This line only fires when logs are generated. Setting to ERROR worked for @apurvis because his requests weren't returning API errors. Until I can post a fix to remove JSON serialization, I suggest disabling logging via the library and catching and logging the response directly in your code to diagnose the API error. Once you've stopped receiving API errors it should be safe to enabling logging and set back to ERROR in order to capture normal logs.

Sorry about the weird workaround. Hope the context makes sense, and that I can get this fixed soon.

@BenRKarl
Copy link
Contributor

BenRKarl commented Apr 9, 2019

@rubberviscous linking this related issue here with a comment from @apurvis

#75

@rubberviscous
Copy link

rubberviscous commented Apr 24, 2019

@BenRKarl Thank you for your response.
I'm trying to make the following request:

client = (google.ads.google_ads.client.GoogleAdsClient.load_from_storage())
ga_service = client.get_service('GoogleAdsService', version='v1')
response = ga_service.search(account_id, query, page_size=page_size)

The query:

SELECT segments.date, segments.ad_network_type, metrics.video_quartile_25_rate, metrics.video_quartile_50_rate, metrics.video_quartile_75_rate, metrics.video_quartile_100_rate, metrics.active_view_viewability, metrics.average_cost, metrics.average_cpm, metrics.average_cpv, metrics.cost_micros, metrics.impressions, metrics.video_views, metrics.video_view_rate
FROM ad_group
WHERE segments.ad_network_type='YOUTUBE_WATCH'
LIMIT 10

I'm assuming the definition for API errors could mean a bad query sent to the API? If that's the case, my requests were working fine. It only started blowing up after I added the metrics.video_quartile_25_rate, etc.

Can you articulate on which library to disable logging for? The protobuf library or google-ads? I tried disabling via the standard logging module, but that didn't work:
logging.getLogger("google.protobuf.json_format").setLevel(logging.WARNING)

I also see that the google.ads.google_ads.client.GoogleAdsClient has a logging_dict parameter. I also tried passing in a logging dict but that didn't work either. What's the correct way to disable logging here? Thanks in advance.

@rubberviscous
Copy link

rubberviscous commented Apr 24, 2019

@BenRKarl I just discovered that there is a logging.disable method. I added logging.disable(logging.ERROR) to the script and it seems to work now. Is this what you were referring to?

I was able to get the query working by adding this line logging.getLogger('google.ads.google_ads.client').setLevel(logging.ERROR)

Thanks!

@BenRKarl
Copy link
Contributor

@rubberviscous that definitely works, or you could also disable logging via the configuration by commenting out or removing the lines related to logging.

I'm hoping to refactor the logging interceptor very soon so that it doesn't serialize to JSON, once that's finished I'll comment here to let you know.

@BenRKarl
Copy link
Contributor

BenRKarl commented May 2, 2019

@rubberviscous @apurvis I was finally able to post a PR that removes json serialization of proto messages for logging. You can review it here: #95

Please feel free to leave comments or questions. Once it's merged and published you can start requesting fields such as video_quartile_25_rate again.

Sorry for the delay on this one!

@BenRKarl BenRKarl added bug Something isn't working P1 Breaking issue affecting some users or workflows labels May 10, 2019
@BenRKarl
Copy link
Contributor

Closing this as #95 was merged and published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 Breaking issue affecting some users or workflows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants