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

Add Error Reporting gRPC #2687

Closed
waprin opened this issue Nov 4, 2016 · 23 comments
Closed

Add Error Reporting gRPC #2687

waprin opened this issue Nov 4, 2016 · 23 comments
Assignees
Labels
api: clouderrorreporting Issues related to the Error Reporting API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@waprin
Copy link
Contributor

waprin commented Nov 4, 2016

This is a tracking work item to get the Error Reporting GAPIC into this library:

https://pypi.python.org/pypi/gapic-google-cloud-error-reporting-v1beta1

@steren and I agree that the existing (small) report and report_exception is what we want to keep, along with possible more framework integrations in the future. So I think what we primarily want is the same thing as with done with Logging where we add the GAPIC as a dependency, and then fallback to the existing code if gRPC is disabled.

One small question is if and to what extent we want to expose the rest of the GAPIC besides events.report via this library. There are a few API calls that will likely only be used by power users, but it might be worth having some sort of pass-through mechanism. Unless I'm mistake, it looks like so far we either wrap all the GAPIC calls or pass-through completely, but we haven't done both, so not sure exactly how this would look.

cc @bjwatson @jgeewax @omaray in case you have comments.

@bjwatson
Copy link

bjwatson commented Nov 7, 2016

@jskeet IIRC, you have experience doing thin and/or partial hand-written layers on the C# GAPIC code. To date, we have done thick hand-written layers for Python. Do you have any advice for how best to manage thinner layers?

cc @anthmgoogle

@jskeet
Copy link
Contributor

jskeet commented Nov 7, 2016

Hmm... it's likely to be language-specific. In C#, we can add extra methods/properties/etc to existing classes using partial... I don't know what the equivalent is in Python. But aside from anything else, we've already decided that all the GAPIC code is exposed in C#; is that true in Python?

If you're happy to expose the GAPIC code for power users, you could have a very simple wrapper that exposes simple operations but also exposes a property to access all the GAPIC methods if necessary.

@bjwatson
Copy link

bjwatson commented Nov 7, 2016

Thanks @jskeet. Makes sense how you're leveraging the C# partial feature.

