Opened 2 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#31407 closed enhancement (fixed)

Create a broker spec for metrics collection

Reported by: cohosh Owned by: cohosh
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords: tor-spec BugSmashFund
Cc: dcf, arlolra, cohosh, phw Actual Points:
Parent ID: Points:
Reviewer: phw Sponsor:

Description

We're in the process of creating a module for the collection of snowflake metrics (#21315, #29461). We need a better place to put the spec for the metrics data output by the snowflake broker than a comment in the source code (see https://trac.torproject.org/projects/tor/ticket/29461#comment:5)

A spec for the broker will also be useful to expand upon later to specify how the broker interacts with other pieces of either Snowflake or the Tor ecosystem in the case that the broker assumes more of the responsibilities of BridgeDB.

Child Tickets

Attachments (1)

0001-Added-snowflake-broker-metrics-spec.patch (2.6 KB) - added by cohosh 2 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 2 months ago by cohosh

Owner: set to cohosh
Status: newassigned

comment:2 Changed 2 months ago by cohosh

Status: assignedneeds_review

comment:3 Changed 2 months ago by dgoulet

Status: needs_reviewneeds_information

So this is specific to Snowflake if I read correctly. Or more specifically Snowflake -> CollecTor.

torspec.git repository usually has specification related to "How Tor works" that ultimately creates the network.

Most spec files are about how little-t tor components interacts with each other within the tor network. There is also all the exposed ABI/API from tor like the ControlPort spec.

So I'm wondering if this Snowflake -> Metrics spec should be in there. I would say that it might be better to put it directly into the Snowflake repository since this describes a data format exposed by Snowflake itself for Metrics to digest.

It feels more "natural" for someone looking for Snowflake's specs to go to the Snowflake repository no?

comment:4 Changed 2 months ago by cohosh

Makes sense to me, karsten had recommend torspec but I think the snowflake repository sounds fine. Afaik all we need is someplace other than the comments of the source code for the metrics team to reference.

comment:5 Changed 8 weeks ago by cohosh

Component: Core Tor/TorCircumvention/Snowflake
Status: needs_informationneeds_review

Here's an update to the spec and a proposal on where to put it.

comment:6 Changed 8 weeks ago by cohosh

Cc: dcf arlolra cohosh phw added

comment:8 Changed 7 weeks ago by phw

Reviewer: phw

comment:9 Changed 6 weeks ago by phw

Status: needs_reviewneeds_revision

I added a bunch of comments to the patch. The location looks good to me and is consistent with obfs4, which has its spec in doc/obfs4-spec.txt.

comment:10 Changed 5 weeks ago by cohosh

Status: needs_revisionneeds_review

Made suggested fixes: https://github.com/cohosh/snowflake/commit/2bf398af39bda0d2fa684f9d0571e9ea727e1ef9

I also renamed the folder to doc to match obfs4.

comment:11 Changed 5 weeks ago by phw

Status: needs_reviewmerge_ready

Looks good to me.

Small suggestion: revisions are easier to review if we address them in separate follow-up patches. Once everything looks good, we can squash the revision patches and end up with a single, well-formatted commit. Without separate revision patches, it's a bit tricky to see what changed between revisions.

comment:12 Changed 4 weeks ago by cohosh

Resolution: fixed
Status: merge_readyclosed

Merged in b29b49fc1c

comment:13 Changed 4 weeks ago by gaba

Keywords: BugSmashFund added
Note: See TracTickets for help on using tickets.