-
Notifications
You must be signed in to change notification settings - Fork 549
feat(spans): Record flag evaluations as span attributes #4280
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
Changes from 3 commits
f576126
e6e2d97
ef0dc37
dea1a94
0c71277
68a7b79
0763e45
8075658
7ac3a5c
b391602
aca8bde
7944811
19a0475
c99fdab
a7f6024
0934e31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
import sentry_sdk | ||
from sentry_sdk.integrations import DidNotEnable | ||
from sentry_sdk.integrations.launchdarkly import LaunchDarklyIntegration | ||
from sentry_sdk import start_span, start_transaction | ||
from tests.conftest import ApproxDict | ||
|
||
|
||
@pytest.mark.parametrize( | ||
|
@@ -202,3 +204,43 @@ def test_launchdarkly_integration_did_not_enable(monkeypatch): | |
monkeypatch.setattr(client, "is_initialized", lambda: False) | ||
with pytest.raises(DidNotEnable): | ||
LaunchDarklyIntegration(ld_client=client) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"use_global_client", | ||
(False, True), | ||
) | ||
def test_launchdarkly_span_integration( | ||
sentry_init, use_global_client, capture_events, uninstall_integration | ||
): | ||
td = TestData.data_source() | ||
td.update(td.flag("hello").variation_for_all(True)) | ||
td.update(td.flag("world").variation_for_all(True)) | ||
# Disable background requests as we aren't using a server. | ||
config = Config( | ||
"sdk-key", update_processor_class=td, diagnostic_opt_out=True, send_events=False | ||
) | ||
|
||
uninstall_integration(LaunchDarklyIntegration.identifier) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [question] why are we uninstalling the integration here? My guess is this would reduce the possibility of interactions between tests, but I this is my first time seeing this pattern in our tests so just want to check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is how I found the test suite so I'm following the pattern. I'm not sure why its necessary or what the implications are if its removed. I could look into this more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, perhaps I just haven't touched this part of the codebase – if that's the pattern you can disregard my comment. Maybe @antonpirker or @sentrivana would know why this is needed. |
||
if use_global_client: | ||
ldclient.set_config(config) | ||
sentry_init(traces_sample_rate=1, integrations=[LaunchDarklyIntegration()]) | ||
cmanallen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
client = ldclient.get() | ||
else: | ||
client = LDClient(config=config) | ||
sentry_init( | ||
traces_sample_rate=1, | ||
cmanallen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
integrations=[LaunchDarklyIntegration(ld_client=client)], | ||
) | ||
|
||
events = capture_events() | ||
|
||
with start_transaction(name="hi"): | ||
with start_span(op="foo", name="bar"): | ||
client.variation("hello", Context.create("my-org", "organization"), False) | ||
client.variation("other", Context.create("my-org", "organization"), False) | ||
|
||
(event,) = events | ||
assert event["spans"][0]["data"] == ApproxDict( | ||
{"flag.hello": True, "flag.other": False} | ||
cmanallen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) |
Uh oh!
There was an error while loading. Please reload this page.