Include currently running software versions in responses (collector.tp.o, onionoo.tp.o) and on the website (metrics.tp.o)

Should metrics web sites (metrics.tp.o, collector.tp.o, onionoo.tp.o) display currently running version?

For example, the two recent releases of Onionoo and CollecTor contained important fixes and for Onionoo a protocol change. The fix for collector was about the quaity of exit-list descriptors. A version information about the software running placed in some prominent place on the respective web sites would be helpful for users to determine, if fixes are running on the mirror.

1. Where should the version info be placed?
2. How to automate this? (Somewhere in the build, maybe include the version from a textfile generated when packaging ...)

changed milestone to %CollecTor 1.4.0

added component::metrics/collector metrics-2017 milestone::CollecTor 1.4.0 owner::metrics-team priority::medium resolution::implemented severity::normal status::closed type::enhancement labels

Trac:
Cc: N/A to RaBe

I like the idea! How about we place some version string in the page footer, in a way that doesn't look to awkward and technical to users?

Yes, a footer with version is fine.

Another topic: shouldn't there be the word "mirror" somewhere to distinguish systems operated by other from the main Tor instances?

So, if we go ahead with #21551 (moved) and merge CollecTor and Onionoo sites into the Metrics website, we'll have to adapt this ticket a bit. In that case it would still make sense for CollecTor and Onionoo to reveal the version they're running, just not on an HTML page:

• CollecTor could include an "implementation_version" field in its /index/index.json* files. That field could contain the same value that we're including as Implementation-Version in the build file, e.g., "1.1.2-ef96cf8" for the latest release.

• Onionoo could likewise include an "implementation_version" in the header of all its responses. It already contains the protocol "version", like "4.0", but including something like "1.2.0-4717532" might turn out to be really helpful for debugging. We should be careful with modifying the required "version" field though.

• Metrics would not include a version number if we move forward with #21551 (moved), because we would continue with not putting out releases for it. But it might still make sense to put the Git commit (short, 7 characters) in the page footer. After all, the graphs will remain part of Metrics, and knowing which exact version is deployed might make debugging easier. But that's really something that the average user wouldn't care about, so we should make it tiny. Or we could even put it as comment into the page sources, except that we'd be the only ones knowing where to find it. Though maybe that's okay?

• The new service discussed in #21551 (moved) for providing .csv files should include an implementation version, too. Maybe it'll contain a machine-readable index file of some sort, too.

The mirror topic depends a lot of #21551 (moved). We should probably move forward with that discussion first.

Replying to karsten:

So, if we go ahead with #21551 (moved) and merge CollecTor and Onionoo sites into the Metrics website, we'll have to adapt this ticket a bit. In that case it would still make sense for CollecTor and Onionoo to reveal the version they're running, just not on an HTML page:

That's true.

• CollecTor could include an "implementation_version" field in its /index/index.json* files. That field could contain the same value that we're including as Implementation-Version in the build file, e.g., "1.1.2-ef96cf8" for the latest release.

Good place; and the json could even be used to include it into a web page (I assume), if necessary or maybe the non-standard collector operators might make use of it, too.

• Onionoo could likewise include an "implementation_version" in the header of all its responses. It already contains the protocol "version", like "4.0", but including something like "1.2.0-4717532" might turn out to be really helpful for debugging. We should be careful with modifying the required "version" field though.

Right, 'version' should not be changed as it is a machine detectable protocol field. Adding 'implementation_version' is fine. Maybe make it optional to prevent people from relying on it instead of 'version'?

• Metrics would not include a version number if we move forward with #21551 (moved), because we would continue with not putting out releases for it. But it might still make sense to put the Git commit (short, 7 characters) in the page footer. After all, the graphs will remain part of Metrics, and knowing which exact version is deployed might make debugging easier. But that's really something that the average user wouldn't care about, so we should make it tiny. Or we could even put it as comment into the page sources, except that we'd be the only ones knowing where to find it. Though maybe that's okay?

It might easy to add the version to the graphs' png meta data?

• The new service discussed in #21551 (moved) for providing .csv files should include an implementation version, too. Maybe it'll contain a machine-readable index file of some sort, too.

The mirror topic depends a lot of #21551 (moved). We should probably move forward with that discussion first.

That's right. Just added things I didn't want to forget in the mean time.

Rephrase summary a bit to make it more actionable and also to reflect that neither CollecTor nor Onionoo have their own websites anymore.

Trac:
Summary: Should metrics web sites (metrics.tp.o, collector.tp.o, onionoo.tp.o) display currently running software version? to Include currently running software versions in responses (collector.tp.o, onionoo.tp.o) and on the website (metrics.tp.o)

Trac:
Keywords: N/A deleted, metrics-2018 added

