feat: adds Superset usage metadata charts to Operator Dashboard #488

pomegranited · 2023-10-24T07:20:12Z

Adds a Superset tab to the Operator Dashboard with some introspection charts adapted from https://github.com/hometogo/hometogo-data-code-snippets. (I omitted charts tracking "naming conventions" and charts counting datasets over time, as these didn't seem interesting to the Aspects project.)

Closes: #427

Screenshots

Author Notes & Concerns

Fix filter warning shown when dashboard loads ("Error loading chart datasources; Filters may not work correctly.") 8d71dfc
Add chart descriptions f0db289
More charts?

adapted from https://github.com/hometogo/hometogo-data-code-snippets * Dashboard available to Admins and Operators. * Accesses superset mysql with a dedicated, read-only user * Imports database, datasets, charts, and dashboard

openedx-webhooks · 2023-10-24T07:20:18Z

Thanks for the pull request, @pomegranited! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

pomegranited · 2023-10-24T07:23:09Z

tutoraspects/templates/openedx-assets/assets/datasets/monitoring.superset_action_log.yaml

+offset: 0
+cache_timeout: null
+schema: {{SUPERSET_DB_METADATA_NAME}}
+sql: "select\nl.dttm as action_date,\ncase \n  when lower(l.action)\


note: original query used PostgreSQL's date_trunc('day', l.dttm) as action_date here.

MySQL's equivalent is extract(day from l.dttm) as action_date, but this caused errors in the charts, so I left the l.dttm datetime field as-is.

Ian2012 · 2023-10-24T13:52:10Z

The CI is failing because the final assets.yaml is too big. An contribution is expected from our team, I'll let you know to rebase

Was causing an error when loading the dashboard.

and deletes references to chart IDs -- they are not needed.

bmtcril

Looks good, thanks for taking this on! Just requesting the one change.

bmtcril · 2023-10-25T13:48:44Z

tutoraspects/templates/aspects/jobs/init/init-mysql.sh

+# Superset metadata user (read-only)
+mysql -u {{ MYSQL_ROOT_USERNAME }} --password="{{ MYSQL_ROOT_PASSWORD }}" --host "{{ MYSQL_HOST }}" --port {{ MYSQL_PORT }} -e "CREATE USER IF NOT EXISTS '{{ SUPERSET_DB_METADATA_USERNAME }}';"
+mysql -u {{ MYSQL_ROOT_USERNAME }} --password="{{ MYSQL_ROOT_PASSWORD }}" --host "{{ MYSQL_HOST }}" --port {{ MYSQL_PORT }} -e "ALTER USER '{{ SUPERSET_DB_METADATA_USERNAME }}'@'%' IDENTIFIED BY '{{ SUPERSET_DB_METADATA_PASSWORD }}';"
+mysql -u {{ MYSQL_ROOT_USERNAME }} --password="{{ MYSQL_ROOT_PASSWORD }}" --host "{{ MYSQL_HOST }}" --port {{ MYSQL_PORT }} -e "GRANT SELECT, EXECUTE ON {{ SUPERSET_DB_METADATA_NAME }}.* TO '{{ SUPERSET_DB_METADATA_USERNAME }}'@'%';"


Can we limit this to the tables we need? I'm somewhat concerned about things like access to the dbs table which stores credentials in plaintext.

bmtcril · 2023-10-25T13:51:27Z

@Ian2012 do you have a ticket or an ETA for the configmap changes? I'm likely going to run into this myself in my next PR.

Ian2012 · 2023-10-25T13:55:16Z

@bmtcril not sure, the plan is to build the image with the assets.yml file bundled in it and extend it via configmap

pomegranited · 2023-10-26T06:26:50Z

I've addressed your review @bmtcril . I can work on adding Clickhouse performance metrics next week, or as part of a separate PR, if you'd prefer.

bmtcril · 2023-10-26T11:54:51Z

I think a separate pr would be good. That could probably all go in the existing ClickHouse tab in the operator dash.

pomegranited · 2023-10-30T04:43:17Z

@bmtcril

I think a separate pr would be good. That could probably all go in the existing ClickHouse tab in the operator dash.

👍

Should these Superset charts go in the Operator Dashboard too? I can easily move the tabs over there if that's better.

bmtcril · 2023-11-01T17:46:36Z

@pomegranited If you can put these in the operator dash that would be great, it should help with keeping the permissions easy. Otherwise I think once you get past the current errors and rebase, this should be good to go.

…r dashboard and moves the assets to their new location in the repo after changes on latest main.

because we can't grant table-specific permissions for tables that do not exist yet. Step 1: unchanged Step 2: Grants SELECT access to superset metadata tables created by Superset migrations

pomegranited · 2023-11-02T03:43:14Z

Ready for another review @bmtcril @Ian2012 . I've:

