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

feat(scope): Emit buffer_overflow discard reason when popping breadcrumbs #3993

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AbhiPrasad
Copy link
Member

ref getsentry/team-sdks#116

This PR implements a new client discard reason for buffer_overflow. This will be used to track when the internal breadcrumbs buffer overflows for the new logs product that we are working on.

log_item is the new data category for logs, which we are basing out of breadcrumbs. Eventually we will be sending standalone envelope items that only contain breadcrumbs.

Copy link

codecov bot commented Jan 24, 2025

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
22080 3 22077 5708
View the top 3 failed tests by shortest run time
tests.integrations.aiohttp.test_aiohttp test_crumb_capture
Stack Traces | 0.002s run time
file .../integrations/aiohttp/test_aiohttp.py, line 476
  @pytest.mark.asyncio
  async def test_crumb_capture(
      sentry_init, aiohttp_raw_server, aiohttp_client, loop, capture_events
  ):
      def before_breadcrumb(crumb, hint):
E       fixture 'loop' not found
>       available fixtures: DictionaryContaining, ObjectDescribedBy, StringContaining, _capture_internal_warnings, _session_event_loop, aiohttp_client, aiohttp_client_cls, aiohttp_raw_server, aiohttp_server, benchmark, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, capture_envelopes, capture_events, capture_events_forksafe, capture_exceptions, capture_record_lost_event_calls, clean_scopes, cov, doctest_namespace, event_loop, event_loop_policy, httpserver, httpsserver, internal_exceptions, maybe_monkeypatched_threading, monkeypatch, no_cover, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, render_span_tree, reset_integrations, sentry_init, smtpserver, suppress_deprecation_warnings, teardown_profiling, .../integrations/aiohttp/test_aiohttp.py::<event_loop>, tests/integrations/aiohttp::<event_loop>, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, uninstall_integration, unused_tcp_port, unused_tcp_port_factory, unused_udp_port, unused_udp_port_factory, validate_event_schema
>       use 'pytest --fixtures [testpath]' for help on them.

.../integrations/aiohttp/test_aiohttp.py:476
tests.integrations.aiohttp.test_aiohttp test_crumb_capture
Stack Traces | 0.002s run time
file .../integrations/aiohttp/test_aiohttp.py, line 476
  @pytest.mark.asyncio
  async def test_crumb_capture(
      sentry_init, aiohttp_raw_server, aiohttp_client, loop, capture_events
  ):
      def before_breadcrumb(crumb, hint):
E       fixture 'loop' not found
>       available fixtures: DictionaryContaining, ObjectDescribedBy, StringContaining, _capture_internal_warnings, _session_event_loop, aiohttp_client, aiohttp_client_cls, aiohttp_raw_server, aiohttp_server, benchmark, cache, capfd, capfdbinary, caplog, capsys, capsysbinary, capture_envelopes, capture_events, capture_events_forksafe, capture_exceptions, capture_record_lost_event_calls, clean_scopes, cov, doctest_namespace, event_loop, event_loop_policy, httpserver, httpsserver, internal_exceptions, maybe_monkeypatched_threading, monkeypatch, no_cover, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, render_span_tree, reset_integrations, sentry_init, smtpserver, suppress_deprecation_warnings, teardown_profiling, .../integrations/aiohttp/test_aiohttp.py::<event_loop>, tests/integrations/aiohttp::<event_loop>, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, uninstall_integration, unused_tcp_port, unused_tcp_port_factory, unused_udp_port, unused_udp_port_factory, validate_event_schema
>       use 'pytest --fixtures [testpath]' for help on them.

.../integrations/aiohttp/test_aiohttp.py:476
tests.integrations.stdlib.test_httplib test_http_timeout
Stack Traces | 0.263s run time
.../integrations/stdlib/test_httplib.py:373: in test_http_timeout
    assert len(transaction["spans"]) == 1
E   assert 0 == 1
E     +0
E     -1

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@AbhiPrasad AbhiPrasad force-pushed the abhi-python-sdk-breadcrumbs-buffer-overflow branch from 51e600f to 2621c08 Compare January 24, 2025 17:59
@sentrivana
Copy link
Contributor

@AbhiPrasad The change looks good to me but we should have a test for this.

I imagine we can come up with a test case by combining some of the client report tests in https://github.com/getsentry/sentry-python/blob/master/tests/test_transport.py and this max_breadcrumbs test.

Do you have some bandwidth to look into adding a test case? Otherwise I can try coming up with something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants