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

Fix httpx tests #3810

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Fix httpx tests #3810

merged 1 commit into from
Nov 21, 2024

Conversation

sl0thentr0py
Copy link
Member

No description provided.

Copy link

codecov bot commented Nov 21, 2024

❌ 622 Tests Failed:

Tests completed Failed Passed Skipped
19684 622 19062 4424
View the top 3 failed tests by shortest run time
tests.integrations.opentelemetry.test_span_processor test_on_start_transaction
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'
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'
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.10.15....../x64/lib/python3.10/unittest/mock.py:1447: in __enter__
    original, local = self.get_original()
.../hostedtoolcache/Python/3.10.15....../x64/lib/python3.10/unittest/mock.py:1420: 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'

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

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.

Lgtm. Just one small q.

Comment on lines +303 to +304
assert "sentry-trace" in request_headers
assert request_headers["sentry-trace"] == sentry_sdk.get_traceparent()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why we were expecting to not propagate in the original version of this test case? The updated behavior is what I'd have expected to always be the case

Copy link
Member Author

Choose a reason for hiding this comment

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

was part of this change
#3070
and I believe this was an unintended side effect of that change...

but now we fixed the span without transaction problem another way with only_if_parent and this is the desired twp behaviour.

@sl0thentr0py sl0thentr0py merged commit ad96b24 into potel-base Nov 21, 2024
50 of 125 checks passed
@sl0thentr0py sl0thentr0py deleted the neel/potel/fix-httpx branch November 21, 2024 16:00
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