Added a new integration for Google Cloud Functions and automation test case framework #785

shantanu73 · 2020-08-12T14:29:37Z

Changes:

Added new GCP Integration for Google Cloud Functions.
Added event processor for this integration.
Added code for sending timeout warning.
Added automation test cases for the integration.
Modified tox.ini file for running automation test cases for GCP Integration.
Added code to capture timeout warning as well.

1) Added new GCP Integration for Google Cloud Functions. 2) Added automation test cases for the integration. 3) Modified requirements-test.txt file for running automation test cases for GCP Integration.

untitaker

The integration looks fine, I have concerns about the test setup and test performance.

untitaker · 2020-08-12T14:39:23Z

test-requirements.txt

@@ -4,6 +4,10 @@ tox==3.7.0
 Werkzeug==0.15.5
 pytest-localserver==0.5.0
 pytest-cov==2.8.1
+google-api-python-client==1.10.0


Please put those in tox.ini like the boto3 dependency

Okay sure, I'll update this.

untitaker · 2020-08-12T14:41:27Z

tests/integrations/gcp/test_gcp.py

+    with open(
+        os.environ.get("SENTRY_PYTHON_TEST_GCP_CREDENTIALS_JSON"), "rb"
+    ) as creds_file:
+        for line in creds_file.readlines():


We will not be able to store any files in CI. Can we store the entire JSON in an envvar?

Also do I read it correctly that you're expecting that file to contain multiple JSON objects where you're only interested in the last one? Because you're splitting by line separator and override creds_json in each iteration.

Actually it is only a single json object which is there in the file I'm just reading the file line by line and fetching one single json.
The thing is that, these oauth client credentials are downloaded from as a .json file. I was just setting this environment variable to that json which is stored in the file.
This will be a minor overhead on user to copy the json from the file and set it to the environment variable. Do we want to do that? If so, I'll modify this implementation.

My main point was that it's easy to set secrets in Travis using environment variables but I may have misunderstood the auth flow.

Reading your other comment is there even a way to run this in CI if there's manual user interaction involved?

I'm not completely sure about this, though what I can do is try to give all permissions from my google account to the APIs which are using the credentials. But I think for the first run atleast it still requires user interaction.

untitaker · 2020-08-12T14:41:56Z

tests/integrations/gcp/test_gcp.py

+    if not project_id:
+        pytest.skip("Credentials json file is not valid")
+
+    if os.path.exists("token.pickle"):


Can we avoid using pickle? If possible I'd just stick to JSON.

The thing is that, the pickle basically is used so that user doesn't have to re-authenticate its credentials from the browser for every test case.
So, do we want to do that or keep this usage?

no I would just replace pickle.dumps with json.dumps or something like that

This .pickle file contains actually the authentication token and not json. Since we are using that authentication token to authenticate our requests, I feel this can't be avoided. If we avoid this .pickle file, we'll have to verify each test case run through the browser.

I understand the purpose of the file but I don't understand why we need to use pickle as a serialization format specifically. It seems to me that we could use any other format, like yaml or json.

okay, I got your point, I'll try to do it with json serialization.

untitaker · 2020-08-12T14:43:12Z

tests/integrations/gcp/test_gcp.py

+
+        # STEP : Invoke the cloud function
+        # Adding delay of 60 seconds for new created function to get deployed.
+        time.sleep(60)


That is an awfully slow test for CI. Is there a way around this, such as polling the deployment status?

I need to check if polling deployment status is possible or not. Although, GCP deployments do take an awful lot of time. I've checked and figured that this is a stable minimum time which is needed for the deployment to finish, (the minimum time which I observed was 45 seconds nothing less than that).
So, I'll check if it is possible to poll and verify the status of deployment, but the test cases will still be slow.

Is there a way we could reuse existing deployments then perhaps?

Well, that is something I haven't check yet and I'm not completely sure about this.
Although, even if a small change is there in a particular file in cloud function we need to redeploy the code. That's the reason building this integration took so much more time as compared to AWS. I'm not completely aware if there is a separate process than this.

tests/integrations/gcp/test_gcp.py

1) Corrected log_flag value in the test_gcp.py

1) modified tox.ini and removed changes from test-requirements.txt

shantanu73 · 2020-08-14T07:49:25Z

tox.ini

@@ -44,6 +44,9 @@ envlist =
    # The aws_lambda tests deploy to the real AWS and have their own matrix of Python versions.
    py3.7-aws_lambda

+    # The gcp deploy to the real AWS and have their own matrix of Python versions.
+    py3.7-gcp


@untitaker I've pushed new changes regarding GCP integration tox.ini file for test cases, does this look fine?

1) MOdified GCP Integration to add new event processor. 2) Modified GCP Integration to add code to capture Timeout Warning Exception. 3) Added automation test cases for handled exception, unhandled exception & timeout error.

rhcarvalho

Summary from call with @shantanu73:

Fix linting error
Fix missing test dependency (tox env config)
Document limitation that the current solution only works on Python 3.7 and not on 3.8 / disable the integration if the environment is unsupported to avoid breaking user functions

For now tests will be skipped in CI. As a follow up, convert the tests to use https://github.com/GoogleCloudPlatform/functions-framework-python to implement run_cloud_function so that we don't need to deal with credentials nor network calls.

1) Added code to disable integration in case of versions different from Python 3.7. 2) Modified GCP automation test case code for case of timeout error. 3) Fixed linting issues. 4) Modified tox.ini file to include requests library in case of GCP Integration.

rhcarvalho · 2020-08-19T13:12:48Z

