GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(django): trace cache.add operations by immanuwell · Pull Request #6403 · getsentry/sentry-python

Conversation

@immanuwell

@immanuwell immanuwell commented May 26, 2026

Copy link
Copy Markdown

Refs #6402

cache.add() was a blind spot. cache.set() emitted cache.put, cache.add() emitted nothing. kinda odd for a normal Django cache write path.

Repro

from django.core.cache import cache
import sentry_sdk
from sentry_sdk.integrations.django import DjangoIntegration

sentry_sdk.init(
    integrations=[DjangoIntegration(cache_spans=True)],
    traces_sample_rate=1.0,
)

with sentry_sdk.start_transaction(name="t", op="test"):
    cache.add("k", "value")

Before this patch: no cache span.
After this patch: one cache.put span for cache.add().

What changed

  • instrument cache.add()
  • treat it as cache.put
  • add a regression test for static + stream mode

Tested

  • tox -e py3.12-django-v4.2.30 -- -k test_cache_spans_add

I also checked a wider Django cache test slice, but this env doesnt have the repo's local Postgres test service, so the db-marked Django tests bail early.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@immanuwell
fix(django): trace cache.add operations
01bfc73
@immanuwell
immanuwell force-pushed the fix/django-cache-add-span branch from 5512f0e to 01bfc73 Compare May 26, 2026 04:42
ericapisani
ericapisani requested changes May 26, 2026
View reviewed changes

@ericapisani ericapisani left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for opening this PR! These changes are overall looking good, just need to update something in the span streaming test before we look to get it across the finish line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread tests/integrations/django/test_cache_module.py
if span_streaming:
items = capture_items("span")

with sentry_sdk.start_transaction(name="cache-add", op="test"):

@ericapisani ericapisani May 26, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

In the context of span streaming, we no longer have the concept of a transaction, we instead have a segment.

Because of that, we shouldn't be invoking this method, but if we need to create a span to attach the generated cache span to, use with sentry_sdk.traces.start_span(...) instead.

The assertions from lines 575 to 579 will likely need to be updated as a result, as there will be another span caught by the capture_items line above, so the index value used will change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

alexander-alderman-webb reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@alexander-alderman-webb alexander-alderman-webb May 26, 2026

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes, just adapt the test to follow the existing pattern in the same file @immanuwell

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@alexander-alderman-webb

alexander-alderman-webb commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Please merge master into your branch as well to resolve unrelated CI failures.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-alderman-webb alexander-alderman-webb alexander-alderman-webb left review comments
@ericapisani ericapisani ericapisani requested changes

Requested changes must be addressed to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@immanuwell @alexander-alderman-webb @ericapisani
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.