Skip to content

Commit

Permalink
ref(profiling): Do not send single sample profiles (#1879)
Browse files Browse the repository at this point in the history
Single sample profiles are dropped in relay so there's no reason to send them to
begin with. Save the extra bytes by just not sending it.
  • Loading branch information
Zylphrex committed Feb 1, 2023
1 parent bac5bb1 commit 0233e27
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 29 deletions.
28 changes: 22 additions & 6 deletions sentry_sdk/profiler.py
Expand Up @@ -135,14 +135,18 @@ def is_gevent():

_scheduler = None # type: Optional[Scheduler]

# The default sampling frequency to use. This is set at 101 in order to
# mitigate the effects of lockstep sampling.
DEFAULT_SAMPLING_FREQUENCY = 101


# The minimum number of unique samples that must exist in a profile to be
# considered valid.
PROFILE_MINIMUM_SAMPLES = 2


def setup_profiler(options):
# type: (Dict[str, Any]) -> bool
"""
`buffer_secs` determines the max time a sample will be buffered for
`frequency` determines the number of samples to take per second (Hz)
"""

global _scheduler

if _scheduler is not None:
Expand All @@ -153,7 +157,7 @@ def setup_profiler(options):
logger.warn("profiling is only supported on Python >= 3.3")
return False

frequency = 101
frequency = DEFAULT_SAMPLING_FREQUENCY

if is_gevent():
# If gevent has patched the threading modules then we cannot rely on
Expand Down Expand Up @@ -429,6 +433,8 @@ def __init__(
self.stacks = [] # type: List[ProcessedStack]
self.samples = [] # type: List[ProcessedSample]

self.unique_samples = 0

transaction._profile = self

def update_active_thread_id(self):
Expand Down Expand Up @@ -540,6 +546,8 @@ def write(self, ts, sample):
self.stop()
return

self.unique_samples += 1

elapsed_since_start_ns = str(offset)

for tid, (stack_id, stack) in sample:
Expand Down Expand Up @@ -641,6 +649,14 @@ def to_json(self, event_opt, options):
],
}

def valid(self):
# type: () -> bool
return (
self.sampled is not None
and self.sampled
and self.unique_samples >= PROFILE_MINIMUM_SAMPLES
)


class Scheduler(object):
mode = "unknown"
Expand Down
2 changes: 1 addition & 1 deletion sentry_sdk/tracing.py
Expand Up @@ -716,7 +716,7 @@ def finish(self, hub=None, end_timestamp=None):
"spans": finished_spans,
} # type: Event

if self._profile is not None and self._profile.sampled:
if self._profile is not None and self._profile.valid():
event["profile"] = self._profile
contexts.update({"profile": self._profile.get_profile_context()})
self._profile = None
Expand Down
44 changes: 25 additions & 19 deletions tests/integrations/django/asgi/test_asgi.py
Expand Up @@ -7,6 +7,11 @@
from sentry_sdk.integrations.django import DjangoIntegration
from tests.integrations.django.myapp.asgi import channels_application

try:
from unittest import mock # python 3.3 and above
except ImportError:
import mock # python < 3.3

APPS = [channels_application]
if django.VERSION >= (3, 0):
from tests.integrations.django.myapp.asgi import asgi_application
Expand Down Expand Up @@ -81,32 +86,33 @@ async def test_async_views(sentry_init, capture_events, application):
async def test_active_thread_id(
sentry_init, capture_envelopes, teardown_profiling, endpoint, application
):
sentry_init(
integrations=[DjangoIntegration()],
traces_sample_rate=1.0,
_experiments={"profiles_sample_rate": 1.0},
)
with mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0):
sentry_init(
integrations=[DjangoIntegration()],
traces_sample_rate=1.0,
_experiments={"profiles_sample_rate": 1.0},
)

envelopes = capture_envelopes()
envelopes = capture_envelopes()

comm = HttpCommunicator(application, "GET", endpoint)
response = await comm.get_response()
assert response["status"] == 200, response["body"]
comm = HttpCommunicator(application, "GET", endpoint)
response = await comm.get_response()
assert response["status"] == 200, response["body"]

await comm.wait()
await comm.wait()

data = json.loads(response["body"])
data = json.loads(response["body"])

envelopes = [envelope for envelope in envelopes]
assert len(envelopes) == 1
envelopes = [envelope for envelope in envelopes]
assert len(envelopes) == 1

profiles = [item for item in envelopes[0].items if item.type == "profile"]
assert len(profiles) == 1
profiles = [item for item in envelopes[0].items if item.type == "profile"]
assert len(profiles) == 1

