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

Capturing Performance monitoring transactions for AWS and GCP #830

Merged
merged 12 commits into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 20 additions & 17 deletions sentry_sdk/integrations/aws_lambda.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
import sys

from sentry_sdk.hub import Hub, _should_send_default_pii
from sentry_sdk.tracing import Transaction
from sentry_sdk._compat import reraise
from sentry_sdk.utils import (
AnnotatedValue,
capture_internal_exceptions,
event_from_exception,
logger,
Expand Down Expand Up @@ -78,10 +78,10 @@ def sentry_handler(event, context, *args, **kwargs):
with hub.push_scope() as scope:
with capture_internal_exceptions():
scope.clear_breadcrumbs()
scope.transaction = context.function_name
scope.add_event_processor(
_make_request_event_processor(event, context, configured_time)
)
scope.set_tag("region", context.invoked_function_arn.split(":")[3])
shantanu73 marked this conversation as resolved.
Show resolved Hide resolved
# Starting the Timeout thread only if the configured time is greater than Timeout warning
# buffer and timeout_warning parameter is set True.
if (
Expand All @@ -99,17 +99,22 @@ def sentry_handler(event, context, *args, **kwargs):
# Starting the thread to raise timeout warning exception
timeout_thread.start()

try:
return handler(event, context, *args, **kwargs)
except Exception:
exc_info = sys.exc_info()
event, hint = event_from_exception(
exc_info,
client_options=client.options,
mechanism={"type": "aws_lambda", "handled": False},
)
hub.capture_event(event, hint=hint)
reraise(*exc_info)
headers = _filter_headers(event.get("headers", {}))
untitaker marked this conversation as resolved.
Show resolved Hide resolved
transaction = Transaction.continue_from_headers(
headers, op="serverless.function", name=context.function_name
)
with hub.start_transaction(transaction):
try:
return handler(event, context, *args, **kwargs)
except Exception:
exc_info = sys.exc_info()
event, hint = event_from_exception(
exc_info,
client_options=client.options,
mechanism={"type": "aws_lambda", "handled": False},
)
hub.capture_event(event, hint=hint)
reraise(*exc_info)

return sentry_handler # type: ignore

Expand Down Expand Up @@ -277,10 +282,8 @@ def event_processor(event, hint, start_time=start_time):
if "headers" in aws_event:
request["headers"] = _filter_headers(aws_event["headers"])

if aws_event.get("body", None):
# Unfortunately couldn't find a way to get structured body from 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", "")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@untitaker I made this change to display the request body appear on Sentry Dashboard as shown below. This change was discussed with AJ.

req_body

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I mean

if send_default_pii:
    new_code
else:
    old_code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll make these changes asap.


if _should_send_default_pii():
user_info = event.setdefault("user", {})
Expand Down
67 changes: 45 additions & 22 deletions sentry_sdk/integrations/gcp.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from datetime import datetime, timedelta
from os import environ
import sys
import json

from sentry_sdk.hub import Hub
from sentry_sdk.tracing import Transaction
from sentry_sdk._compat import reraise
from sentry_sdk.utils import (
capture_internal_exceptions,
Expand All @@ -11,6 +13,7 @@
TimeoutThread,
)
from sentry_sdk.integrations import Integration
from sentry_sdk.integrations._wsgi_common import _filter_headers

from sentry_sdk._types import MYPY

Expand All @@ -31,13 +34,13 @@

def _wrap_func(func):
# type: (F) -> F
def sentry_func(*args, **kwargs):
# type: (*Any, **Any) -> Any
def sentry_func(functionhandler, event, *args, **kwargs):
# type: (Any, Any, *Any, **Any) -> Any
untitaker marked this conversation as resolved.
Show resolved Hide resolved

hub = Hub.current
integration = hub.get_integration(GcpIntegration)
if integration is None:
return func(*args, **kwargs)
return func(functionhandler, event, *args, **kwargs)
untitaker marked this conversation as resolved.
Show resolved Hide resolved

# If an integration is there, a client has to be there.
client = hub.client # type: Any
Expand All @@ -47,7 +50,7 @@ def sentry_func(*args, **kwargs):
logger.debug(
"The configured timeout could not be fetched from Cloud Functions configuration."
)
return func(*args, **kwargs)
return func(functionhandler, event, *args, **kwargs)
untitaker marked this conversation as resolved.
Show resolved Hide resolved

configured_time = int(configured_time)

Expand All @@ -56,11 +59,10 @@ def sentry_func(*args, **kwargs):
with hub.push_scope() as scope:
with capture_internal_exceptions():
scope.clear_breadcrumbs()
scope.transaction = environ.get("FUNCTION_NAME")
scope.add_event_processor(
_make_request_event_processor(configured_time, initial_time)
_make_request_event_processor(event, configured_time, initial_time)
)
try:
scope.set_tag("region", environ.get("FUNCTION_REGION"))
if (
integration.timeout_warning
and configured_time > TIMEOUT_WARNING_BUFFER
Expand All @@ -71,19 +73,28 @@ def sentry_func(*args, **kwargs):

# Starting the thread to raise timeout warning exception
timeout_thread.start()
return func(*args, **kwargs)
except Exception:
exc_info = sys.exc_info()
event, hint = event_from_exception(
exc_info,
client_options=client.options,
mechanism={"type": "gcp", "handled": False},
)
hub.capture_event(event, hint=hint)
reraise(*exc_info)
finally:
# Flush out the event queue
hub.flush()

headers = {}
if hasattr(event, "headers"):
headers = _filter_headers(event.headers)
untitaker marked this conversation as resolved.
Show resolved Hide resolved
transaction = Transaction.continue_from_headers(
headers, op="serverless.function", name=environ.get("FUNCTION_NAME", "")
)
with hub.start_transaction(transaction):
try:
return func(functionhandler, event, *args, **kwargs)
except Exception:
exc_info = sys.exc_info()
event, hint = event_from_exception(
exc_info,
client_options=client.options,
mechanism={"type": "gcp", "handled": False},
)
hub.capture_event(event, hint=hint)
reraise(*exc_info)
finally:
# Flush out the event queue
hub.flush()

return sentry_func # type: ignore

Expand Down Expand Up @@ -113,8 +124,8 @@ def setup_once():
)


def _make_request_event_processor(configured_timeout, initial_time):
# type: (Any, Any) -> EventProcessor
def _make_request_event_processor(gcp_event, configured_timeout, initial_time):
# type: (Any, Any, Any) -> EventProcessor

def event_processor(event, hint):
# type: (Event, Hint) -> Optional[Event]
Expand Down Expand Up @@ -143,6 +154,18 @@ def event_processor(event, hint):

request["url"] = "gcp:///{}".format(environ.get("FUNCTION_NAME"))

if hasattr(gcp_event, "method"):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@untitaker I've modified event processor for GCP integration to get request related for Sentry dashboard.

request["method"] = gcp_event.method

if hasattr(gcp_event, "query_string"):
request["query_string"] = gcp_event.query_string.decode("utf-8")

if hasattr(gcp_event, "headers"):
request["headers"] = _filter_headers(gcp_event.headers)

if hasattr(gcp_event, "data"):
shantanu73 marked this conversation as resolved.
Show resolved Hide resolved
request["data"] = json.loads(gcp_event.data)

event["request"] = request

return event
Expand Down
89 changes: 80 additions & 9 deletions tests/integrations/aws_lambda/test_aws.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ def event_processor(event):
# to print less to logs.
return event

def envelope_processor(envelope):
untitaker marked this conversation as resolved.
Show resolved Hide resolved
(item,) = envelope.items
envelope_json = json.loads(item.get_bytes())

envelope_data = {}
envelope_data[\"contexts\"] = {}
envelope_data[\"type\"] = envelope_json[\"type\"]
envelope_data[\"transaction\"] = envelope_json[\"transaction\"]
envelope_data[\"contexts\"][\"trace\"] = envelope_json[\"contexts\"][\"trace\"]
envelope_data[\"request\"] = envelope_json[\"request\"]

return envelope_data

class TestTransport(HttpTransport):
def _send_event(self, event):
event = event_processor(event)
Expand All @@ -49,6 +62,10 @@ def _send_event(self, event):
# us one.
print("\\nEVENT: {}\\n".format(json.dumps(event)))

def _send_envelope(self, envelope):
envelope = envelope_processor(envelope)
print("\\nENVELOPE: {}\\n".format(json.dumps(envelope)))

def init_sdk(timeout_warning=False, **extra_init_args):
sentry_sdk.init(
dsn="https://123abc@example.com/123",
Expand Down Expand Up @@ -91,21 +108,26 @@ def inner(code, payload, timeout=30, syntax_check=True):
)

events = []
envelopes = []

for line in base64.b64decode(response["LogResult"]).splitlines():
print("AWS:", line)
if not line.startswith(b"EVENT: "):
if line.startswith(b"EVENT: "):
line = line[len(b"EVENT: ") :]
events.append(json.loads(line.decode("utf-8")))
elif line.startswith(b"ENVELOPE: "):
line = line[len(b"ENVELOPE: ") :]
envelopes.append(json.loads(line.decode("utf-8")))
else:
continue
line = line[len(b"EVENT: ") :]
events.append(json.loads(line.decode("utf-8")))

return events, response
return envelopes, events, response

return inner


def test_basic(run_lambda_function):
events, response = run_lambda_function(
envelopes, events, response = run_lambda_function(
LAMBDA_PRELUDE
+ dedent(
"""
Expand Down Expand Up @@ -160,7 +182,7 @@ def test_initialization_order(run_lambda_function):
as seen by AWS already runs. At this point at least draining the queue
should work."""

events, _response = run_lambda_function(
envelopes, events, _response = run_lambda_function(
LAMBDA_PRELUDE
+ dedent(
"""
Expand All @@ -180,7 +202,7 @@ def test_handler(event, context):


def test_request_data(run_lambda_function):
events, _response = run_lambda_function(
envelopes, events, _response = run_lambda_function(
LAMBDA_PRELUDE
+ dedent(
"""
Expand Down Expand Up @@ -235,7 +257,7 @@ def test_init_error(run_lambda_function, lambda_runtime):
if lambda_runtime == "python2.7":
pytest.skip("initialization error not supported on Python 2.7")

events, response = run_lambda_function(
envelopes, events, response = run_lambda_function(
LAMBDA_PRELUDE
+ (
"def event_processor(event):\n"
Expand All @@ -252,7 +274,7 @@ def test_init_error(run_lambda_function, lambda_runtime):


def test_timeout_error(run_lambda_function):
events, response = run_lambda_function(
envelopes, events, response = run_lambda_function(
LAMBDA_PRELUDE
+ dedent(
"""
Expand Down Expand Up @@ -291,3 +313,52 @@ def test_handler(event, context):
log_stream = event["extra"]["cloudwatch logs"]["log_stream"]

assert re.match(log_stream_re, log_stream)


def test_performance_no_error(run_lambda_function):
envelopes, events, response = run_lambda_function(
LAMBDA_PRELUDE
+ dedent(
"""
init_sdk(traces_sample_rate=1.0)

def test_handler(event, context):
return "test_string"
"""
),
b'{"foo": "bar"}',
)

(envelope,) = envelopes
assert envelope["type"] == "transaction"
assert envelope["contexts"]["trace"]["op"] == "serverless.function"
assert envelope["transaction"].startswith("test_function_")
assert envelope["transaction"] in envelope["request"]["url"]


def test_performance_error(run_lambda_function):
envelopes, events, response = run_lambda_function(
LAMBDA_PRELUDE
+ dedent(
"""
init_sdk(traces_sample_rate=1.0)

def test_handler(event, context):
raise Exception("something went wrong")
"""
),
b'{"foo": "bar"}',
)

(event,) = events
assert event["level"] == "error"
(exception,) = event["exception"]["values"]
assert exception["type"] == "Exception"
assert exception["value"] == "something went wrong"

(envelope,) = envelopes

assert envelope["type"] == "transaction"
assert envelope["contexts"]["trace"]["op"] == "serverless.function"
assert envelope["transaction"].startswith("test_function_")
assert envelope["transaction"] in envelope["request"]["url"]