Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add local instance metrics page using new Dataverse metrics API #52

Merged
merged 36 commits into from
Jan 19, 2022

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Aug 31, 2020

Adds a page showing the metrics available from Dataverse PR 7178 for a single dataverse or sub-Dataverse within it. See IQSS/dataverse#7177 (comment) for a summary. These all include CSV download buttons (#51).

The existing metrics across dataverses (which I understand may be becoming obsolete), are moved to a /global subdirectory but otherwise work as before except for the addition of two graphs showing the distribution of Dataverse versions in use (#50). (Could be dropped if they are fully replaced.)

@qqmyers
Copy link
Member Author

qqmyers commented Aug 31, 2020

Images of the new local page from QDR's test instance. The tree widget at the top allows you to get the metrics for just that sub-Dataverse (note QDR calls these sub-Collections - that's configurable).
image
image
image

Conflicts:
	config.json.sample
	download.py
	update-all-installations-list.sh
@pdurbin pdurbin self-assigned this Oct 22, 2020
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new plots look great!

My first thought with all of these changes is that we need to bump the release number quite a bit due to incompatible changes. Currently, the next milestone is v0.2.8 but what should we change it to? v0.3.0? v1.0.0? Probably the latter. I saw somewhere a suggestion to move all this code to another repo. That's fine too, I suppose.

I was surprised to read "the existing metrics across dataverses (which I understand may be becoming obsolete)." Is this true? They are becoming obsolete?

The installation at https://dataverse.org/metrics is operated by @donsizemore so we'll definitely want to coordinate with him about when and how to roll out the "global" mode in dataverse-metrics 1.0. I assume "global" will continue to aggregate across any Dataverse 4.9+ installation. If this is not the case, please let us know.

With regard to the plots themselves, I like them but last time they when through several rounds of design review. Is this planned?

I'm a little confused about what's going on with CSV vs TSV. dataverse-metrics has always produced TSV files that the front end (d3plus) visualizes. Now the orginal code (now called "global") continues to use TSV but the new code uses CSV? I assume this is because new endpoints on the Dataverse side produce CSV. Is there also just a general preference for CSV? Is TSV weird?

I also left a technical comment or two in the review itself.

Overall, great work!

See also my review on the related pull request on the main Dataverse repo: IQSS/dataverse#7178

@@ -0,0 +1,2 @@
/*! jQuery v3.5.1 | (c) JS Foundation and other contributors | jquery.org/license */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we delete this file and just get jQuery from a CDN?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - the index.html file already does get it remote, so this must just be from earlier testing.

@pdurbin pdurbin removed their assignment Oct 22, 2020
@qqmyers
Copy link
Member Author

qqmyers commented Nov 30, 2021

~Merged. The conflicts were with the config.json.sample, download.py, and plots.js files that were moved to ./global and then changed in both places. The list of sites in global/config.json.sample is the same as Don had in the master branch. For download.py, 01074f2 shows the minor difference in error handling.
Note that the main thing that happened with the old code is that it moved to the ./global subdir. Otherwise, the only real changes is the addition of a plot of Dataverse versions found.

@qqmyers qqmyers removed their assignment Nov 30, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from IQSS Team - In Progress 💻 to QA 🔎✅ Nov 30, 2021
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't run the code but I think it looks ok.

@djbrooke I'm leaving you assigned for now. The two outstanding questions are:

  • Do we want any kind of design review?
  • When we release, should we bump the version to v.1.0.0? We can decide this later, of course.

@djbrooke
Copy link
Contributor

Thanks @qqmyers and @pdurbin. Sorry if I missed this, but is there an option to keep the view that we currently have on dataverse.org/metrics (screenshot attached) or will we move to this new view automatically? I'd just like to understand what the options are.

I'll defer to @TaniaSchlatter on the design review. We have https://www.iq.harvard.edu/metrics as well, so there may also be a question of where we want to spend time reviewing/changing views for the similar metrics.

Screen Shot 2021-11-30 at 3 55 00 PM

@qqmyers
Copy link
Member Author

qqmyers commented Nov 30, 2021

The current metrics app is still 'as is' in the new branch, but it has moved to the ./global subdirectory. So, the options to keep things as they are for a given deployment are: just don't update, or update and change the relevant paths/links/cron jobs to include /global (or use symlinks/change docroot etc. - basically adapt to the new subdir). One doesn't have to expose the local app at all.

The new app, taking over the base URL (e.g. what would be https://metrics.dataverse.org/ with the global one now at https://metrics.dataverse.org/global/ ) is different in a few ways:

  • it's only capturing metrics from the local app
  • it is simpler in that it is only html/javascript - no python that has to be run periodically by cron job
  • it allows per-sub-collection metrics with a tree widget at the top to select what you want to see.
  • if you have configured to run the global app as well, the local app can display a link to it at the bottom.

FWIW: The decision to move the global app to a subdir is ~arbitrary - the local app could go into a local subdir instead. The reason I did it this way because I assumed more sites would want to run the local app to get sub-collection metrics and only some of those would also set up the global app and it would be easier to have them ignore and/or delete the global subdir than to leave the global code with .py files etc. at the root dir.

@qqmyers
Copy link
Member Author

qqmyers commented Nov 30, 2021

Also - the demo from the community call should be around for this somewhere - that should be up-to-date with the PR.

@TaniaSchlatter
Copy link
Member

The design seems fine for QDR and others who do not have many collections. Collapsable side navigation would be a good choice. I'm not sure accordions are useful here. They are typically used for displaying sections of information when one is open and the rest are closed by default, to help a user focus on one thing at a time. We do not follow this best practice on Dataverse, but it's not too late to start ;) Charts could be broken up by text headers if all are displayed by default.

FWIW: all had 4 sites that don't respond/have ssl issues:
dataverse.bhp.org.bw,
dataverse.ileel.ufu.br,
dataverse.intracidacs.org,
dataverse.mpi-sws.org,

and 5 that are <v4.11:
dataspace.ust.hk,
dataverse.acg.maine.edu/dvn,
dataverse.ada.edu.au,
dataverse.ufabc.edu.br,
dvn.fudan.edu.cn/dataverse,
@djbrooke
Copy link
Contributor

I feel that this can be merged as is, as long as the metrics shown at dataverse.org/metrics can remain the same. Capacity permitting, we can revisit in the future in order to make any appropriate changes to the local app and the global app.

@djbrooke djbrooke removed their assignment Jan 19, 2022
@kcondon kcondon self-assigned this Jan 19, 2022
@kcondon kcondon merged commit 84d64fe into master Jan 19, 2022
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Jan 19, 2022
@kcondon kcondon deleted the newmetrics branch January 19, 2022 16:04
@pdurbin pdurbin added this to the v0.2.9 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants