Skip to content

ref(flags): allow OpenFeature integration to track a single client#3895

Closed
aliu39 wants to merge 6 commits into
masterfrom
aliu/openfeature-single-client
Closed

ref(flags): allow OpenFeature integration to track a single client#3895
aliu39 wants to merge 6 commits into
masterfrom
aliu/openfeature-single-client

Conversation

@aliu39

@aliu39 aliu39 commented Dec 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

All provider SDKs so far have the concept of a "client" class, which is used to connect to the server and query flags. Since clients can query multiple projects/environments by specifying a context, it is rare for an app to use >1 client.

I discussed with @billyvg and we want to stay consistent for all FF integrations, and focus on tracking one client. This means users should pass a client to the Integration, and we'll register hooks on that client, rather than globally.

Update 12/26:
For backwards compatibility, I think it's best to

  • keep LD as-is: optionally takes a client. If one is not given, then hook into the global singleton.
  • mirror this in OF, optionally taking in a client.

Updated setup code can be seen at: https://github.com/getsentry/sentry-docs/pull/12222/files

@aliu39
aliu39 requested a review from cmanallen December 23, 2024 22:38
@codecov

codecov Bot commented Dec 23, 2024 •
edited
Loading

Copy link
Copy Markdown

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
17189 4 17185 4771
View the top 2 failed tests by shortest run time 
tests.integrations.launchdarkly.test_launchdarkly test_launchdarkly_integration_threaded
Stack Traces | 0.104s run time 
.../integrations/launchdarkly/test_launchdarkly.py:104: in test_launchdarkly_integration_threaded
    assert len(events) == 3
E   AssertionError: assert 4 == 3
E    +  where 4 = len([{'breadcrumbs': {'values': []}, 'contexts': {'flags': {'values': [{'flag': 'hello', 'result': True}, {'flag': 'world', 'result': False}]}, 'runtime': {'build': '3.13.1 (main, Dec  4 2024, 06:21:55) [GCC 9.4.0]', 'name': 'CPython', 'version': '3.13.1'}, 'trace': {'parent_span_id': None, 'span_id': 'a7f4715ac316a810', 'trace_id': 'a878f46bec2e489d9780457570f62bb5'}}, 'environment': 'production', 'event_id': 'c0ce8f969dde4809ac3403603b9822a6', ...}, {'breadcrumbs': {'values': []}, 'contexts': {'flags': {'values': [{'flag': 'hello', 'result': True}, {'flag': 'other', 'result': False}]}, 'runtime': {'build': '3.13.1 (main, Dec  4 2024, 06:21:55) [GCC 9.4.0]', 'name': 'CPython', 'version': '3.13.1'}, 'trace': {'parent_span_id': None, 'span_id': 'a7f4715ac316a810', 'trace_id': 'a878f46bec2e489d9780457570f62bb5'}}, 'environment': 'production', 'event_id': '0780d1bb1638401aa162f1e77ab94a50', ...}, {'breadcrumbs': {'values': []}, 'contexts': {'runtime': {'build': '3.13.1 (main, Dec  4 2024, 06:21:55) [GCC 9.4.0]', 'name': 'CPython', 'version': '3.13.1'}, 'trace': {'parent_span_id': None, 'span_id': '90fe13aacddc1ad2', 'trace_id': '888602915d98427dade2f287b3bae04a'}}, 'environment': 'production', 'event_id': '4f4b914610ca40c7b00fc284595a52b8', ...}, {'breadcrumbs': {'values': []}, 'contexts': {'flags': {'values': [{'flag': 'hello', 'result': True}]}, 'runtime': {'build': '3.13.1 (main, Dec  4 2024, 06:21:55) [GCC 9.4.0]', 'name': 'CPython', 'version': '3.13.1'}, 'trace': {'parent_span_id': None, 'span_id': 'a7f4715ac316a810', 'trace_id': 'a878f46bec2e489d9780457570f62bb5'}}, 'environment': 'production', 'event_id': '7b7c3f0e9b8c4e8ea94dba59715ae1b1', ...}])
tests.test_basics test_classmethod_tracing
Stack Traces | 0.143s run time 
tests/test_basics.py:946: in test_classmethod_tracing
    assert fake_start_child.call_count == 1
E   AssertionError: assert 3 == 1
E    +  where 3 = <MagicMock name='mock.start_child' id='140384176632496'>.call_count
View the full list of 1 ❄️ flaky tests 
tests.test_basics test_staticmethod_tracing

Flake rate in main: 18.84% (Passed 56 times, Failed 13 times)

Stack Traces | 0.079s run time 
tests/test_basics.py:925: in test_staticmethod_tracing
    assert fake_start_child.call_count == 1
E   AssertionError: assert 2 == 1
E    +  where 2 = <MagicMock name='mock.start_child' id='140283105610336'>.call_count

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

This was referenced Dec 23, 2024
Merged
Closed
@aliu39 aliu39 changed the title ref(flags): change LaunchDarkly and OpenFeature integrations to track a single client instance ref(flags): allow OpenFeature integration to track a single client Dec 27, 2024
@aliu39 aliu39 mentioned this pull request Dec 27, 2024
Closed
2 tasks
@cmanallen

cmanallen commented Jan 3, 2025

Copy link
Copy Markdown
Member

I discussed with @billyvg and we want to stay consistent for all FF integrations, and focus on tracking one client. This means users should pass a client to the Integration, and we'll register hooks on that client, rather than globally.

This needs to be discussed as a team and documented. Writing code is the last step not the first. I'm closing this for now. Not to stop us from working on this forever but to prevent someone from merging code accidentally. We can re-open once we understand the implications of the decisions we're making.

@cmanallen cmanallen closed this Jan 3, 2025
antonpirker added a commit that referenced this pull request Jan 7, 2025
@aliu39 @antonpirker @cmanallen
feat(flags): add Unleash feature flagging integration (#3888)
c6a89d6 
Adds an integration for tracking flag evaluations from [Unleash](https://www.getunleash.io/) customers. 

Implementation
Unleash has no native support for evaluation hooks/listeners, unless the user opts in for each flag. Therefore we decided to patch the `is_enabled` and `get_variant` methods on the `UnleashClient` class. The methods are wrapped and the only side effect is writing to Sentry scope, so users shouldn't see any change in behavior. 

We patch one `UnleashClient` instance instead of the whole class. The reasons for this are described in
- #3895

It's also safer to not modify the unleash import.

References
- https://develop.sentry.dev/sdk/expected-features/#feature-flags
- https://docs.getunleash.io/reference/sdks/python for methods we're patching/wrapping

---------

Co-authored-by: Anton Pirker <anton.pirker@sentry.io>
Co-authored-by: Colton Allen <colton.allen@sentry.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmanallen cmanallen Awaiting requested review from cmanallen

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aliu39 @cmanallen