Opened 14 months ago

Closed 7 months ago

#19553 closed defect (fixed)

Do not plot empty graphs

Reported by: twim Owned by: irl
Priority: Medium Milestone:
Component: Metrics/Atlas Version:
Severity: Normal Keywords:
Cc: iwakeh, irl@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Atlas does plot graphs even if there is no data. It should instead say that "No Data Available" for empty datasets.

Child Tickets

Attachments (1)

0001-Do-not-plot-empty-datasets.patch (4.1 KB) - added by twim 14 months ago.

Download all attachments as: .zip

Change History (24)

Changed 14 months ago by twim

comment:1 Changed 14 months ago by twim

Status: newneeds_review

comment:2 Changed 14 months ago by twim

Component: Metrics/Censorship analysisMetrics/Atlas

Misclicked.

comment:3 Changed 14 months ago by karsten

Taking one step back, and assuming you mean the "3 days" and "1 week" bandwidth graphs, the data for those graphs is not exactly missing. Onionoo simply doesn't include it, because it has become a strict subset of the "1 month" data when relays raised the reporting interval for bandwidth histories from 15 minutes to 4 hours. It should be easy to plot the most recent 3 days or most recent week of the "1 month" data. The data resolution would be lower, but most people wouldn't notice or care about that.

comment:4 Changed 14 months ago by twim

It should be easy to plot the most recent 3 days or most recent week of the "1 month" data. The data resolution would be lower, but most people wouldn't notice or care about that.

I don't see any reason to plot same data tree times on one page. For me it seems absolutely useless.
Also I have to point out that we shouldn't show any "No Data Available" image for lost data and just show nothing instead. But it's another issue.

comment:5 in reply to:  4 Changed 14 months ago by karsten

Replying to twim:

It should be easy to plot the most recent 3 days or most recent week of the "1 month" data. The data resolution would be lower, but most people wouldn't notice or care about that.

I don't see any reason to plot same data tree times on one page. For me it seems absolutely useless.

Technically, you're right that you wouldn't add any information by plotting those "3 days" and "1 week" graphs. But be sure to ask your users, I could imagine that a majority of those would find those graphs useful.

Also I have to point out that we shouldn't show any "No Data Available" image for lost data and just show nothing instead. But it's another issue.

Can't say, maybe ask your users, too.

comment:6 Changed 10 months ago by irl

karsten: Could we have a detailed_bandwidth endpoint from Onionoo?

I think even if that just added back the 1_week values, and we dropped the 3_days graph, that would be reasonable. Reconstructing the old datasets is going to mean slicing up the Onionoo results in Atlas and I'd rather be dealing with pure Onionoo responses with Atlas being a thin layer of presentation on top.

comment:7 Changed 10 months ago by irl

Status: needs_reviewneeds_information

comment:8 Changed 10 months ago by karsten

Cc: iwakeh added

irl: I'm not sure what you mean by "detailed_bandwidth endpoint". Do you mean a new document type similar to bandwidth documents but more detailed? If so, that would be a lot of effort, so maybe we can find a simpler solution.

iwakeh: How about we add back the 1_week graph with whatever detail we have, even if that duplicates information from the 1_month graph (which hurts a bit), and leave out the 3_days graph regardless of what data we have (which also hurts a tiny bit)? I do see irl's point of not wanting to add too much logic to Atlas, which would be logic that other clients would have to implement separately.

comment:9 Changed 10 months ago by irl

Yes, I meant a new document type. I can't remember the reasoning for removing these from the bandwidth document, which I why I suggested the addition of a new document type with more detail. If we added back 1_week to the document, and dropped 3 days from Atlas, I think that is very reasonable and an improvement on the current situation where those graphs do not have data.

comment:10 Changed 10 months ago by iwakeh

I think the "No data available" shouldn't be displayed, the graphs should just be omitted.

The other suggestions seem fine drop 3-day and include 1-week again. Maybe, we should add this topic to our UX meeting?

(I just wanted to take a look at the graphs, but there seems to be a problem as none have any data. What could that be)

comment:11 in reply to:  9 Changed 10 months ago by karsten

Replying to irl:

Yes, I meant a new document type. I can't remember the reasoning for removing these from the bandwidth document, which I why I suggested the addition of a new document type with more detail. If we added back 1_week to the document, and dropped 3 days from Atlas, I think that is very reasonable and an improvement on the current situation where those graphs do not have data.

The reason for removing graph data for 3_days and 1_week is that relays stopped providing bandwidth histories per 15 minutes and instead switched to bandwidth histories per 4 hours. And we already provide graph data on a detail level of 4 hours for an entire month, so there was no (obvious) reason to provide a data set for 1 week or 3 days with the same level of detail.

