Opened 2 years ago

Closed 8 months ago

#27981 closed defect (fixed)

3 days young relay is not shown in 1-month graph, but in 6-month graph

Reported by: toralf Owned by: metrics-team
Priority: High Milestone:
Component: Metrics/Onionoo Version:
Severity: Normal Keywords:
Cc: arma, metrics-team Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I'd expect either no data at all or it should be visible in the 1-months graph too.

https://metrics.torproject.org/rs.html#details/63BF46A63F9C21FD315CD061B3EAA3EB05283A0A

Child Tickets

Change History (9)

comment:1 Changed 2 years ago by arma

Cc: arma added

comment:2 Changed 2 years ago by karsten

Cc: metrics-team added

Suggestion from discussing this with armadev and irl on IRC: Relay Search could display two sets of tabs, one for bandwidth graphs and one for weights graphs, and each set of tabs only contains the tabs that Onionoo provides data for.

comment:3 Changed 9 months ago by karsten

Component: Metrics/Relay SearchMetrics/Onionoo
Status: newneeds_review

The issue is that the "1 month" graph has a data resolution of 1 data point per 4 hours, but recent relays are reporting bandwidth data for 24 hour periods.

In theory, an Onionoo client could look at the "6 month" graph and only display the last month. But as mentioned on #28871 this is a rather data-centric view and not as developer-friendly as it could be.

Let's fix this in Onionoo by changing the data resolution of the "1 month" graph to 24 hours. The result will be that all relays and bridges will have a "1 month" graph for bandwidth again. That graph will be a little less detailed for older relays reporting higher-resolution data, but that seems acceptable to me.

Let's also consider providing the same graph periods for all document types, also to keep the graphing code in Onionoo clients simple. In particular:

  • Remove "3 days" and "1 week" bandwidth graphs with data resolutions of 15 minutes and 1 hour. These are only created for older relays, and Relay Search doesn't display them anyway. The data will be found in the "1 month" graph.
  • Add back the "1 month" graph for client documents. We removed them in 7.0 for redundancy reasons, but it turns out that we should also consider easy of client code here. Otherwise, users will next complain about missing clients graphs for "1 month". We could just add them back at the same time as we make these other changes.
  • Remove "1 week" uptime and weights graph. These are not displayed by Relay Search, and their data will be contained in the "1 month" graph.

This patch should be based on the #28871 patch to avoid more unexpected effects like missing graphs that are already contained in other graphs.

If, for whichever reason, we change our minds later, we can always revert this patch and generate the old graphs again. The data will not be lost.

All these changes require a major protocol update to 8.0.

Please review commit d447fac in my tasks-28871-and-27981 branch.

comment:4 Changed 9 months ago by irl

Status: needs_reviewmerge_ready

lgtm

comment:5 Changed 9 months ago by karsten

Great! Scheduled next major protocol version update for in 1 month from now.

comment:6 Changed 8 months ago by karsten

Priority: MediumHigh
Status: merge_readyneeds_review

Oops, looks like forgot to update unit tests. Please also review commit 340e26b. Setting priority to high, so that we can still deploy on Thursday. Thanks!

comment:7 Changed 8 months ago by irl

Status: needs_reviewmerge_ready

LGTM.

Aside, our tests fail (not these tests, but other Onionoo tests) if you run them in 1990. We're unlikely to do this in practice, but perhaps there is some deeper issue there we should fix.

All our Onionoo tests pass in 2050.

comment:8 Changed 8 months ago by karsten

Thanks for the review! I just filed #33360 for the 1990 issue, but I think we can leave the fix for after the 8.0 protocol update. Will release the graph changes and unit test fixes in two days. Thanks!

comment:9 Changed 8 months ago by karsten

Resolution: fixed
Status: merge_readyclosed

Merged, released, and deployed. Closing. Thanks!

Note: See TracTickets for help on using tickets.