You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2025/03/27 00:26:53 UTC
(superset) branch 4.1 updated: fix(log): Update recent_activity by event name (#32681)
This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch 4.1
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/4.1 by this push:
new d0376596a1 fix(log): Update recent_activity by event name (#32681)
d0376596a1 is described below
commit d0376596a10832586d33e5c74b1f7f9408e3213b
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Mon Mar 17 09:18:47 2025 -0700
fix(log): Update recent_activity by event name (#32681)
(cherry picked from commit 449f51aed56b3b1f91b04476b20033efc3610c8b)
---
.../components/ExploreViewContainer/index.jsx | 9 +-
superset-frontend/src/middleware/logger.test.js | 5 +-
.../src/middleware/loggerMiddleware.js | 11 +-
superset/daos/log.py | 19 ++-
superset/utils/log.py | 4 +-
superset/views/log/api.py | 2 +-
tests/integration_tests/log_api_tests.py | 162 +++++++++++++--------
7 files changed, 138 insertions(+), 74 deletions(-)
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index 229d21aa2c..4f06231bc6 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 d67c681caa..5d51649e00 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}/`,
},
},
};
@@ -91,6 +93,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 5408edb62c..ea4160f365 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 002c3f2307..fe32ef8169 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 71c5528833..70f6a1bda2 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 33f4ad51d4..ffa3a86006 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 fae09754aa..0666f4e156 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)
@@ -184,21 +194,18 @@ class TestLogApi(SupersetTestCase):
db.session.delete(dash)
db.session.commit()
- self.assertEqual(
- response,
- {
- "result": [
- {
- "action": "dashboard",
- "item_type": "dashboard",
- "item_url": "/superset/dashboard/dash_slug/",
- "item_title": "dash_title",
- "time": ANY,
- "time_delta_humanized": ANY,
- }
- ]
- },
- )
+ assert response == {
+ "result": [
+ {
+ "action": "log",
+ "item_type": "dashboard",
+ "item_url": "/superset/dashboard/dash_slug/",
+ "item_title": "dash_title",
+ "time": ANY,
+ "time_delta_humanized": ANY,
+ }
+ ]
+ }
def test_get_recent_activity_actions_filter(self):
"""
@@ -207,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)
@@ -232,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)}"
@@ -256,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,29 +313,26 @@ class TestLogApi(SupersetTestCase):
self.assertEqual(rv.status_code, 200)
response = json.loads(rv.data.decode("utf-8"))
- self.assertEqual(
- response,
- {
- "result": [
- {
- "action": "dashboard",
- "item_type": "dashboard",
- "item_url": "/superset/dashboard/dash3_slug/",
- "item_title": "dash3_title",
- "time": ANY,
- "time_delta_humanized": ANY,
- },
- {
- "action": "dashboard",
- "item_type": "dashboard",
- "item_url": "/superset/dashboard/dash2_slug/",
- "item_title": "dash2_title",
- "time": ANY,
- "time_delta_humanized": ANY,
- },
- ]
- },
- )
+ assert response == {
+ "result": [
+ {
+ "action": "log",
+ "item_type": "dashboard",
+ "item_url": "/superset/dashboard/dash3_slug/",
+ "item_title": "dash3_title",
+ "time": ANY,
+ "time_delta_humanized": ANY,
+ },
+ {
+ "action": "log",
+ "item_type": "dashboard",
+ "item_url": "/superset/dashboard/dash2_slug/",
+ "item_title": "dash2_title",
+ "time": ANY,
+ "time_delta_humanized": ANY,
+ },
+ ]
+ }
arguments = {"page": 1, "page_size": 2}
uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}"
@@ -309,18 +348,15 @@ class TestLogApi(SupersetTestCase):
self.assertEqual(rv.status_code, 200)
response = json.loads(rv.data.decode("utf-8"))
- self.assertEqual(
- response,
- {
- "result": [
- {
- "action": "dashboard",
- "item_type": "dashboard",
- "item_url": "/superset/dashboard/dash_slug/",
- "item_title": "dash_title",
- "time": ANY,
- "time_delta_humanized": ANY,
- }
- ]
- },
- )
+ assert response == {
+ "result": [
+ {
+ "action": "log",
+ "item_type": "dashboard",
+ "item_url": "/superset/dashboard/dash_slug/",
+ "item_title": "dash_title",
+ "time": ANY,
+ "time_delta_humanized": ANY,
+ }
+ ]
+ }