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

AAP-20249: Admin Dashboard: [Feature flag] -M-A-G-I-C- Organizations #804

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class TelemetrySettingsView(RetrieveAPIView, CreateAPIView):
def get(self, request, *args, **kwargs):
logger.debug("Telemetry settings:: GET handler")

if not settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED:
if not settings.TELEMETRY_SCHEMA_2_ENABLED:
raise ServiceUnavailable()

exception = None
Expand Down Expand Up @@ -94,7 +94,7 @@ def get(self, request, *args, **kwargs):
def post(self, request, *args, **kwargs):
logger.debug("Telemetry settings:: POST handler")

if not settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED:
if not settings.TELEMETRY_SCHEMA_2_ENABLED:
raise ServiceUnavailable()

exception = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def test_permission_classes(self, *args):
for permission in required_permissions:
self.assertTrue(permission in view.permission_classes)

@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=False)
def test_get_settings_when_feature_disabled(self, *args):
self.client.force_authenticate(user=self.user)
r = self.client.get(reverse('telemetry_settings'))
Expand Down Expand Up @@ -80,7 +80,7 @@ def test_set_settings_authentication_error(self, *args):
r = self.client.post(reverse('telemetry_settings'))
self.assertEqual(r.status_code, HTTPStatus.UNAUTHORIZED)

@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=False)
def test_set_settings_when_feature_disabled(self, *args):
self.client.force_authenticate(user=self.user)
r = self.client.get(reverse('telemetry_settings'))
Expand Down
26 changes: 20 additions & 6 deletions ansible_wisdom/ai/api/utils/segment_analytics_telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from ai.api.utils.segment import base_send_segment_event, send_segment_event
from attr import asdict
from django.conf import settings
from organizations.models import Organization
from segment.analytics import Client
from users.models import User

Expand All @@ -29,16 +30,29 @@ def send_segment_analytics_event(event_enum, event_payload_supplier, user: User)
if not user.rh_user_has_seat:
logger.info("Skipping analytics telemetry event for users that has no seat.")
return
if not settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED:
logger.info("Analytics telemetry not active.")
return
organization = user.organization

organization: Organization = user.organization
if not organization:
logger.info("Analytics telemetry not active, because of no organization assigned for user.")
return

if organization.telemetry_opt_out:
logger.info("Analytics telemetry not active for organization.")
return
if not organization.is_schema_2_telemetry_override_enabled:
manstis marked this conversation as resolved.
Show resolved Hide resolved
logger.info("Analytics telemetry not active for organization.")
return
logger.info(
f'Organization {organization.id} telemetry settings overridden. '
f'Telemetry will be captured for Organization {organization.id}.'
)

if not settings.TELEMETRY_SCHEMA_2_ENABLED:
if not organization.is_schema_2_telemetry_override_enabled:
logger.info("Analytics telemetry not active.")
return
logger.info(
f'System telemetry settings overridden. '
f'Telemetry will be captured for Organization {organization.id}.'
)

event_name = event_enum.value
try:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from unittest.mock import Mock, patch

import ai.feature_flags as feature_flags
from ai.api.tests.test_views import WisdomServiceAPITestCaseBase
from ai.api.utils import segment_analytics_telemetry
from ai.api.utils.analytics_telemetry_model import (
Expand Down Expand Up @@ -76,9 +77,9 @@ def _assert_segment_analytics_error_sent(self, error, send_segment_event):

@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
def test_send_segment_analytics_event(self, base_send_segment_event):
analytics_event_object = AnalyticsProductFeedback("3", 123)
analytics_event_object = AnalyticsProductFeedback(3, 123)
payload = Mock(return_value=analytics_event_object)
send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user)
payload.assert_called()
Expand All @@ -91,7 +92,7 @@ def test_send_segment_analytics_event(self, base_send_segment_event):

@patch("ai.api.utils.segment_analytics_telemetry.send_segment_event")
@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
def test_send_segment_analytics_event_error_validation(self, send_segment_event):
payload = Mock(side_effect=ValueError)
send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user)
Expand All @@ -108,41 +109,70 @@ def test_send_segment_analytics_event_error_validation(self, send_segment_event)
)

@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
def test_send_segment_analytics_event_error_not_write_key(self, base_send_segment_event):
self._assert_event_not_sent(base_send_segment_event)

@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
def test_send_segment_analytics_event_error_user_no_seat(self, base_send_segment_event):
self.user.rh_user_has_seat = False
self._assert_event_not_sent(base_send_segment_event)

@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=False)
def test_send_segment_analytics_event_error_no_telemetry_enabled(self, base_send_segment_event):
self._assert_event_not_sent(base_send_segment_event)

@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
def test_send_segment_analytics_event_error_no_org(self, base_send_segment_event):
self.user.organization = None
self._assert_event_not_sent(base_send_segment_event)

@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
def test_send_segment_analytics_event_error_no_org_telemetry_enabled(
self, base_send_segment_event
):
self.user.organization.telemetry_opt_out = True
self._assert_event_not_sent(base_send_segment_event)