Trac:
Keywords: metrics-2018 deleted, metrics-2017 added

iwakeh, maybe this is obvious, but I don‘t see it: what‘s the best way to access an implementation version string, that is generated during build time, at runtime? I guess we‘ll write it somewhere during the build process. But how do we learn about that when writing actual index.json files or query responses? I‘d want to avoid coming up with a hack here, so maybe there‘s a clean way to do it? Thanks!

Here is the example process for index.json files:

1. The patch for metrics-base creates a build.properties file and adds it to the jar. This will currently happen to all products using metrics-base, which is fine and even useful not have to look at the manifest to find out about the revision used.

2. Index json is provided by metrics-lib. This patch adds the revision field to IndexNode and sets it to "not-available" in case there is a problem determining the revision, which is also helpful for testing.

3. CollecTor's index module is patched to read the properties file from its jar and add the revision to the IndexNode.

Examples for the resulting index.json

{"index_created":"2016-08-22 10:48","build_revision":"not-available","path":"htt...

{"index_created":"2016-08-22 10:48","build_revision":"abcdef1","path":"htt...

(Once 1. is merged the Onionoo patch is very similar to the CollecTor patch and Onionoo doesn't need 2. The patches 1. and 2. have to be merged before 3., but their merge order doesn't matter.)

Please review the above changes.

Trac:
Status: new to needs_review

Adding this to the next CollecTor milestones.

All other like Onionoo or even Metrics-web should have a new ticket for the implementation.

Trac:
Milestone: N/A to CollecTor 1.4.0

Thanks for starting this!

The metrics-base patch looks good, merged.

However, I'd want us to make one change: we should just leave out the "build_revision" field from index.json if no revision is available. The "not-available" string only makes the protocol a bit more complex, because we need to explain what it means, but it doesn't add any information. And we need to check for null anyway, because that field does not exist until now. I'm happy to make that change.

But there's another issue that I don't know how to fix: when I apply the patches for 2 and 3 and start a test CollecTor instance, it includes the revision of metrics-lib rather than its own revision. Not sure why, possibly because there are two build.properties files now, one from metrics-lib and one from CollecTor. Hmm, do you have a fix for that?

Thanks!

Trac:
Status: needs_review to needs_revision

Replying to karsten:

Thanks for starting this!

The metrics-base patch looks good, merged.

However, I'd want us to make one change: we should just leave out the "build_revision" field from index.json if no revision is available. The "not-available" string only makes the protocol a bit more complex, because we need to explain what it means, but it doesn't add any information. And we need to check for null anyway, because that field does not exist until now. I'm happy to make that change.

But there's another issue that I don't know how to fix: when I apply the patches for 2 and 3 and start a test CollecTor instance, it includes the revision of metrics-lib rather than its own revision. Not sure why, possibly because there are two build.properties files now, one from metrics-lib and one from CollecTor. Hmm, do you have a fix for that?

Thanks for checking thoroughly. The wrong revision is a good catch! There are now fixup commits on all three branches:

• metrics-base now creates a properties file with the product name and writes a property that also contains the product name. Thus, making sure everything is unique.

• metrics-lib now omits unavailable revisions (cf. test data) in index.json files.

• collector has to adapt to the new naming and hand 'null' to IndexNode when it cannot determine the revision.

With the product dependent build revision properties file name we even know which metrics-lib version was used for creating the jar, i.e., it contains the file metrics-lib.buildrevision.properties.

Please review again, I think all points above should be resolved.

Trac:
Status: needs_revision to needs_review

Looks better now!

The metrics-base patch is merged, and I made some minor tweaks in this metrics-lib patch and this collector patch. Please take a look if these branches are now good to go!

Finally, we'll want to document this new field on the CollecTor page. How about: "build_revision": Git revision used to create this file, which will be omitted if unknown.

I don't see my fixup commits in your branch (which is a little confusing) and some of the changes you made were in there. But, the changes look ok; except that I prefer to explicitly set the fallback value 'null' instead of relying on the default in getProperty, because this states that the decision was made consciously and not by accident -- some way to document in code.

Regarding the changelog of metrics-lib: This needs to be clearer, like "the Git revision supplied by the calling software" or similar. It should make clear that this is a revision of the software using the API and that metrics-lib doesn't interfere with it. metrics-lib creates the files, but the git-revision supplied is not metrics-lib.

The same for CollecTor, maybe: //Git revision of the CollecTor instance's software used to create this file, which will be omitted if unknown.//

(Wondering if the metrics-lib revision should be appended to the one given by CollecTor as it does have some influence on the files, i.e., "build_revision":"abcdfe7-zyxabc5"? It would be easy, but of course we would have to discuss and explain what happens when there is only one of these available)

Replying to iwakeh:

I don't see my fixup commits in your branch (which is a little confusing) and some of the changes you made were in there.

Ah, the reason is that I already made very similar changes when trying out your earlier branches, so I manually applied changes from your most recent commits rather than rebasing my remaining changes on your fixup commit. I figured it's not that much of an issue, because these will be squashed very soon anyway, but I didn't mean to confuse you.

But, the changes look ok; except that I prefer to explicitly set the fallback value 'null' instead of relying on the default in getProperty, because this states that the decision was made consciously and not by accident -- some way to document in code.

Makes sense, changed.

Regarding the changelog of metrics-lib: This needs to be clearer, like "the Git revision supplied by the calling software" or similar. It should make clear that this is a revision of the software using the API and that metrics-lib doesn't interfere with it. metrics-lib creates the files, but the git-revision supplied is not metrics-lib.

Changed to:

   - Add new optional "build_revision" field to index.json with the
     Git revision supplied by the calling software.

The same for CollecTor, maybe: //Git revision of the CollecTor instance's software used to create this file, which will be omitted if unknown.//

Changed to:

   - Add new optional "build_revision" field to index.json with the
     Git revision of the CollecTor instance's software used to create
     this file, which will be omitted if unknown.

(I pushed both changes as fixup commits to my existing task-21414 branches.)

(Wondering if the metrics-lib revision should be appended to the one given by CollecTor as it does have some influence on the files, i.e., "build_revision":"abcdfe7-zyxabc5"? It would be easy, but of course we would have to discuss and explain what happens when there is only one of these available)

Let's start with the CollecTor's Git revision, and if we figure out that we also need to provide more details about the metrics-lib version or maybe even other dependent libraries, let's add that. In theory, library versions are determined by the Git revision, unless we use dev versions, which we probably shouldn't do.

Speaking of! :) Should we put out metrics-lib version 2.2.0 today? I could provide a pre-release tarball in ~2 hours.

Replying to karsten:

... (I pushed both changes as fixup commits to my existing task-21414 branches.)

Thanks! All changes above look fine.

(Wondering if the metrics-lib revision should be appended to the one given by CollecTor as it does have some influence on the files, i.e., "build_revision":"abcdfe7-zyxabc5"? It would be easy, but of course we would have to discuss and explain what happens when there is only one of these available)

Let's start with the CollecTor's Git revision, and if we figure out that we also need to provide more details about the metrics-lib version or maybe even other dependent libraries, let's add that. In theory, library versions are determined by the Git revision, unless we use dev versions, which we probably shouldn't do.

Yes, CollecTor's revision is sufficient.

Speaking of! :) Should we put out metrics-lib version 2.2.0 today? I could provide a pre-release tarball in ~2 hours.