sentry_sdk/integrations/gcp.py

+        # If an integration is there, a client has to be there.
+        client = hub.client  # type: Any
+
+        configured_time = int(environ.get("FUNCTION_TIMEOUT_SEC"))


This is failing in mypy because environ.get may return None if the env var is not set.

In [1]: import os In [2]: os.environ.get("FUNCTION_TIMEOUT_SEC") In [3]: int(os.environ.get("FUNCTION_TIMEOUT_SEC")) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-3-d034f2d5d55a> in <module> ----> 1 int(os.environ.get("FUNCTION_TIMEOUT_SEC")) TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

We need to handle the case in which the variable is not set.

If we don't know the configured timeout, we cannot start the TimeoutThread.

I suggest handling this by writing a debug log and then ignoring the rest of the wrapper, similar to when the integration is not in the current hub, as above.

Okay, sure. I'll make this change.

@rhcarvalho modified this.

@rhcarvalho Maybe bcoz f the last check-in the linting got failed. I'll use make format and make lint. Though I do follow this as standard practice.

I'll check for the import errors in test jobs.

@rhcarvalho I'm a little doubtful on updating of tox.ini file. I've updated it as per the reference from aws_lambda. But, it still gave the import error.
Are my changes appropriate?
Also, I've tried running the commands you suggested, it took a lot of time to run but didn't complete.

you are missing an tests/integrations/aws_lambda/__init__.py which would skip the tests if the dependency is not installed (in other jobs)

@untitaker okay, so I need a similar file for gcp tests right?

1) Modified Sentry function wrapper to handled configured time.

rhcarvalho · 2020-08-19T15:52:22Z

@shantanu73 now the linting job fails because of code formatting -- https://travis-ci.com/github/getsentry/sentry-python/jobs/375368281

You can run the lint job locally before submitting your commit with make lint and you can update the code formatting with make format. That's faster than waiting for Travis 😉

There are still import errors in the test jobs: https://travis-ci.com/github/getsentry/sentry-python/jobs/375368274

You can run them locally / partially using tox -e [ENVIRONMENT_NAME], or scripts/runtox.sh (this is what is used in Travis, see .travis.yml file) to test the fixes locally.

untitaker · 2020-08-20T08:45:34Z

tests/integrations/gcp/test_gcp.py

 import os.path
 import os
 from googleapiclient.discovery import build
 from google_auth_oauthlib.flow import InstalledAppFlow
 from google.auth.transport.requests import Request

+requests = pytest.importorskip("requests")
+pickle = pytest.importorskip("pickle")


pickle is stdlib, this should not be necessary

okay, I'll change this.

untitaker · 2020-08-20T08:46:30Z

tests/integrations/gcp/test_gcp.py

+import os.path
+import os
+
+requests = pytest.importorskip("requests")


I would importorskip the google cloud sdk additionally

Sure I'll do that as well.

1) Added google-cloud-sdk inportorskip statement. 2) Reverted import changes related to pickle module/library.

1) Commented the code to run gcp automation test cases from tox.ini file.

Changes:

b139e98

1) Added new GCP Integration for Google Cloud Functions. 2) Added automation test cases for the integration. 3) Modified requirements-test.txt file for running automation test cases for GCP Integration.

untitaker requested changes Aug 12, 2020

View reviewed changes

Shantanu Dhiman added 2 commits August 12, 2020 20:43

Changes:

2c79bfc

1) Corrected log_flag value in the test_gcp.py

Changes:

cf02b37

1) modified tox.ini and removed changes from test-requirements.txt

shantanu73 commented Aug 14, 2020

View reviewed changes

Changes:

b4f15ec

1) MOdified GCP Integration to add new event processor. 2) Modified GCP Integration to add code to capture Timeout Warning Exception. 3) Added automation test cases for handled exception, unhandled exception & timeout error.

rhcarvalho reviewed Aug 19, 2020

View reviewed changes

Changes:

95c459a

1) Added code to disable integration in case of versions different from Python 3.7. 2) Modified GCP automation test case code for case of timeout error. 3) Fixed linting issues. 4) Modified tox.ini file to include requests library in case of GCP Integration.

rhcarvalho reviewed Aug 19, 2020

View reviewed changes

Changes:

bd316e7

1) Modified Sentry function wrapper to handled configured time.

Shantanu Dhiman added 3 commits August 19, 2020 22:54

Modified gcp.py to resolve linting errors

2350288

Modified tox.ini & test_gcp.py file to resolve issue related to import

56d1bcc

Resolved all import issues in test_gcp.py

e52eff6

untitaker reviewed Aug 20, 2020

View reviewed changes

Changes:

717b1c9

1) Added google-cloud-sdk inportorskip statement. 2) Reverted import changes related to pickle module/library.

untitaker approved these changes Aug 20, 2020

View reviewed changes

Changes:

e74be8c

1) Commented the code to run gcp automation test cases from tox.ini file.

rhcarvalho merged commit 3b37cb5 into getsentry:master Aug 20, 2020

sentrivana mentioned this pull request Nov 14, 2023

Cannot make Sentry work with Google Cloud Function (python 3.9) #1666

Closed

Added a new integration for Google Cloud Functions and automation test case framework #785

Added a new integration for Google Cloud Functions and automation test case framework #785

Uh oh!

Conversation

shantanu73 commented Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

untitaker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shantanu73 Aug 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhcarvalho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhcarvalho commented Aug 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shantanu73 commented Aug 12, 2020 •

edited

Loading

shantanu73 Aug 14, 2020 •

edited

Loading