Opened 16 months ago

Closed 9 months ago

#21427 closed enhancement (implemented)

allow to filter for tor_version

Reported by: cypherpunks Owned by: metrics-team
Priority: Medium Milestone: Onionoo-1.4.0
Component: Metrics/Onionoo Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

use case:
"show me all relays with contact X and tor version Z"

ideally it would be possible to search for

  • multiple versions
  • version ranges (older than, newer than)

https://lists.torproject.org/pipermail/tor-relays/2017-February/011921.html

Child Tickets

Change History (8)

comment:1 Changed 10 months ago by karsten

Status: newneeds_review

There, I implemented a simple version of this in my task-21427 branch, which would probably be specified on the website as follows:

diff --git a/website/src/main/resources/web/WEB-INF/onionoo.jsp b/website/src/main/resources/web/WEB-INF/onionoo.jsp
index 3fa4c5e..8ea310a 100644
--- a/website/src/main/resources/web/WEB-INF/onionoo.jsp
+++ b/website/src/main/resources/web/WEB-INF/onionoo.jsp
@@ -472,6 +472,16 @@ a family.
 </p>
 </li>
 
+<li>
+<b>version <span class="label label-primary">new</span></b>
+<p>
+Return only relays running a Tor version that starts with the
+parameter value <i>without</i> leading <code>"Tor"</code>.
+Searchers are case-insensitive.
+<span class="blue">Added on August 8, 2017.</span>
+</p>
+</li>
+
 </ul>
 
 <p>

Still untested (except for unit tests), and subject to change if we decide to implement some of the extensions suggested above.

comment:2 Changed 9 months ago by iwakeh

Milestone: Onionoo-1.4.0

Will have a minor protocol update b/c of removing beta fields. Therefore it makes sense to add this, too.

comment:3 Changed 9 months ago by iwakeh

Status: needs_reviewmerge_ready

All checks and tests pass now with the two small commits on this branch.

Could it be confusing to use use 'version' as search key name, because actually the 'platform' field of the details document is used and there are fields that contain the word 'version'?

Other than that: merge ready.

I'll add a ticket for documentation update.

comment:4 Changed 9 months ago by iwakeh

Documentation will be added in #23339.

comment:5 in reply to:  3 ; Changed 9 months ago by karsten

Replying to iwakeh:

All checks and tests pass now with the two small commits on this branch.

Thanks for the review and those two additional commits.

Could it be confusing to use use 'version' as search key name, because actually the 'platform' field of the details document is used and there are fields that contain the word 'version'?

Wait, we're only looking at the last part of the "platform" line:

+    this.version = version.substring(version.lastIndexOf(" ") + 1);

And which other fields are there that contain the word "version" which might confuse users?

Let's eliminate any confusions before merging. (But we'll also have to be careful not to call the parameter "platform" if we don't look at the entire platform string.)

Other than that: merge ready.

One thing, though: Should we briefly talk about possible extensions to support multiple versions or version ranges? How about "version=0.3.0,0.3.1" (all 0.3.0 and 0.3.1 versions), "version=0.2.4.21-0.2.4.29" (everything between 0.2.4.21 and 0.2.4.29), "version=0.2.9-" (0.2.9 or higher), "version=-0.2.9999" (everything up to 0.2.9999), etc.?