@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=False)
@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@patch.object(feature_flags, 'LDClient')
def test_send_segment_analytics_event_no_telemetry_enabled_with_override(
self, base_send_segment_event, LDClient
):
LDClient.return_value.variation.return_value = str(self.user.organization.id)
self._assert_event_sent(base_send_segment_event)

@override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey")
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=True)
@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event")
@patch.object(feature_flags, 'LDClient')
def test_send_segment_analytics_event_no_org_telemetry_enabled_with_override(
self, base_send_segment_event, LDClient
):
LDClient.return_value.variation.return_value = str(self.user.organization.id)
self.user.organization.telemetry_opt_out = True
self._assert_event_sent(base_send_segment_event)

def _assert_event_sent(self, base_send_segment_event):
payload = Mock(return_value=AnalyticsProductFeedback(3, 123))
send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user)
payload.assert_not_called()
base_send_segment_event.assert_called()

def _assert_event_not_sent(self, base_send_segment_event):
payload = Mock(return_value=AnalyticsProductFeedback("3", 123))
payload = Mock(return_value=AnalyticsProductFeedback(3, 123))
send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user)
payload.assert_not_called()
base_send_segment_event.assert_not_called()
10 changes: 7 additions & 3 deletions ansible_wisdom/ai/feature_flags.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
import os.path
from enum import Enum
from typing import Union

from django.conf import settings
from ldclient import Context
Expand All @@ -13,7 +14,10 @@


class WisdomFlags(str, Enum):
MODEL_NAME = "model_name" # model name selection
# model name selection
MODEL_NAME = "model_name"
# white list of org_id's for which Schema 2 Telemetry is enabled overriding all other settings.
SCHEMA_2_TELEMETRY_ORG_ID_WHITE_LIST = "schema_2_telemetry_org_id_white_list"


class FeatureFlags:
Expand Down Expand Up @@ -42,9 +46,9 @@ def __init__(self):
)
logger.info("feature flag client initialized")

def get(self, name: str, user: User, default: str):
def get(self, name: str, user: Union[User, None], default: str):
manstis marked this conversation as resolved.
Show resolved Hide resolved
if self.client:
if user.is_anonymous:
if not user or user.is_anonymous:
user_context = Context.builder("AnonymousUser").anonymous(True).build()
else:
groups = list(user.groups.values_list("name", flat=True))
Expand Down
11 changes: 11 additions & 0 deletions ansible_wisdom/ai/tests/test_feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,18 @@ def test_feature_flags_with_sdk_key(self, LDClient):

ff = feature_flags.FeatureFlags()
value = ff.get('model_name', self.user, 'default_value')
self.assert_test_feature_flags_with_sdk_key(LDClient, value)

@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch.object(feature_flags, 'LDClient')
def test_feature_flags_with_sdk_key_without_user(self, LDClient):
LDClient.return_value.variation.return_value = 'server:port:model_name:index'

ff = feature_flags.FeatureFlags()
value = ff.get('model_name', None, 'default_value')
self.assert_test_feature_flags_with_sdk_key(LDClient, value)

def assert_test_feature_flags_with_sdk_key(self, LDClient, value):
self.assertEqual(value, 'server:port:model_name:index')
LDClient.assert_called_once()
_, config_arg, kwargs = LDClient.mock_calls[0]
Expand Down
2 changes: 1 addition & 1 deletion ansible_wisdom/main/settings/development.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,4 @@
WCA_CLIENT_BACKEND_TYPE = os.getenv("WCA_CLIENT_BACKEND_TYPE", "dummy") # or wcaclient

# Enable Telemetry Opt In/Out settings in the Admin Portal
ADMIN_PORTAL_TELEMETRY_OPT_ENABLED = True
TELEMETRY_SCHEMA_2_ENABLED = True
2 changes: 1 addition & 1 deletion ansible_wisdom/main/settings/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@
SESSION_CACHE_ALIAS = "default"

# Disable Telemetry Opt In/Out settings in the Admin Portal
ADMIN_PORTAL_TELEMETRY_OPT_ENABLED = os.getenv('ADMIN_PORTAL_TELEMETRY_OPT_ENABLED', False)
TELEMETRY_SCHEMA_2_ENABLED = os.getenv('TELEMETRY_SCHEMA_2_ENABLED', False)
2 changes: 1 addition & 1 deletion ansible_wisdom/main/templates/console/console.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@
<!-- as it is only used for display purposes and servers no other value -->
<div id="user_name" hidden>{{user_name}}</div>
<!-- Flag whether Telemetry settings is supported -->
<div id="telemetry_opt_enabled" hidden>{{telemetry_opt_enabled}}</div>
<div id="telemetry_schema_2_enabled" hidden>{{telemetry_schema_2_enabled}}</div>
{% endblock content %}
8 changes: 4 additions & 4 deletions ansible_wisdom/main/tests/test_console_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,20 @@ def test_extra_data(self, *args):
context = response.context_data
self.assertEqual(context['user_name'], self.user.username)
self.assertEqual(context['rh_org_has_subscription'], self.user.rh_org_has_subscription)
self.assertTrue(context['telemetry_opt_enabled'])
self.assertTrue(context['telemetry_schema_2_enabled'])

