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

Add a context manager object and a method to call said object #357

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open
Changes from 21 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8d1ae15
quick context manager prototype
Oct 20, 2020
529f0a4
adding some defaults
Oct 20, 2020
5bac851
default param added
Oct 20, 2020
4eeed1a
assignment, duh
Oct 20, 2020
4714ba4
debugging
Oct 20, 2020
021661d
dont delete feature flag data if exception was raise
Oct 20, 2020
2401f78
some quick refactoring to flush data when getting out of scope
Oct 20, 2020
00e4967
adding user default params
Oct 20, 2020
9c51dcc
Utilize linked list structure for scope of context manager
Oct 28, 2020
642cc20
Create watch wrapper method and rename FeatureFlags
Nov 4, 2020
ea80652
Revert renamte of FeatureFlags
Nov 4, 2020
83b77b5
Generalize FeatureFlags context manager
Nov 6, 2020
9e4c34a
Remove unnecessary re-assignment
Nov 9, 2020
5b63fc0
Clear out scopes after report_exc_info is called
Nov 9, 2020
0483523
Merge branch 'master' into context-manager-ld
Nov 10, 2020
ae692bf
Clear scope on exit only
Nov 11, 2020
fa6cf69
Change reference to scope to tag and force extra_data to be a dict
Nov 11, 2020
e7c0b5c
Attach the tag to the exception on exit
Nov 12, 2020
8d89888
Refactor tags check
Nov 17, 2020
9ca6df6
Implement specific feature flags interface
Nov 17, 2020
48eaa57
change context manager object to take in dict
Nov 17, 2020
e5fb916
Make feature flags stack thread safe
Nov 17, 2020
5bae76e
Use tags instead of feature flags
Nov 18, 2020
7378835
Add check for attr
Nov 18, 2020
69b035e
Model feature flags with tags
Nov 20, 2020
430fd29
Fix typo in comment
Nov 24, 2020
5eb2da2
Add test cases for the feature_flag context manager
Nov 25, 2020
00c89ec
Use [0][0] to index mocked object
Nov 25, 2020
25045f9
Remove feature_flag.data.order
Nov 25, 2020
bcece26
use self.tags rather than self.tag
Nov 30, 2020
1153529
Initialize _tags in thread_local on append and value
Dec 1, 2020
d3894db
Move flattening to _LocalTags and rename _tags
Dec 4, 2020
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
55 changes: 55 additions & 0 deletions rollbar/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,23 @@ def wait(f=None):
return f()


def watch_feature_flag(key):
"""
Sets the feature flag payload for the code wrapped by this method.

key: key of the feature flag.

Usage:

with rollbar.watch_feature_flag('feature_flag_a'):
do_something_risky()
"""
feature_flag_obj = {'key': key}

return _FeatureFlagManager(feature_flag_obj)



class ApiException(Exception):
"""
This exception will be raised if there was a problem decoding the
Expand Down Expand Up @@ -702,6 +719,12 @@ def _report_exc_info(exc_info, request, extra_data, payload_data, level=None):
if extra_trace_data and not extra_data:
data['custom'] = extra_trace_data

# if there are feature flags attached to the exception, append that to the feature flags on
# singleton to create the full feature flags stack.
feature_flags = _feature_flags + getattr(exc_info[1], '_rollbar_feature_flags', [])[::-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be that we have repeated feature flags here? I am thinking if we should use set() for this, but in that case we need to add a FeatureVariation class or similar in order to generate a unique __hash__ since dict is not hashable.

Copy link
Author

@ajtran ajtran Nov 17, 2020

Choose a reason for hiding this comment

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

Hm. I think it's alright to have repeated feature flags here, right? Are you thinking of a case like:

with rollbar.watch_feature_flag('feature-a'):
  # do some stuff
  with rollbar.watch_feature_flag('feature-b'):
    # do some other stuff
    with rollbar.watch_feature_flag('feature-a'):
      # do some more stuff

where the exception happens in the deepest level of code

if feature_flags:
data['feature_flags'] = feature_flags

request = _get_actual_request(request)
_add_request_data(data, request)
_add_person_data(data, request)
Expand Down Expand Up @@ -788,6 +811,9 @@ def _report_message(message, level, request, extra_data, payload_data):
_add_lambda_context_data(data)
data['server'] = _build_server_data()

if _feature_flags:
data['feature_flags'] = _feature_flags

if payload_data:
data = dict_merge(data, payload_data, silence_errors=True)

Expand Down Expand Up @@ -1606,3 +1632,32 @@ def _wsgi_extract_user_ip(environ):
if real_ip:
return real_ip
return environ['REMOTE_ADDR']


_feature_flags = []
ajtran marked this conversation as resolved.
Show resolved Hide resolved


class _FeatureFlagManager(object):
"""
Context manager object that interfaces with the `_feature_flags` stack:

On enter, puts the feature flag object at top of the stack.
On exit, pops off the top element of the stack.
- If there is an exception, attach the feature flag object to the exception
for rebuilding of the `_feature_flags` stack before reporting.
"""
def __init__(self, feature_flag_obj):
self.feature_flag = feature_flag_obj

def __enter__(self):
_feature_flags.append(self.feature_flag)

def __exit__(self, exc_type, exc_value, traceback):

if exc_value:
if not hasattr(exc_value, '_rollbar_feature_flags'):
exc_value._rollbar_feature_flags = []

exc_value._rollbar_feature_flags.append(self.feature_flag)

_feature_flags.pop()