feat: unpack payload into log function (apache#28521)

Author: mistercrunch

superset/config.py

@@ -79,6 +79,8 @@
 
 # By default will log events to the metadata database with `DBEventLogger`
 # Note that you can use `StdOutEventLogger` for debugging
+# Note that you can write your own event logger by extending `AbstractEventLogger`
+# https://github.com/apache/superset/blob/master/superset/utils/log.py
 EVENT_LOGGER = DBEventLogger()
 
 SUPERSET_LOG_VIEW = True

superset/utils/log.py

@@ -91,6 +91,31 @@ def get_logger_from_status(
 
 
 class AbstractEventLogger(ABC):
+    # Parameters that are passed under the `curated_payload` arg to the log method
+    curated_payload_params = {
+        "force",
+        "standalone",
+        "runAsync",
+        "json",
+        "csv",
+        "queryLimit",
+        "select_as_cta",
+    }
+    # Similarly, parameters that are passed under the `curated_form_data` arg
+    curated_form_data_params = {
+        "dashboardId",
+        "sliceId",
+        "viz_type",
+        "force",
+        "compare_lag",
+        "forecastPeriods",
+        "granularity_sqla",
+        "legendType",
+        "legendOrientation",
+        "show_legend",
+        "time_grain_sqla",
+    }
+
     def __call__(
         self,
         action: str,
@@ -120,6 +145,16 @@ def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
             **self.payload_override,
         )
 
+    @classmethod
+    def curate_payload(cls, payload: dict[str, Any]) -> dict[str, Any]:
+        """Curate payload to only include relevant keys/safe keys"""
+        return {k: v for k, v in payload.items() if k in cls.curated_payload_params}
+
+    @classmethod
+    def curate_form_data(cls, payload: dict[str, Any]) -> dict[str, Any]:
+        """Curate form_data to only include relevant keys/safe keys"""
+        return {k: v for k, v in payload.items() if k in cls.curated_form_data_params}
+
     @abstractmethod
     def log(  # pylint: disable=too-many-arguments
         self,
@@ -129,6 +164,8 @@ def log(  # pylint: disable=too-many-arguments
         duration_ms: int | None,
         slice_id: int | None,
         referrer: str | None,
+        curated_payload: dict[str, Any] | None,
+        curated_form_data: dict[str, Any] | None,
         *args: Any,
         **kwargs: Any,
     ) -> None:
@@ -180,6 +217,7 @@ def log_with_context(  # pylint: disable=too-many-locals,too-many-arguments
                 "database_driver": database.driver,
             }
 
+        form_data: dict[str, Any] = {}
         if "form_data" in payload:
             form_data, _ = get_form_data()
             payload["form_data"] = form_data
@@ -207,6 +245,8 @@ def log_with_context(  # pylint: disable=too-many-locals,too-many-arguments
             slice_id=slice_id,
             duration_ms=duration_ms,
             referrer=referrer,
+            curated_payload=self.curate_payload(payload),
+            curated_form_data=self.curate_form_data(form_data),
             **database_params,
         )
 
@@ -380,6 +420,8 @@ def log(  # pylint: disable=too-many-arguments
         duration_ms: int | None,
         slice_id: int | None,
         referrer: str | None,
+        curated_payload: dict[str, Any] | None,
+        curated_form_data: dict[str, Any] | None,
         *args: Any,
         **kwargs: Any,
     ) -> None:
@@ -390,6 +432,8 @@ def log(  # pylint: disable=too-many-arguments
             duration_ms=duration_ms,
             slice_id=slice_id,
             referrer=referrer,
+            curated_payload=curated_payload,
+            curated_form_data=curated_form_data,
             **kwargs,
         )
         print("StdOutEventLogger: ", data)

tests/integration_tests/event_logger_tests.py

@@ -141,14 +141,19 @@ def log(
             with logger(action="foo", engine="bar"):
                 pass
 
-        assert logger.records == [
-            {
-                "records": [{"path": "/", "engine": "bar"}],
-                "database_id": None,
-                "user_id": 2,
-                "duration": 15000,
-            }
-        ]
+        self.assertEquals(
+            logger.records,
+            [
+                {
+                    "records": [{"path": "/", "engine": "bar"}],
+                    "database_id": None,
+                    "user_id": 2,
+                    "duration": 15000,
+                    "curated_payload": {},
+                    "curated_form_data": {},
+                }
+            ],
+        )
 
     @patch("superset.utils.core.g", spec={})
     def test_context_manager_log_with_context(self, mock_g):
@@ -183,20 +188,25 @@ def log(
                 payload_override={"engine": "sqlite"},
             )
 
-        assert logger.records == [
-            {
-                "records": [
-                    {
-                        "path": "/",
-                        "object_ref": {"baz": "food"},
-                        "payload_override": {"engine": "sqlite"},
-                    }
-                ],
-                "database_id": None,
-                "user_id": 2,
-                "duration": 5558756000,
-            }
-        ]
+        self.assertEquals(
+            logger.records,
+            [
+                {
+                    "records": [
+                        {
+                            "path": "/",
+                            "object_ref": {"baz": "food"},
+                            "payload_override": {"engine": "sqlite"},
+                        }
+                    ],
+                    "database_id": None,
+                    "user_id": 2,
+                    "duration": 5558756000,
+                    "curated_payload": {},
+                    "curated_form_data": {},
+                }
+            ],
+        )
 
     @patch("superset.utils.core.g", spec={})
     def test_log_with_context_user_null(self, mock_g):