control: Add a key to GETINFO to fetch the circuit onion handshake rephist values

Basically something that would export the rephist value of the onion handshakes:

onion_handshakes_assigned
onion_handshakes_requested

The heartbeat uses rep_hist_log_circuit_handshake_stats() to log the ntor and tap stats. I want those from the GETINFO command in order to be able to graph and keep stats over time of those values.

changed milestone to %Tor: unspecified in legacy/trac

added component::core tor/tor in Legacy / Trac milestone::Tor: unspecified in Legacy / Trac owner::neel in Legacy / Trac priority::low in Legacy / Trac reviewer::dgoulet in Legacy / Trac severity::normal in Legacy / Trac status::needs-review in Legacy / Trac tor-control in Legacy / Trac tor-spec in Legacy / Trac type::enhancement in Legacy / Trac labels

Trac:
Type: defect to enhancement

Trac:
Status: new to assigned
Owner: N/A to neel
Cc: N/A to neel

PRs:

• Tor: https://github.com/torproject/tor/pull/1907

• Torspec: https://github.com/torproject/torspec/pull/117

Setting as needs review.

Trac:
Status: assigned to needs_review

Trac:
Reviewer: N/A to dgoulet

mentioned in issue legacy/trac#28280 (moved)

moved from legacy/trac#28279 (moved)

added Feature Needs Review labels and removed 1 deleted label

added Reviewer label

assigned to @dgoulet and unassigned @neel

Thanks neel! No show stopper except the naming of the option.

@nickm, @ahf, @asn: If any of you can weigh in on the naming of the GETINFO option would be great. I made suggestions here: https://github.com/torproject/torspec/pull/117#pullrequestreview-437413511

No need to bikeshed this forever, just need rough consensus on a name. This is user expose naming so I prefer we spend couple minutes thinking about it.

added 1 deleted label

I'm +1 on stats, -1 on exposing the rephist name.

Same: +1 on stats, but rephist is confusing even for for me at times. Don't think it's a good idea to expose it via the ControlPort.

Awesome thanks. I guess even if @asn votes for rephist, no majority ahahaha :)

@neel, stats/... seems the accepted consensus so whenever you can, quick fix would be grand. Thanks!

Please re-review and merge if good, I changed to stats/.

Not sure you saw my comments on the PR though except for that naming?

+1 on stats

Oops, sorry.

Made the other requested changes, please approve if okay?

One problem and one request for name change. Should be good after this. Thanks!

added Control API Specifications labels and removed Reviewer + 1 deleted label

removed 1 deleted label

added 1 deleted label

Made the changes, please merge if okay.

PREFIX is still wrong.

PREFIX("rephist/ntor", rephist, "NTor circuit handshake stats."),

Fixed it.

Well doesn't need to be stats/ntor and not rephist ?

True. Made the fix.

Sorry if I didn't catch this earlier.

mentioned in commit b558971c

Great. Squashed and merged!

(I added a commit to simply rename rep_hist in the unit tests to stats.)

closed