By the way, let's look at the numbers before deciding something: Of 10,651 bandwidth documents with graph data for 1_month, only 1,277 or 12% still have graph data for 1_week. We might just remove these graphs in Atlas regardless of whether there's data or not (as twim suggests above, when I didn't check the numbers...) and call it a day. That wouldn't even require any changes to Onionoo.

comment:12 in reply to:  10 ; Changed 10 months ago by karsten

Replying to iwakeh:

I think the "No data available" shouldn't be displayed, the graphs should just be omitted.

Plausible. After all, this is JavaScript, where we can do such magical things like making parts of the page disappear.

The other suggestions seem fine drop 3-day and include 1-week again. Maybe, we should add this topic to our UX meeting?

We could, if we cannot resolve this before the meeting.

(I just wanted to take a look at the graphs, but there seems to be a problem as none have any data. What could that be)

Good question. I can see graphs. Maybe a problem with security slider settings?

comment:13 in reply to:  12 Changed 10 months ago by iwakeh

Replying to karsten:

Replying to iwakeh:
...

Good question. I can see graphs. Maybe a problem with security slider settings?

You're right.
Security needs to be set to 'medium-high', enabling javascript alone doesn't work.
Maybe, that should be stated somewhere as a hint?

comment:14 in reply to:  12 Changed 10 months ago by twim

Replying to karsten:

Replying to iwakeh:

I think the "No data available" shouldn't be displayed, the graphs should just be omitted.

Plausible. After all, this is JavaScript, where we can do such magical things like making parts of the page disappear.

Agreed. Atlas should just draw what is available from Onionoo data. Just as simple as that.

Actually I've implemented hiding of empty graphs on top of my patch in this ticket. I don't quite remember details (it was long ago) but this may be useful. The corresponding code seems to be in commit 6c41495e80b99059b7605279a40be3f53a4845f6 at https://github.com/nogoegst/atlas.

comment:15 Changed 10 months ago by dcf

FYI, duplicate tickets from the past on the same topic:

#15453
Globe's bandwidth graphs over 3 days/1 week do not update for relays running Tor >= 2.5.11
#16184
Corrupt 3-day and month graphs for certain relays
#17326
Atlas not showing 3-day and 1-week graphs for bytes read/wriiten by node
#17828
no traffic graph for 3 days and for one weeks traffic


I agree with comment:10 that the useless graphs should just be removed, as a first step. If there are any users who depend on them, they've been SOL for a year and a half already.

In the longer term, it's silly that there are separate graphs for 1 month, 3 months, 1 year, 5 years, just because that's how Onionoo partitions the data. What if I want a graph of the last 6 months? Atlas should be able to make that for me by subsetting an Onionoo 1_year series—or even better, by combining a 1_month, 3_months, and 1_year series, for better resolution in the more recent dates.

comment:16 Changed 10 months ago by karsten

dcf: Thanks for digging out the duplicates. The issue is around for too long now, we should really fix it now (and close the remaining duplicate).

Regarding your longer-term idea, I totally agree that Atlas could be smarter there. But I also think that Onionoo could be smarter by providing data for 1_month, 3_months, and 1_year in a single data structure by using a smarter format. For example, rather than using a fixed-width data interval it could include a list of x coordinates for data points. That would allow providing more detail for more recent dates. And it would allow condensing long series of null y values into a single null value and kicking out the rest. There are surely plenty of ways to be smarter here. Part of me thinks we must be reinventing a wheel here, but that part is also too lazy to read up what wheels already exist. Anyway, I think we should first build the short-term solution by just removing the graphs that don't have any data. The other thing deserves its own ticket. :)

comment:17 Changed 7 months ago by arma

comment:18 Changed 7 months ago by irl

Status: needs_informationneeds_review

Following metrics team meeting today, we're going with the short term solution of removing these plots. Here is a patch for review:

https://gitweb.torproject.org/user/irl/atlas.git/patch/?id=49562bf3eb21833381ae4dc0341f4cfc540bdbb2

If this is good, I can merge and deploy.

comment:19 Changed 7 months ago by irl

Owner: set to RaBe
Status: needs_reviewassigned

comment:20 Changed 7 months ago by irl

Cc: irl@… added
Status: assignedneeds_review

comment:21 Changed 7 months ago by RaBe

Owner: changed from RaBe to irl
Status: needs_reviewassigned

comment:22 Changed 7 months ago by RaBe

Status: assignedmerge_ready

comment:23 Changed 7 months ago by irl

Resolution: fixed
Status: merge_readyclosed

Merged and deployed (:

Thanks for the review RaBe.

Note: See TracTickets for help on using tickets.