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(spans): track and report spans that were dropped #4005

Merged
merged 14 commits into from
Feb 4, 2025

Conversation

constantinius
Copy link
Contributor

@constantinius constantinius commented Jan 29, 2025

_SpanRecorder now keeps track of dropped_spans, i.e. when above max_spans.

When spans were dropped, the "spans" property will be wrapped in an AnnotatedValue, reporting the mutation.

<!-- Describe your PR here -->

---

Thank you for contributing to `sentry-python`! Please add tests to validate your changes, and lint your code using `tox -e linters`.

Running the test suite on your PR might require maintainer approval. The AWS Lambda tests additionally require a maintainer to add a special label, and they will fail until this label is added.
@constantinius constantinius changed the title build(deps): bump codecov/codecov-action from 5.1.2 to 5.3.1 (#3995) feat(spans): track and report spans that were dropped Jan 30, 2025
Extend tests to check for correct `_meta` content.
Cleanup.
@constantinius constantinius marked this pull request as ready for review February 3, 2025 08:38
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.23%. Comparing base (2724d65) to head (66b50f8).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4005      +/-   ##
==========================================
+ Coverage   80.15%   80.23%   +0.07%     
==========================================
  Files         139      139              
  Lines       15403    15388      -15     
  Branches     2597     2597              
==========================================
- Hits        12347    12346       -1     
+ Misses       2211     2198      -13     
+ Partials      845      844       -1     
Files with missing lines Coverage Δ
sentry_sdk/client.py 79.01% <100.00%> (+0.41%) ⬆️
sentry_sdk/scrubber.py 94.44% <100.00%> (+1.29%) ⬆️
sentry_sdk/tracing.py 77.84% <100.00%> (+0.27%) ⬆️
sentry_sdk/transport.py 80.43% <100.00%> (+0.38%) ⬆️
sentry_sdk/utils.py 84.01% <100.00%> (+0.30%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

Thanks for simplifying! Overall looks good to me, but please address comments before merging

sentry_sdk/_types.py Outdated Show resolved Hide resolved
tests/tracing/test_misc.py Show resolved Hide resolved
sentry_sdk/client.py Outdated Show resolved Hide resolved
@sentrivana
Copy link
Contributor

sentrivana commented Feb 3, 2025

Oh yeah, and the CI / Lint Sources step is failing -- should be fixed if you run the codebase through black. (Or pip install pre-commit in the venv where you have the SDK installed and it should run the checks on commit and also reformat.)

@constantinius constantinius merged commit bba389e into master Feb 4, 2025
151 checks passed
@constantinius constantinius deleted the constantinius/feat/spans/track-dropped-spans branch February 4, 2025 12:49
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