Since Python is package delivery (i.e. the GAPIC code has its own package on https://pypi.python.org/pypi), the GAPIC code is always exposed to our users.

@waprin
Copy link
Contributor Author

waprin commented Nov 7, 2016

Ok, now that I am thinking about it, if it's a dependency of this library it will be exposed after installing this library, that makes sense.

I am leaving for Brazil tomorrow and will be busy with GCP Next Sao Paulo but I can loop back on this later in the month.

@bjwatson
Copy link

bjwatson commented Nov 8, 2016

Yep, the other google-cloud-python libraries packages that depend on GAPIC call it out as a dependency in their setup.py. Have fun in Sao Paulo!

@steren
Copy link

steren commented Dec 20, 2016

google-cloud-python contains the Error Reporting instrumentation library, but not the Error Reporting client library.

Is this issue about bringing the client libraries in? Is anybody working on it?

Context: I was looking to call the Error Reporting "list error stats" API today, but could not do it in Python or Node

@daspecster
Copy link
Contributor

@bjwatson
Copy link

Hi @steren. There is no hand-written client library for Error Reporting; we are using just the auto-generated client library (plus the hand-written instrumentation code). Also for Python, we publish the auto-generated client libraries separately from the google-cloud-python libraries.

The client library is published at https://pypi.python.org/pypi/gapic-google-cloud-error-reporting-v1beta1, and you can find the source code at https://github.com/googleapis/api-client-staging/tree/master/generated/python/gapic-google-cloud-error-reporting-v1beta1.

@steren
Copy link

steren commented Dec 20, 2016

My main concern is that https://googlecloudplatform.github.io/google-cloud-python/ does not mention or document the client library.

My question is: Is this GitHub issue about adding the client library to google-cloud-python? Or is it something else?

@bjwatson
Copy link

I think this issue is best resolved by adding a dependency on the GAPIC package to https://github.com/GoogleCloudPlatform/google-cloud-python/blob/master/error_reporting/setup.py.

The documentation issue is covered by https://github.com/GoogleCloudPlatform/gcloud-common/issues/207.

@dhermes
Copy link
Contributor

dhermes commented Dec 21, 2016

I think it's probably easiest if @waprin just answers here and we stop talking past each other?

The implementation in google-cloud-python (which is the project this issue tracker is for) was done exclusively by @waprin. AFAICT it use the Stackdriver Logging API directly to "report errors".

@waprin
Copy link
Contributor Author

waprin commented Dec 21, 2016

So a few things need to be done here:

  • Add the dependency on the GAPIC as @bjwatson says. That's pretty easy (Add Error Reporting GAPIC as dependency #2894) but as @steren points out, there are no docs. The gcloud-common#207 issue still seems like a proposal rather than a solution? And we would also still have no dev-site docs. But sounds like there is some higher level plan to address that.

  • Change the existing instrumentation code to use this client by default, falling back to the the Logging API approach if there's no gRPC. This has just been in my backlog forever, will give it a shot now.

I think once that's done we can close this issue, but maybe start some new ones about how this all gets documented clearly (especially my pet concern of how we relate to cloud.google.com vs github docs).

@bjwatson
Copy link

@waprin Good point about the GAPIC docs. They exist as docstrings in the code, but we have not web-rendered it. We've just started doing this again with Logging, since we declared beta for it a couple weeks ago.

We'll take care of publishing docs for other packages in early January. FYI, @geigerj and @ethanbao.

@dhermes dhermes added api: clouderrorreporting Issues related to the Error Reporting API. and removed api: clouderrorreporting Issues related to the Error Reporting API. error reporting labels Feb 23, 2017
@DizzeePascall
Copy link
Contributor

When running the examples from the documentation, I get the following error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/laurence/miniconda3/envs/sonalytic/lib/python3.5/site-packages/google/cloud/error_reporting/client.py", line 320, in report
    report_location=report_location)
  File "/home/laurence/miniconda3/envs/sonalytic/lib/python3.5/site-packages/google/cloud/error_reporting/client.py", line 277, in _send_error_report
    self.report_errors_api.report_error_event(error_report)
  File "/home/laurence/miniconda3/envs/sonalytic/lib/python3.5/site-packages/google/cloud/error_reporting/client.py", line 162, in report_errors_api
    self._report_errors_api = make_report_error_api(self)
  File "/home/laurence/miniconda3/envs/sonalytic/lib/python3.5/site-packages/google/cloud/error_reporting/_gax.py", line 44, in make_report_error_api
    return _ErrorReportingGaxApi(gax_client, client.project)
AttributeError: 'Client' object has no attribute 'project'

Disabling GRPC via the GOOGLE_CLOUD_DISABLE_GRPC environment variable fixes this.

I'm running on an Ubuntu 14.04 compute instance with python 3.5.3 and version 023.1 of the google-cloud-error-reporting project

@bjwatson
Copy link

FYI @lukesneeringer

@bjwatson bjwatson added priority: p2 Moderately-important priority. Fix may not be included in next release. status: acknowledged type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Mar 15, 2017
@lukesneeringer
Copy link
Contributor

@daspecster Can you look at this one? It looks like it is probably a quick bugfix.

@daspecster
Copy link
Contributor

@lukesneeringer sure thing!

@waprin
Copy link
Contributor Author

waprin commented Mar 16, 2017

@daspecster mentioned this to Luke but I'm OOO next week but spending today and tomorrow trying to catch up on loose ends/random issues so feel free to ping me on anything you want me to look at/try to fix.

@lukesneeringer
Copy link
Contributor

@waprin I am really only referring to this comment, which honestly should be a separate issue.

@bjwatson
Copy link

@lukesneeringer Can you or @daspecster split it out as a separate issue to reflect that?

@waprin
Copy link
Contributor Author

waprin commented Mar 16, 2017

Now I am understanding you're just referring to the reported bug. Yes, I think once we create that new issue, we can close this issue as it was done in #2894 . Only thing I think that's left is the documentation for the GAPIC which is also a separate issue.

@daspecster
Copy link
Contributor

@waprin @lukesneeringer @bjwatson issue created!
#3158

@lukesneeringer
Copy link
Contributor

Closing this issue (done in #2894).

parthea pushed a commit that referenced this issue Sep 22, 2023
…-samples#2687)

* Add voice selection by name

* Add future to requirements.txt

Fixes python2 test failures referencing html.

Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>
parthea pushed a commit that referenced this issue Sep 22, 2023
…-samples#2687)

* Add voice selection by name

* Add future to requirements.txt

Fixes python2 test failures referencing html.

Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: clouderrorreporting Issues related to the Error Reporting API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

8 participants