for profile in profiles:
transactions = profile.payload.json["transactions"]
assert len(transactions) == 1
assert str(data["active"]) == transactions[0]["active_thread_id"]
for profile in profiles:
transactions = profile.payload.json["transactions"]
assert len(transactions) == 1
assert str(data["active"]) == transactions[0]["active_thread_id"]


@pytest.mark.asyncio
Expand Down
6 changes: 6 additions & 0 deletions tests/integrations/fastapi/test_fastapi.py
Expand Up @@ -12,6 +12,11 @@
from sentry_sdk.integrations.starlette import StarletteIntegration
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware

try:
from unittest import mock # python 3.3 and above
except ImportError:
import mock # python < 3.3


def fastapi_app_factory():
app = FastAPI()
Expand Down Expand Up @@ -155,6 +160,7 @@ def test_legacy_setup(


@pytest.mark.parametrize("endpoint", ["/sync/thread_ids", "/async/thread_ids"])
@mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0)
def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, endpoint):
sentry_init(
traces_sample_rate=1.0,
Expand Down
1 change: 1 addition & 0 deletions tests/integrations/starlette/test_starlette.py
Expand Up @@ -846,6 +846,7 @@ def test_legacy_setup(


@pytest.mark.parametrize("endpoint", ["/sync/thread_ids", "/async/thread_ids"])
@mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0)
def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, endpoint):
sentry_init(
traces_sample_rate=1.0,
Expand Down
1 change: 1 addition & 0 deletions tests/integrations/wsgi/test_wsgi.py
Expand Up @@ -287,6 +287,7 @@ def sample_app(environ, start_response):
@pytest.mark.skipif(
sys.version_info < (3, 3), reason="Profiling is only supported in Python >= 3.3"
)
@mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0)
def test_profile_sent(
sentry_init,
capture_envelopes,
Expand Down
38 changes: 35 additions & 3 deletions tests/test_profiler.py
@@ -1,5 +1,4 @@
import inspect
import mock
import os
import sys
import threading
Expand All @@ -21,6 +20,11 @@
from sentry_sdk.tracing import Transaction
from sentry_sdk._queue import Queue

try:
from unittest import mock # python 3.3 and above
except ImportError:
import mock # python < 3.3

try:
import gevent
except ImportError:
Expand Down Expand Up @@ -88,6 +92,7 @@ def test_profiler_setup_twice(teardown_profiling):
pytest.param(None, 0, id="profiler not enabled"),
],
)
@mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0)
def test_profiled_transaction(
sentry_init,
capture_envelopes,
Expand Down Expand Up @@ -115,6 +120,7 @@ def test_profiled_transaction(
assert len(items["profile"]) == profile_count


@mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0)
def test_profile_context(
sentry_init,
capture_envelopes,
Expand Down Expand Up @@ -145,6 +151,32 @@ def test_profile_context(
}


def test_minimum_unique_samples_required(
sentry_init,
capture_envelopes,
teardown_profiling,
):
sentry_init(
traces_sample_rate=1.0,
_experiments={"profiles_sample_rate": 1.0},
)

envelopes = capture_envelopes()

with start_transaction(name="profiling"):
pass

items = defaultdict(list)
for envelope in envelopes:
for item in envelope.items:
items[item.type].append(item)

assert len(items["transaction"]) == 1
# because we dont leave any time for the profiler to
# take any samples, it should be not be sent
assert len(items["profile"]) == 0


def get_frame(depth=1):
"""
This function is not exactly true to its name. Depending on
Expand Down Expand Up @@ -478,7 +510,7 @@ def test_thread_scheduler_single_background_thread(scheduler_class):
pytest.param(GeventScheduler, marks=requires_gevent, id="gevent scheduler"),
],
)
@mock.patch("sentry_sdk.profiler.MAX_PROFILE_DURATION_NS", int(1))
@mock.patch("sentry_sdk.profiler.MAX_PROFILE_DURATION_NS", 1)
def test_max_profile_duration_reached(scheduler_class):
sample = [
(
Expand Down Expand Up @@ -792,7 +824,7 @@ def test_max_profile_duration_reached(scheduler_class):
pytest.param(GeventScheduler, marks=requires_gevent, id="gevent scheduler"),
],
)
@mock.patch("sentry_sdk.profiler.MAX_PROFILE_DURATION_NS", int(5))
@mock.patch("sentry_sdk.profiler.MAX_PROFILE_DURATION_NS", 5)
def test_profile_processing(
DictionaryContaining, # noqa: N803
scheduler_class,
Expand Down

0 comments on commit 0233e27

Please sign in to comment.