Oh, I didn't expect any urgent question here in the ticket :-) Well, I don't think it is that urgent and only a change that will be visible after a CollecTor and/or Onionoo release (I'll add a ticket for the Onionoo change). Maybe collect some more changes for the next release?

The related Onionoo ticket #23778 (moved).

Alright, I squashed and merged the metrics-lib change.

Regarding not release metrics-lib and collecting some more changes, I'm fine with that. But I'd rather not want to merge the CollecTor change yet, because it depends on a metrics-lib dev version. I'd rather want to avoid a situation where we cannot release CollecTor without releasing metrics-lib first. Even more so I want to avoid a situation where we forget to release metrics-lib first and ship a CollecTor release that depends on a metrics-lib dev version.

Here's what remains to be done:

• Put out a metrics-lib release.
• Update my CollecTor task-21414 branch (commit 66ff5dc) to depend on that metrics-lib release.
• Squash that branch and update the commit message to not contain the remark about the metrics-lib dev version anymore.
• Rebase and merge to master.

Trac:
Status: needs_review to merge_ready
Priority: Low to Medium
Cc: RaBe to N/A
Component: Metrics to Metrics/CollecTor

We're about to release metrics-lib, CollecTor, and Onionoo. Once they're out, we'll have to update metrics-web and document the new fields. Please review commit 3437efe in my task-21414 branch.

Trac:
Status: merge_ready to needs_review

Onionoo has a protocol change, so it needs to be listed:

<li><strong>4.2</strong>: Added "build_revision" field on October xx, 2017.</li>

Trac:
Status: needs_review to needs_revision

Good point. Please review this fixup commit.

Trac:
Status: needs_revision to needs_review

October 10

Trac:
Status: needs_review to needs_revision

released and done, closing.

Thanks!

Trac:
Resolution: N/A to implemented
Status: needs_revision to closed

closed

mentioned in issue #23285 (moved)

mentioned in issue #23778 (moved)