diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index 229d21aa2c0..4f06231bc65 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -359,7 +359,14 @@ function ExploreViewContainer(props) { } useComponentDidMount(() => { - props.actions.logEvent(LOG_ACTIONS_MOUNT_EXPLORER); + props.actions.logEvent( + LOG_ACTIONS_MOUNT_EXPLORER, + props.slice?.slice_id + ? { + slice_id: props.slice.slice_id, + } + : undefined, + ); }); useChangeEffect(tabId, (previous, current) => { diff --git a/superset-frontend/src/middleware/logger.test.js b/superset-frontend/src/middleware/logger.test.js index 1decb794094..629940b969e 100644 --- a/superset-frontend/src/middleware/logger.test.js +++ b/superset-frontend/src/middleware/logger.test.js @@ -23,11 +23,12 @@ import { LOG_EVENT } from 'src/logger/actions'; import { LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils'; describe('logger middleware', () => { + const dashboardId = 123; const next = sinon.spy(); const mockStore = { getState: () => ({ dashboardInfo: { - id: 1, + id: dashboardId, }, impressionId: 'impression_id', }), @@ -39,6 +40,7 @@ describe('logger middleware', () => { eventData: { key: 'value', start_offset: 100, + path: `/dashboard/${dashboardId}/`, }, }, }; @@ -94,6 +96,7 @@ describe('logger middleware', () => { source: 'dashboard', source_id: mockStore.getState().dashboardInfo.id, event_type: 'timing', + dashboard_id: mockStore.getState().dashboardInfo.id, }); expect(typeof events[0].ts).toBe('number'); diff --git a/superset-frontend/src/middleware/loggerMiddleware.js b/superset-frontend/src/middleware/loggerMiddleware.js index 5408edb62c3..ea4160f3654 100644 --- a/superset-frontend/src/middleware/loggerMiddleware.js +++ b/superset-frontend/src/middleware/loggerMiddleware.js @@ -78,19 +78,24 @@ const loggerMiddleware = store => next => action => { impression_id: impressionId, version: 'v2', }; - if (dashboardInfo?.id) { + const { eventName } = action.payload; + let { eventData = {} } = action.payload; + + if (dashboardInfo?.id && eventData.path?.includes('/dashboard/')) { logMetadata = { source: 'dashboard', source_id: dashboardInfo.id, + dashboard_id: dashboardInfo.id, ...logMetadata, }; } else if (explore?.slice) { logMetadata = { source: 'explore', source_id: explore.slice ? explore.slice.slice_id : 0, + ...(explore.slice.slice_id && { slice_id: explore.slice.slice_id }), ...logMetadata, }; - } else if (sqlLab) { + } else if (eventData.path?.includes('/sqllab/')) { const editor = sqlLab.queryEditors.find( ({ id }) => id === sqlLab.tabHistory.slice(-1)[0], ); @@ -102,8 +107,6 @@ const loggerMiddleware = store => next => action => { }; } - const { eventName } = action.payload; - let { eventData = {} } = action.payload; eventData = { ...logMetadata, ts: new Date().getTime(), diff --git a/superset/daos/log.py b/superset/daos/log.py index 002c3f23072..fe32ef81693 100644 --- a/superset/daos/log.py +++ b/superset/daos/log.py @@ -59,8 +59,14 @@ class LogDAO(BaseDAO[Log]): .group_by(Log.dashboard_id, Log.slice_id, Log.action) .filter( and_( - Log.action.in_(actions), + Log.action == "log", Log.user_id == user_id, + or_( + *{ + Log.json.contains(f'"event_name": "{action}"') + for action in actions + }, + ), # limit to one year of data to improve performance Log.dttm > one_year_ago, or_(Log.dashboard_id.isnot(None), Log.slice_id.isnot(None)), @@ -99,7 +105,16 @@ class LogDAO(BaseDAO[Log]): .outerjoin(Dashboard, Dashboard.id == Log.dashboard_id) .outerjoin(Slice, Slice.id == Log.slice_id) .filter(has_subject_title) - .filter(Log.action.in_(actions), Log.user_id == user_id) + .filter( + Log.action == "log", + Log.user_id == user_id, + or_( + *{ + Log.json.contains(f'"event_name": "{action}"') + for action in actions + }, + ), + ) .order_by(Log.dttm.desc()) .limit(page_size) .offset(page * page_size) diff --git a/superset/utils/log.py b/superset/utils/log.py index 71c55288330..70f6a1bda26 100644 --- a/superset/utils/log.py +++ b/superset/utils/log.py @@ -394,8 +394,8 @@ class DBEventLogger(AbstractEventLogger): log = Log( action=action, json=json_string, - dashboard_id=dashboard_id, - slice_id=slice_id, + dashboard_id=dashboard_id or record.get("dashboard_id"), + slice_id=slice_id or record.get("slice_id"), duration_ms=duration_ms, referrer=referrer, user_id=user_id, diff --git a/superset/views/log/api.py b/superset/views/log/api.py index 33f4ad51d45..ffa3a860060 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -130,7 +130,7 @@ class LogRestApi(LogMixin, BaseSupersetModelRestApi): """ args = kwargs["rison"] page, page_size = self._sanitize_page_args(*self._handle_page_args(args)) - actions = args.get("actions", ["explore", "dashboard"]) + actions = args.get("actions", ["mount_explorer", "mount_dashboard"]) distinct = args.get("distinct", True) payload = LogDAO.get_recent_activity(actions, distinct, page, page_size) diff --git a/tests/integration_tests/log_api_tests.py b/tests/integration_tests/log_api_tests.py index 0ed588d50be..ddec917cb4a 100644 --- a/tests/integration_tests/log_api_tests.py +++ b/tests/integration_tests/log_api_tests.py @@ -171,8 +171,18 @@ class TestLogApi(SupersetTestCase): admin_user = self.get_user("admin") self.login(ADMIN_USERNAME) dash = create_dashboard("dash_slug", "dash_title", "{}", []) - log1 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) - log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + log1 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) uri = f"api/v1/log/recent_activity/" # noqa: F541 rv = self.client.get(uri) @@ -187,7 +197,7 @@ class TestLogApi(SupersetTestCase): assert response == { "result": [ { - "action": "dashboard", + "action": "log", "item_type": "dashboard", "item_url": "/superset/dashboard/dash_slug/", "item_title": "dash_title", @@ -204,10 +214,20 @@ class TestLogApi(SupersetTestCase): admin_user = self.get_user("admin") self.login(ADMIN_USERNAME) dash = create_dashboard("dash_slug", "dash_title", "{}", []) - log = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) - log2 = self.insert_log("explore", admin_user, dashboard_id=dash.id) + log = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_explorer"}', + ) - arguments = {"actions": ["dashboard"]} + arguments = {"actions": ["mount_dashboard"]} uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" rv = self.client.get(uri) @@ -229,8 +249,18 @@ class TestLogApi(SupersetTestCase): admin_user = self.get_user("admin") self.login(ADMIN_USERNAME) dash = create_dashboard("dash_slug", "dash_title", "{}", []) - log = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) - log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) + log = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) arguments = {"distinct": False} uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" @@ -253,9 +283,24 @@ class TestLogApi(SupersetTestCase): dash = create_dashboard("dash_slug", "dash_title", "{}", []) dash2 = create_dashboard("dash2_slug", "dash2_title", "{}", []) dash3 = create_dashboard("dash3_slug", "dash3_title", "{}", []) - log = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) - log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash2.id) - log3 = self.insert_log("dashboard", admin_user, dashboard_id=dash3.id) + log = self.insert_log( + "log", + admin_user, + dashboard_id=dash.id, + json='{"event_name": "mount_dashboard"}', + ) + log2 = self.insert_log( + "log", + admin_user, + dashboard_id=dash2.id, + json='{"event_name": "mount_dashboard"}', + ) + log3 = self.insert_log( + "log", + admin_user, + dashboard_id=dash3.id, + json='{"event_name": "mount_dashboard"}', + ) now = datetime.now() log3.dttm = now @@ -271,7 +316,7 @@ class TestLogApi(SupersetTestCase): assert response == { "result": [ { - "action": "dashboard", + "action": "log", "item_type": "dashboard", "item_url": "/superset/dashboard/dash3_slug/", "item_title": "dash3_title", @@ -279,7 +324,7 @@ class TestLogApi(SupersetTestCase): "time_delta_humanized": ANY, }, { - "action": "dashboard", + "action": "log", "item_type": "dashboard", "item_url": "/superset/dashboard/dash2_slug/", "item_title": "dash2_title", @@ -306,7 +351,7 @@ class TestLogApi(SupersetTestCase): assert response == { "result": [ { - "action": "dashboard", + "action": "log", "item_type": "dashboard", "item_url": "/superset/dashboard/dash_slug/", "item_title": "dash_title",