Opened 9 months ago

Closed 3 months ago

#28503 closed enhancement (not a bug)

Move annotation handling to the base Descriptor class

Reported by: irl Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


I'd like to be able to have annotations on any kind of descriptor, not just on ServerDescriptors. Can we move the annotation handling to the base class so that all descriptor types can inherit it?

Child Tickets

Change History (5)

comment:1 Changed 9 months ago by atagar

Hi Iain, I suspect there may be some confusion here due to overloaded terminology. There are two things that use the term of 'annotations'...

  • Server descriptor files (ie, '$TOR_DATA_DIR/cached-descriptors') are prefaced with data like the following. These were around before metric's @type was a twinkle in Karsten's eye, and what our ServerDescriptor class' get_annotations method is referencing.
@downloaded-at 2012-03-14 16:31:05
@source ""
router caerSidi 9001 0 0
platform Tor on Linux x86_64
  • The second are metrics annotations, but calling them general purpose annotations is a bit of a misnomer. When Karsten and I discussed them we only speced a single @type value. If metrics were to add a '@my_new_stuff' annotation it will break parsers until they're adjusted - stem included.

Actually, the approach I'd like to take here is to...

  1. Deprecate ServerDescriptor's annotation methods. I included these for completeness, but they're not part of the dir-spec and nowadays stem users rarely read cached files.
  1. Does Karsten want to expand metrics annotations beyond @type? If so I'd be happy to work with you on the spec and expand stem to recognize them. But we should only invest this effort if there's something we plan to use them for.

comment:2 Changed 9 months ago by atagar

There we go. Went ahead and deprecated the server descriptor methods...

comment:3 Changed 9 months ago by irl

We have reading the cached-descriptors et. al. files as a requirement for CollecTor, to load in any descriptors we missed during unexpected downtime, so we do need that to work.

What I was hoping to be able to do is add @downloaded-at and @source annotations to descriptors I fetch via dir-spec before saving them to disk. Attaching metadata to the descriptors will be useful for working out how much we're load balancing when downloading descriptors, average times between valid-after/published and when we first learn about the descriptors by downloading them, and also the source metadata can be used to prefer downloading extra-info descriptors from the authority we got the server descriptor from (the assumption being that it's more likely to have it on average).

There is discussion about a spec for these annotations in #28067. I would like for these annotations to all follow the same specification.

comment:4 Changed 9 months ago by atagar

Status: newneeds_information

Gotcha, sounds good. I'll consider this pending #28067. Pity we don't have a 'pending' status in trac.

comment:5 Changed 3 months ago by atagar

Resolution: not a bug
Status: needs_informationclosed

Think I'm gonna resolve this. Feel free to reopen if there's something to be done on stem's side.

Note: See TracTickets for help on using tickets.