(Note that in particular the ranges might lead to surprises when used in two-digit version numbers; for example, "version=0.2.4.2-0.2.4.3" matches 0.2.4.2, 0.2.4.3, but also 0.2.4.21. I'm not sure whether we can do anything about that without getting into the business of parsing version strings.)

I'll add a ticket for documentation update.

(Thanks!)

comment:6 in reply to:  5 ; Changed 9 months ago by iwakeh

Replying to karsten:

Replying to iwakeh:

All checks and tests pass now with the two small commits on this branch.

Thanks for the review and those two additional commits.

Could it be confusing to use use 'version' as search key name, because actually the 'platform' field of the details document is used and there are fields that contain the word 'version'?

Wait, we're only looking at the last part of the "platform" line:

True. Maybe, use "platform_version", which is kind of long?

...

And which other fields are there that contain the word "version" which might confuse users?

In details we have "recommended_version". Maybe, rather keep 'version' (b/c it is short) and document it well.

Let's eliminate any confusions before merging. (But we'll also have to be careful not to call the parameter "platform" if we don't look at the entire platform string.)

Other than that: merge ready.

One thing, though: Should we briefly talk about possible extensions to support multiple versions or version ranges? How about "version=0.3.0,0.3.1" (all 0.3.0 and 0.3.1 versions), "version=0.2.4.21-0.2.4.29" (everything between 0.2.4.21 and 0.2.4.29), "version=0.2.9-" (0.2.9 or higher), "version=-0.2.9999" (everything up to 0.2.9999), etc.?

(Note that in particular the ranges might lead to surprises when used in two-digit version numbers; for example, "version=0.2.4.2-0.2.4.3" matches 0.2.4.2, 0.2.4.3, but also 0.2.4.21. I'm not sure whether we can do anything about that without getting into the business of parsing version strings.)

I would wait and not implement this, as there are not enough sub-versions that would justify such an effort. And, we offer a range search, because search=version:0.2 returns all versions 'in between'/'below', i.e., 0.2, 0.2.1.1, 0.2.2.1, etc.

I'll add a ticket for documentation update.

(Thanks!)

comment:7 in reply to:  6 Changed 9 months ago by karsten

Replying to iwakeh:

Replying to karsten:

Replying to iwakeh:

All checks and tests pass now with the two small commits on this branch.

Thanks for the review and those two additional commits.

Could it be confusing to use use 'version' as search key name, because actually the 'platform' field of the details document is used and there are fields that contain the word 'version'?

Wait, we're only looking at the last part of the "platform" line:

True. Maybe, use "platform_version", which is kind of long?

Wait, no, we're not using the "platform" line from server descriptors, but we're using the "v" line from consensuses, and that only contains the version. So, "version" is probably more accurate.

...

And which other fields are there that contain the word "version" which might confuse users?

In details we have "recommended_version". Maybe, rather keep 'version' (b/c it is short) and document it well.

Yes, I think that's good.

Let's eliminate any confusions before merging. (But we'll also have to be careful not to call the parameter "platform" if we don't look at the entire platform string.)

Other than that: merge ready.

One thing, though: Should we briefly talk about possible extensions to support multiple versions or version ranges? How about "version=0.3.0,0.3.1" (all 0.3.0 and 0.3.1 versions), "version=0.2.4.21-0.2.4.29" (everything between 0.2.4.21 and 0.2.4.29), "version=0.2.9-" (0.2.9 or higher), "version=-0.2.9999" (everything up to 0.2.9999), etc.?

(Note that in particular the ranges might lead to surprises when used in two-digit version numbers; for example, "version=0.2.4.2-0.2.4.3" matches 0.2.4.2, 0.2.4.3, but also 0.2.4.21. I'm not sure whether we can do anything about that without getting into the business of parsing version strings.)

I would wait and not implement this, as there are not enough sub-versions that would justify such an effort. And, we offer a range search, because search=version:0.2 returns all versions 'in between'/'below', i.e., 0.2, 0.2.1.1, 0.2.2.1, etc.

Agreed. Note, however, that if we decide to make such extensions in the future, they'll be backward-incompatible (requiring a new major version), because we're currently accepting searches like 0.1.2.3-alpha, which would conflict with the 0.1.2.3-0.1.2.6 notation. But, I'm fine with that.

Alright, will merge tomorrow unless there are objections.

I'll add a ticket for documentation update.

(Thanks!)

comment:8 Changed 9 months ago by karsten

Resolution: implemented
Status: merge_readyclosed

Merged together with a change log entry and protocol version bump. Closing.

Note: See TracTickets for help on using tickets.