Skip to content

Fix rq tests #3762

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Fix rq tests #3762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a1fbb4
Fix transaction name setting and forked some tests to make them work …
antonpirker Nov 11, 2024
e46aa88
Transactions in transactins is undefined behavior, so remove this.
antonpirker Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sentry_sdk/integrations/arq.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ async def _sentry_run_job(self, job_id, score):
with sentry_sdk.isolation_scope() as scope:
scope._name = "arq"
scope.set_transaction_name(
DEFAULT_TRANSACTION_NAME, source=TRANSACTION_SOURCE_TASK,
DEFAULT_TRANSACTION_NAME,
source=TRANSACTION_SOURCE_TASK,
)
scope.clear_breadcrumbs()

Expand Down
19 changes: 13 additions & 6 deletions sentry_sdk/integrations/rq.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

from rq.job import Job

DEFAULT_TRANSACTION_NAME = "unknown RQ task"


class RqIntegration(Integration):
identifier = "rq"
Expand All @@ -55,22 +57,27 @@ def setup_once():
def sentry_patched_perform_job(self, job, *args, **kwargs):
# type: (Any, Job, *Queue, **Any) -> bool
with sentry_sdk.new_scope() as scope:
try:
transaction_name = job.func_name or DEFAULT_TRANSACTION_NAME
except AttributeError:
transaction_name = DEFAULT_TRANSACTION_NAME

scope.set_transaction_name(
transaction_name, source=TRANSACTION_SOURCE_TASK
)
scope.clear_breadcrumbs()
scope.add_event_processor(_make_event_processor(weakref.ref(job)))

with sentry_sdk.continue_trace(
job.meta.get("_sentry_trace_headers") or {}
):
with sentry_sdk.start_transaction(
with sentry_sdk.start_span(
op=OP.QUEUE_TASK_RQ,
name="unknown RQ task",
name=transaction_name,
source=TRANSACTION_SOURCE_TASK,
origin=RqIntegration.origin,
custom_sampling_context={"rq_job": job},
) as transaction:
with capture_internal_exceptions():
transaction.name = job.func_name

):
rv = old_perform_job(self, job, *args, **kwargs)

if self.is_horse:
Expand Down
4 changes: 1 addition & 3 deletions tests/integrations/opentelemetry/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,7 @@ def test_span_data_for_db_query():
),
(
SpanKind.SERVER,
Status(
StatusCode.ERROR, "I'm a teapot"
),
Status(StatusCode.ERROR, "I'm a teapot"),
{
"http.method": "POST",
"http.route": "/some/route",
Expand Down
20 changes: 12 additions & 8 deletions tests/integrations/rq/test_rq.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from fakeredis import FakeStrictRedis

import sentry_sdk
from sentry_sdk import start_transaction
from sentry_sdk.integrations.rq import RqIntegration
from sentry_sdk.utils import parse_version

Expand Down Expand Up @@ -46,6 +45,7 @@ def do_trick(dog, trick):
return "{}, can you {}? Good dog!".format(dog, trick)


@pytest.mark.forked
def test_basic(sentry_init, capture_events):
sentry_init(integrations=[RqIntegration()])
events = capture_events()
Expand Down Expand Up @@ -78,6 +78,7 @@ def test_basic(sentry_init, capture_events):
assert "started_at" in extra


@pytest.mark.forked
def test_transport_shutdown(sentry_init, capture_events_forksafe):
sentry_init(integrations=[RqIntegration()])

Expand All @@ -96,6 +97,7 @@ def test_transport_shutdown(sentry_init, capture_events_forksafe):
assert exception["type"] == "ZeroDivisionError"


@pytest.mark.forked
def test_transaction_with_error(
sentry_init, capture_events, DictionaryContaining # noqa:N803
):
Expand Down Expand Up @@ -131,6 +133,7 @@ def test_transaction_with_error(
)


@pytest.mark.forked
def test_error_has_trace_context_if_tracing_disabled(
sentry_init,
capture_events,
Expand All @@ -149,6 +152,7 @@ def test_error_has_trace_context_if_tracing_disabled(
assert error_event["contexts"]["trace"]


@pytest.mark.forked
def test_tracing_enabled(
sentry_init,
capture_events,
Expand All @@ -159,18 +163,17 @@ def test_tracing_enabled(
queue = rq.Queue(connection=FakeStrictRedis())
worker = rq.SimpleWorker([queue], connection=queue.connection)

with start_transaction(op="rq transaction") as transaction:
queue.enqueue(crashing_job, foo=None)
worker.work(burst=True)
Comment on lines -162 to -164
Copy link
Member Author

Choose a reason for hiding this comment

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

This creates a transaction inside a transaction. This has undefined behavior so I removed it from the test.

queue.enqueue(crashing_job, foo=None)
worker.work(burst=True)

error_event, envelope, _ = events
error_event, transaction = events

assert error_event["transaction"] == "tests.integrations.rq.test_rq.crashing_job"
assert error_event["contexts"]["trace"]["trace_id"] == transaction.trace_id

assert envelope["contexts"]["trace"] == error_event["contexts"]["trace"]
assert transaction["transaction"] == "tests.integrations.rq.test_rq.crashing_job"
assert transaction["contexts"]["trace"] == error_event["contexts"]["trace"]


@pytest.mark.forked
def test_tracing_disabled(
sentry_init,
capture_events,
Expand Down Expand Up @@ -251,6 +254,7 @@ def test_traces_sampler_gets_correct_values_in_sampling_context(
)


@pytest.mark.forked
@pytest.mark.skipif(
parse_version(rq.__version__) < (1, 5), reason="At least rq-1.5 required"
)
Expand Down
Loading