#31493 closed defect (wontfix)

Add a version to the metrics output

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

Description

As discussed in #29461, it would be a good idea to add a version number to the snowflake broker metrics spec.

Here's the new spec with the version number included in "snowflake-stats-end": https://github.com/cohosh/snowflake/compare/spec

Child Tickets

Change History (9)

comment:1 Changed 14 months ago by cohosh

Status: assignedneeds_review

comment:2 Changed 14 months ago by dcf

I'm wondering if it would be better to use a separate @type line with a version number, like the other CollecTor formats use?

https://metrics.torproject.org/collector.html#relay-descriptors

  • @type server-descriptor 1.0
  • @type extra-info 1.0
  • @type network-status-consensus-3 1.0
  • etc.

Attaching the version number only to snowflake-stats-end could be difficult to deal with, unless one specifies that snowflake-stats-end must always be the first field, or something.

comment:3 Changed 14 months ago by cohosh

Just trying to coordinate two tickets here. I made this change based on feedback from the metrics team here. The spec says that it does occur "At start, exactly once."

I could make a separate version line so the spec would be as follows:

    "snowflake-stats-end" YYYY-MM-DD HH:MM:SS (NSEC s) NL
        [At start, exactly once.]
        YYYY-MM-DD HH:MM:SS defines the end of the included measurement
        interval of length NSEC seconds (86400 seconds by default).
    "snowflake-stats-version" V.v
        [Exactly once.]
        V.v defines the major and minor version of the snowflake
        broker stats specification being used.
    "snowflake-ips" CC=NUM,CC=NUM,... NL
        [At most once.]
        List of mappings from two-letter country codes to the number of
        unique IP addresses of snowflake proxies that have polled.
    "snowflake-ips-total" NUM NL
        [At most once.]
        A count of the total number of unique IP addresses of snowflake
        proxies that have polled.

[omitting rest of spec]

I'll let the metrics team decide if this is easier/better.

comment:4 Changed 14 months ago by cohosh

I updated https://github.com/cohosh/snowflake/compare/bug31493 to use the following spec:

We export metrics in the following (version 1.0) format:

    "snowflake-stats-end" YYYY-MM-DD HH:MM:SS (NSEC s) NL
        [At start, exactly once.]

        YYYY-MM-DD HH:MM:SS defines the end of the included measurement
        interval of length NSEC seconds (86400 seconds by default).

    "@type snowflake-stats 1.0"
        [Exactly once.]

        Defines the version of snowflake stats output being used
        (currently 1.0)

    "snowflake-ips" CC=NUM,CC=NUM,... NL
        [At most once.]

        List of mappings from two-letter country codes to the number of
        unique IP addresses of snowflake proxies that have polled.

    "snowflake-ips-total" NUM NL
        [At most once.]

        A count of the total number of unique IP addresses of snowflake
        proxies that have polled.

    "snowflake-idle-count" NUM NL
        [At most once.]

        A count of the number of times a proxy has polled but received
        no client offer, rounded up to the nearest multiple of 8.

    "client-denied-count" NUM NL
        [At most once.]

        A count of the number of times a client has requested a proxy
        from the broker but no proxies were available, rounded up to
        the nearest multiple of 8.

    "client-snowflake-match-count" NUM NL
        [At most once.]

        A count of the number of times a client successfully received a
        proxy from the broker, rounded up to the nearest multiple of 8.

comment:5 Changed 14 months ago by karsten

In dir-spec, lines starting with @ are treated as annotations that precede a descriptor and that can be stripped off without changing the descriptor. Including such a line inside a descriptor might be problematic.

But after reading this ticket and #29461, I wonder if my suggestion to add a version number was bad advice. After all, we already have snowflake-stats-end in the first line as unique identifier of this descriptor type. And if something in the data format changes, there will be a specification update introducing new lines anyway. It's probably not worth the effort to update existing specification, implementation, and data to add something that we probably won't need. Can I take this suggestion back and we leave the format unchanged? (Sorry!!)

comment:6 in reply to:  5 ; Changed 14 months ago by cohosh

Replying to karsten:

In dir-spec, lines starting with @ are treated as annotations that precede a descriptor and that can be stripped off without changing the descriptor. Including such a line inside a descriptor might be problematic.

But after reading this ticket and #29461, I wonder if my suggestion to add a version number was bad advice. After all, we already have snowflake-stats-end in the first line as unique identifier of this descriptor type. And if something in the data format changes, there will be a specification update introducing new lines anyway. It's probably not worth the effort to update existing specification, implementation, and data to add something that we probably won't need. Can I take this suggestion back and we leave the format unchanged? (Sorry!!)

Thanks karsten and no worries, it's best to figure out how to make our lives easier in the future and plan for changes now. So if we do end up changing the specification later, will leaving out a version annotation be okay to deal with? I suppose we can archive old versions of the spec in the snowflake repo and make a note of when the change to the new spec was made. I agree now that I'm looking into the annotations more that it's best to leave it out of the descriptor itself.

I'm looking at https://metrics.torproject.org/collector.html now and if the snowflake metrics are going to be archived there, it probably does make sense for them to have an annotation similar to all of the other archived metrics. Just to clarify how the annotations typically work, would we simply move the "@type snowflake-stats 1.0" line to before snowflake-stats-end? Do these other collected metrics provide annotations to CollecTor along with the metrics data in the GET request? Or are they added on your end?

comment:7 Changed 14 months ago by cohosh

Cc: karsten added

comment:8 in reply to:  6 ; Changed 14 months ago by karsten

I think it's fine to leave out version information from the descriptor. You could easily add new keywords or extend existing keywords without breaking compatibility. The only think I can imagine that would be problematic is changing the snowflake-stats-end line or leaving it out in a future version.

Don't worry about the @type annotation line. That gets added by CollecTor. No need to change anything on your side.

comment:9 in reply to:  8 Changed 14 months ago by cohosh

Resolution: wontfix
Status: needs_reviewclosed

Replying to karsten:

I think it's fine to leave out version information from the descriptor. You could easily add new keywords or extend existing keywords without breaking compatibility. The only think I can imagine that would be problematic is changing the snowflake-stats-end line or leaving it out in a future version.

Don't worry about the @type annotation line. That gets added by CollecTor. No need to change anything on your side.

Okay thanks for answering these questions I think we're safe on snowflake-stats-end for the foreseeable future :)

Note: See TracTickets for help on using tickets.