moved the superset metadata charts into a new Superset tab on the Operator Dashboard
split the SQL GRANTs for the metadata tables into an init script that runs after Superset migrations (can't GRANT access to a table that doesn't exist)
redeployed from scratch on my local tutor dev.

bmtcril

Looks great, thanks for all the hard work and iteration!

openedx-webhooks · 2023-11-02T12:42:16Z

@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

feat: adds Superset Metadata

0586ee6

adapted from https://github.com/hometogo/hometogo-data-code-snippets * Dashboard available to Admins and Operators. * Accesses superset mysql with a dedicated, read-only user * Imports database, datasets, charts, and dashboard

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 24, 2023

pomegranited commented Oct 24, 2023

View reviewed changes

pomegranited requested a review from bmtcril October 24, 2023 07:25

itsjeyd added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Oct 24, 2023

Ian2012 approved these changes Oct 24, 2023

View reviewed changes

pomegranited added 3 commits October 25, 2023 16:43

fix: nulls unneeded query_context field in charts

8d71dfc

Was causing an error when loading the dashboard.

chore: removes unneeded filter

30506ff

and deletes references to chart IDs -- they are not needed.

feat: adds description field to Superset Usage charts

f0db289

bmtcril requested changes Oct 25, 2023

View reviewed changes

pomegranited changed the title ~~feat: adds Superset Metadata~~ feat: adds Superset Usage metadata dashboard and charts Oct 26, 2023

pomegranited added 2 commits October 26, 2023 16:52

feat: adds "Most-used Charts" to Superset Usage dashboard

cc86114

fix: grant SELECT access to a subset of superset metadata tables

60d0f37

pomegranited force-pushed the jill/superset-metadata branch from aabf13e to 60d0f37 Compare October 26, 2023 06:24

Merge branch 'main' into jill/superset-metadata

efe9de4

pomegranited added 3 commits November 2, 2023 08:18

Merge branch 'main' into jill/superset-metadata

399527e

refactor: moves Superset Metadata dashboard into a tab on the Operato…

9ad42bf

…r dashboard and moves the assets to their new location in the repo after changes on latest main.

refactor: splits mysql init into two steps

ba89145

because we can't grant table-specific permissions for tables that do not exist yet. Step 1: unchanged Step 2: Grants SELECT access to superset metadata tables created by Superset migrations

pomegranited changed the title ~~feat: adds Superset Usage metadata dashboard and charts~~ feat: adds Superset usage metadata charts to Operator Dashboard Nov 2, 2023

itsjeyd added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Nov 2, 2023

bmtcril approved these changes Nov 2, 2023

View reviewed changes

bmtcril merged commit a45f763 into main Nov 2, 2023
5 checks passed

bmtcril deleted the jill/superset-metadata branch November 2, 2023 12:42

itsjeyd removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Nov 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds Superset usage metadata charts to Operator Dashboard #488

feat: adds Superset usage metadata charts to Operator Dashboard #488

pomegranited commented Oct 24, 2023 •

edited

Loading

openedx-webhooks commented Oct 24, 2023 •

edited

Loading

pomegranited Oct 24, 2023

Ian2012 commented Oct 24, 2023 •

edited

Loading

bmtcril left a comment

bmtcril Oct 25, 2023

bmtcril commented Oct 25, 2023

Ian2012 commented Oct 25, 2023

pomegranited commented Oct 26, 2023

bmtcril commented Oct 26, 2023

pomegranited commented Oct 30, 2023

bmtcril commented Nov 1, 2023

pomegranited commented Nov 2, 2023

bmtcril left a comment

openedx-webhooks commented Nov 2, 2023

feat: adds Superset usage metadata charts to Operator Dashboard #488

feat: adds Superset usage metadata charts to Operator Dashboard #488

Conversation

pomegranited commented Oct 24, 2023 • edited Loading

openedx-webhooks commented Oct 24, 2023 • edited Loading

pomegranited Oct 24, 2023

Choose a reason for hiding this comment

Ian2012 commented Oct 24, 2023 • edited Loading

bmtcril left a comment

Choose a reason for hiding this comment

bmtcril Oct 25, 2023

Choose a reason for hiding this comment

bmtcril commented Oct 25, 2023

Ian2012 commented Oct 25, 2023

pomegranited commented Oct 26, 2023

bmtcril commented Oct 26, 2023

pomegranited commented Oct 30, 2023

bmtcril commented Nov 1, 2023

pomegranited commented Nov 2, 2023

bmtcril left a comment

Choose a reason for hiding this comment

openedx-webhooks commented Nov 2, 2023

pomegranited commented Oct 24, 2023 •

edited

Loading

openedx-webhooks commented Oct 24, 2023 •

edited

Loading

Ian2012 commented Oct 24, 2023 •

edited

Loading