def test_extra_data_telemetry_opt_in(self, *args):
self.client.force_authenticate(user=self.user)
response = self.client.get(reverse('console'))
self.assertIsInstance(response.context_data, dict)
context = response.context_data
# The default setting for tests is True
self.assertTrue(context['telemetry_opt_enabled'])
self.assertTrue(context['telemetry_schema_2_enabled'])

@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=False)
def test_extra_data_telemetry_opt_out(self, *args):
self.client.force_authenticate(user=self.user)
response = self.client.get(reverse('console'))
self.assertIsInstance(response.context_data, dict)
context = response.context_data
self.assertFalse(context['telemetry_opt_enabled'])
self.assertFalse(context['telemetry_schema_2_enabled'])
2 changes: 1 addition & 1 deletion ansible_wisdom/main/tests/test_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_telemetry_patterns_when_enabled(self):
)
self.assertEqual(1, len(patterns))

@override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False)
@override_settings(TELEMETRY_SCHEMA_2_ENABLED=False)
def test_telemetry_patterns_when_disabled(self):
reload(main.urls)
r = compile("api/v0/telemetry/")
Expand Down
2 changes: 1 addition & 1 deletion ansible_wisdom/main/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
path('console/<slug:slug1>/<slug:slug2>/', ConsoleView.as_view(), name='console'),
]

if settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED:
if settings.TELEMETRY_SCHEMA_2_ENABLED:
urlpatterns += [
path(
f'api/{WISDOM_API_VERSION}/telemetry/',
Expand Down
2 changes: 1 addition & 1 deletion ansible_wisdom/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ def get_context_data(self, **kwargs):
if self.request.user:
context["user_name"] = self.request.user.username
context["rh_org_has_subscription"] = self.request.user.rh_org_has_subscription
context["telemetry_opt_enabled"] = settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED
context["telemetry_schema_2_enabled"] = settings.TELEMETRY_SCHEMA_2_ENABLED
return context
19 changes: 19 additions & 0 deletions ansible_wisdom/organizations/models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
import logging

from django.conf import settings
from django.db import models
from django.utils.functional import cached_property

logger = logging.getLogger(__name__)


class Organization(models.Model):
id = models.IntegerField(primary_key=True)
telemetry_opt_out = models.BooleanField(default=False)

@cached_property
def is_schema_2_telemetry_override_enabled(self):
manstis marked this conversation as resolved.
Show resolved Hide resolved
if not settings.LAUNCHDARKLY_SDK_KEY:
return False

# Avoid circular dependency issue with lazy import
from ai.feature_flags import FeatureFlags, WisdomFlags

feature_flags = FeatureFlags()
org_ids: str = feature_flags.get(WisdomFlags.SCHEMA_2_TELEMETRY_ORG_ID_WHITE_LIST, None, '')
if len(org_ids) == 0:
return False

# Favor cast to str vs cast to int as we cannot
# guarantee Users defined numbers in LaunchDarkly
manstis marked this conversation as resolved.
Show resolved Hide resolved
return any(org_id == str(self.id) for org_id in org_ids.split(','))
43 changes: 43 additions & 0 deletions ansible_wisdom/organizations/tests/test_organizations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from unittest.mock import patch

import ai.feature_flags as feature_flags
from django.test import TestCase, override_settings
from organizations.models import Organization


class TestIsOrgLightspeedSubscriber(TestCase):
def test_org_with_telemetry_schema_2_enabled(self):
organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=True)[0]
self.assertFalse(organization.is_schema_2_telemetry_override_enabled)

def test_org_with_telemetry_schema_2_disabled(self):
organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0]
self.assertFalse(organization.is_schema_2_telemetry_override_enabled)

@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch.object(feature_flags, 'LDClient')
def test_org_with_telemetry_schema_2_disabled_with_feature_flags(self, LDClient):
LDClient.return_value.variation.return_value = ''
organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0]
self.assertFalse(organization.is_schema_2_telemetry_override_enabled)

@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch.object(feature_flags, 'LDClient')
def test_org_with_telemetry_schema_2_disabled_with_feature_flags_no_override(self, LDClient):
LDClient.return_value.variation.return_value = '999'
organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0]
self.assertFalse(organization.is_schema_2_telemetry_override_enabled)

@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch.object(feature_flags, 'LDClient')
def test_org_with_telemetry_schema_2_disabled_with_feature_flags_with_override(self, LDClient):
LDClient.return_value.variation.return_value = '123'
organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0]
self.assertTrue(organization.is_schema_2_telemetry_override_enabled)

@override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key')
@patch.object(feature_flags, 'LDClient')
def test_org_with_telemetry_schema_2_disabled_with_feature_flags_with_overrides(self, LDClient):
LDClient.return_value.variation.return_value = '000,999,123'
organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0]
self.assertTrue(organization.is_schema_2_telemetry_override_enabled)
Loading
Loading