chore(features) Expand documentation for sentry.features (#74943)

Expand the documentation for features.handler and features.manager.

Author: markstory

src/sentry/features/__init__.py

@@ -23,12 +23,17 @@
 #   NOTE: There is no currently established convention for features which do not
 #         fall under these scopes. Use your best judgment for these.
 #
+# - Decide if your feature needs to be exposed in API responses or not
+#   If your feature is not used in the frontend, it is recommended that you don't
+#   expose the feature flag as feature flag checks add latency and bloat to organization
+#   details and project details responses.
+#
 # - Set a default for your features.
 #
-#   Feature defaults are configured in the sentry.conf.server.SENTRY_FEATURES
-#   module variable. This is the DEFAULT value for a feature, the default may be
-#   overridden by the logic in the exposed feature.manager.FeatureManager
-#   instance. See the ``has`` method here for a detailed understanding of how
+#   Feature defaults are configured with the `default` parameter. Default values
+#   can also be defined in `settings.SENTRY_FEATURES`. Default values
+#   are used if no registered handler makes a decision for the feature.
+#   See the ``has`` method here for a detailed understanding of how
 #   the default values is overridden.
 #
 # - Use your feature.

src/sentry/features/handler.py

@@ -17,6 +17,16 @@
 
 
 class FeatureHandler:
+    """
+    Base class for defining custom logic for feature decisions.
+
+    Subclasses should implement `has` and contain the logic
+    necessary for the feature check.
+
+    Generally FeatureHandlers are only implemented in `getsentry.features`
+    as we don't programatically release features in self-hosted.
+    """
+
     features: MutableSet[str] = set()
 
     def __call__(self, feature: Feature, actor: User) -> bool | None:
@@ -50,13 +60,20 @@ def batch_has(
         raise NotImplementedError
 
 
-# It is generally better to extend BatchFeatureHandler if it is possible to do
-# the check with no more than the feature name, organization, and actor. If it
-# needs to unpack the Feature object and examine the flagged entity, extend
-# FeatureHandler directly.
+class BatchFeatureHandler(FeatureHandler):
+    """
+    Base class for feature handlers that apply to an organization
+    and an optional collection of `objects` (e.g. projects).
 
+    Subclasses are expected to implement `_check_for_batch` and perform a feature check
+    using only the organization.
+
+    It is generally better to extend BatchFeatureHandler if it is possible to do
+    the check with no more than the feature name, organization, and actor. If it
+    needs to unpack the Feature object and examine the flagged entity, extend
+    FeatureHandler instead.
+    """
 
-class BatchFeatureHandler(FeatureHandler):
     @abc.abstractmethod
     def _check_for_batch(
         self, feature_name: str, entity: Organization | User, actor: User

src/sentry/features/manager.py

@@ -77,23 +77,22 @@ def has_for_batch(
         actor: User | None = None,
     ) -> Mapping[Project, bool]:
         """
-        Determine in a batch if a feature is enabled.
+        Determine if a feature is enabled for a batch of objects.
 
-        This applies the same procedure as ``FeatureManager.has``, but with a
-        performance benefit where the objects being checked all belong to the
-        same organization. The objects are entities (e.g., projects) with the
-        common parent organization, as would be passed individually to ``has``.
+        This method enables checking a feature for an organization and a collection
+        of objects (e.g. projects). Feature handlers for batch checks are expected to
+        subclass `features.BatchFeatureHandler` and implement `has_for_batch` or
+        `_check_for_batch`. BatchFeatureHandlers will receive a `FeatureCheckBatch`
+        that contains the organization and object list.
 
         Feature handlers that depend only on organization attributes, and not
         on attributes of the individual objects being checked, will generally
-        perform faster if this method is used in preference to ``has``.
+        perform faster if this method is used in instead of ``has``.
 
-        The return value is a dictionary with the objects as keys. Each value
-        is what would be returned if the key were passed to ``has``.
+        The return value is a dictionary with the objects as keys, and each
+        value is the result of the feature check on the organization.
 
-        The entity handler can handle both batch project/organization
-        contexts so it'll likely have an entirely different implementation
-        of this functionality.
+        This method *does not* work with the `entity_handler`.
 
         >>> FeatureManager.has_for_batch('projects:feature', organization, [project1, project2], actor=request.user)
         """