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

ref(profiling): Do not send single sample profiles #1879

Merged
merged 3 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 18 additions & 6 deletions sentry_sdk/profiler.py
Original file line number Diff line number Diff line change
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,10 @@ 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
Original file line number Diff line number Diff line change
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
2 changes: 2 additions & 0 deletions tests/integrations/django/asgi/test_asgi.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import mock

import django
import pytest
Expand Down Expand Up @@ -78,6 +79,7 @@ async def test_async_views(sentry_init, capture_events, application):
@pytest.mark.skipif(
django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
)
@mock.patch("sentry_sdk.profiler.PROFILE_MINIMUM_SAMPLES", 0)
async def test_active_thread_id(
sentry_init, capture_envelopes, teardown_profiling, endpoint, application
):
Expand Down
2 changes: 2 additions & 0 deletions tests/integrations/fastapi/test_fastapi.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import mock
import threading

import pytest
Expand Down Expand Up @@ -155,6 +156,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
2 changes: 2 additions & 0 deletions tests/integrations/starlette/test_starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import base64
import functools
import json
import mock
import os
import threading

Expand Down Expand Up @@ -846,6 +847,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
2 changes: 2 additions & 0 deletions tests/integrations/wsgi/test_wsgi.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import mock
import sys

from werkzeug.test import Client
Expand Down Expand Up @@ -287,6 +288,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
32 changes: 30 additions & 2 deletions tests/test_profiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,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 +116,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 +147,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 +506,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 +820,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