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

Extract span attrs from RQ job object & fix tests #3786

Merged
merged 9 commits into from
Nov 21, 2024

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Nov 15, 2024

  • replace rq_job with some span attrs to populate the sampling context in traces sampler
  • make test_rq pass
  • do not use generic op like message if we already explicitly set op

This is not great since we have to serialize everything (args, kwargs, function to be executed). But it's the best we can do with span attrs.

@sentrivana sentrivana mentioned this pull request Nov 15, 2024
12 tasks
Copy link

codecov bot commented Nov 15, 2024

❌ 622 Tests Failed:

Tests completed Failed Passed Skipped
19692 622 19070 4426
View the top 2 failed tests by shortest run time
pytest internal
Stack Traces | 0s run time
No failure message available
tests.integrations.opentelemetry.test_span_processor test_on_start_transaction
Stack Traces | 0.002s run time
.../integrations/opentelemetry/test_span_processor.py:287: in test_on_start_transaction
    with mock.patch(
.../hostedtoolcache/Python/3.12.7....../x64/lib/python3.12/unittest/mock.py:1463: in __enter__
    original, local = self.get_original()
.../hostedtoolcache/Python/3.12.7....../x64/lib/python3.12/unittest/mock.py:1436: in get_original
    raise AttributeError(
E   AttributeError: <module 'sentry_sdk.integrations.opentelemetry.span_processor' from '.../integrations/opentelemetry/span_processor.py'> does not have the attribute 'start_transaction'
View the full list of 1 ❄️ flaky tests
tests.integrations.opentelemetry.test_span_processor test_on_start_transaction

Flake rate in main: 50.00% (Passed 4 times, Failed 4 times)

Stack Traces | 0.001s run time
.../integrations/opentelemetry/test_span_processor.py:289: in test_on_start_transaction
    fake_start_transaction,
.../hostedtoolcache/Python/3.7.17....../x64/lib/python3.7/unittest/mock.py:1307: in __enter__
    original, local = self.get_original()
.../hostedtoolcache/Python/3.7.17....../x64/lib/python3.7/unittest/mock.py:1281: in get_original
    "%s does not have the attribute %r" % (target, name)
E   AttributeError: <module 'sentry_sdk.integrations.opentelemetry.span_processor' from '.../integrations/opentelemetry/span_processor.py'> does not have the attribute 'start_transaction'

To view more test analytics, go to the Test Analytics Dashboard
Got feedback? Let us know on Github

@sentrivana sentrivana changed the title Extract span attrs from RQ job object Extract span attrs from RQ job object & fix tests Nov 21, 2024
@@ -54,8 +64,8 @@ def setup_once():
old_perform_job = Worker.perform_job

@ensure_integration_enabled(RqIntegration, old_perform_job)
def sentry_patched_perform_job(self, job, *args, **kwargs):
# type: (Any, Job, *Queue, **Any) -> bool
def sentry_patched_perform_job(self, job, queue, *args, **kwargs):
Copy link
Contributor Author

@sentrivana sentrivana Nov 21, 2024

Choose a reason for hiding this comment

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

Checked that the signature of perform_job is stable in the minimal supported version and in the current release. There are actually no args and kwargs, just job and queue, but let's do it like this for forwards compat.

@sentrivana sentrivana marked this pull request as ready for review November 21, 2024 15:28
@sentrivana
Copy link
Contributor Author

Forgot migration guide, added now

@sentrivana sentrivana merged commit 25d311e into potel-base Nov 21, 2024
43 of 120 checks passed
@sentrivana sentrivana deleted the ivana/potel/custom-sampling-context-rq branch November 21, 2024 15:51
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