Capturing Performance monitoring transactions for AWS and GCP #830
Conversation
shantanu73
commented
Sep 22, 2020
shantanu73
commented
- Added code to capture transactions related to performance monitoring for AWS & GCP integrations.
- Added test cases as per above changes.
1) Modified aws_lambda.py file to include continue_from_headers method for transaction and kept try-except bock inside transaction, changed op value to serverless.function. 2) Modified gcp.py file to include continue_from_headers method for transaction and kept try-except block inside transaction context, changed op value to serverless.function. 3) Modified gcp.py file to include request data and headers data. 4) Modified test_aws.py file to remove redundant traces_sample_rate parameter and moved the logic for fetching events & envelopes to same loop. 5) Modified test_gcp.py file to remove redundant traces_sample_rate parameter and moved the logic for fetching events & envelopes to same loop (similar to AWS).
| # event. Meaning every body is unstructured to us. | ||
| request["data"] = AnnotatedValue("", {"rem": [["!raw", "x", 0, 0]]}) | ||
| if "body" in aws_event: | ||
| request["data"] = aws_event.get("body", "") |
There was a problem hiding this comment.
@untitaker I made this change to display the request body appear on Sentry Dashboard as shown below. This change was discussed with AJ.
There was a problem hiding this comment.
Please restore the old behavior when the client option send_default_pii is in its default False. If the user set send_default_pii=True we can do what you propose.
Also please restore the code comment.
The reason for this is that we want to be safe by default: the request body can contain a lot of sensitive data and setting sendDefaultPii is our way of having the user say "I accept the risks".
There was a problem hiding this comment.
@untitaker you mean that essentially, I move this section of code I've written to the if section for send_default_pii part and revert the changes here.
There was a problem hiding this comment.
sorry I mean
if send_default_pii:
new_code
else:
old_code
There was a problem hiding this comment.
Okay, I'll make these changes asap.
| @@ -143,6 +153,18 @@ def event_processor(event, hint): | |||
|
|
|||
| request["url"] = "gcp:///{}".format(environ.get("FUNCTION_NAME")) | |||
|
|
|||
| if hasattr(gcp_event, "method"): | |||
There was a problem hiding this comment.
@untitaker I've modified event processor for GCP integration to get request related for Sentry dashboard.
| ) | ||
| try: | ||
| scope.set_tag("gcp_region", environ.get("FUNCTION_REGION")) |
There was a problem hiding this comment.
@untitaker Made all changes as per review comments, you may review them.
sentry_sdk/integrations/gcp.py
Outdated
| @@ -50,7 +50,7 @@ def sentry_func(functionhandler, event, *args, **kwargs): | |||
| logger.debug( | |||
| "The configured timeout could not be fetched from Cloud Functions configuration." | |||
| ) | |||
| return func(functionhandler, event, *args, **kwargs) | |||
There was a problem hiding this comment.
@untitaker wanted to know why was this changed?
If we are going to change this, then I might need to fix code to get event object, as well as fix unit tests for this coz they might break.
There was a problem hiding this comment.
Sorry, I thought this was an unnecessary change, will revert.
There was a problem hiding this comment.
okay, alright. Actually this change was done considering all trigger types for GCP Cloud functions. In all the trigger types, there are 2 parameters which are passed on to the "invoke_user_function" (similar to "request_handler" in AWS) which are "functionhandler" & "event" (similar to "event" & "context" in AWS).
And, since we do need headers from event object and other request related data, this change was made.
|
Perfect, thanks. I will release later this week, out of bandwidth right now unfortunately. |
Sure. Thanks. |
