On Wed, Dec 14, 2022 at 12:14:55PM +0100, Claudio Jeker wrote:
> This diff adds per repository statistics, tracks a few more bits like how
> long it took to sync a repo and finally adds a new openmetrics output.
> The ometric code is from bgpctl but currently has two hacks in to display
> the info and stateset types as gauge because node_exporter is unable to
> properly parse openmetric files.
>
> I did not expose additional stats in any of the other output formats, just
> adjusted them for the new layout. Also I changes the stats from size_t to
> uint32_t (size_t is not the right type and uint64_t is overkill).
Agreed, uint32_t should be fine.
> The metrics file can be exported via webserver directly to prometheus or
> as a node_exporter collector textfile.
Regress will no longer compile with this. At the end is a diff that fixes
regress in case you don't already have a diff for this.
I like it. Below a first pass with a few comments, questions and nits.
> --
> :wq Claudio
>
> Index: Makefile
[...]
> Index: aspa.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 aspa.c
> --- aspa.c 29 Nov 2022 20:41:32 -0000 1.9
> +++ aspa.c 1 Dec 2022 14:54:13 -0000
> @@ -351,9 +351,9 @@ aspa_read(struct ibuf *b)
> * Ensure there are no duplicates in the 'providers' array.
> * Always compare 'expires': use the soonest expiration moment.
> */
> -static void
> +static int
> insert_vap(struct vap_tree *tree, uint32_t cas, uint32_t pas, time_t expires,
> - enum afi afi)
> + enum afi afi, struct repo *rp)
> {
> struct vap *v, *found;
> size_t i;
> @@ -371,7 +371,8 @@ insert_vap(struct vap_tree *tree, uint32
> v->providers[0] = pas;
> v->providersz = 1;
>
> - return;
> + repo_stat_inc(rp, RTYPE_ASPA, UNIQUE);
> + return 1;
> }
>
> free(v);
> @@ -381,7 +382,7 @@ insert_vap(struct vap_tree *tree, uint32
>
> for (i = 0; i < found->providersz; i++) {
> if (found->providers[i] == pas)
> - return;
> + return 0;
> }
>
> found->providers = reallocarray(found->providers,
> @@ -389,6 +390,7 @@ insert_vap(struct vap_tree *tree, uint32
> if (found->providers == NULL)
> err(1, NULL);
> found->providers[found->providersz++] = pas;
> + return 1;
> }
>
> /*
> @@ -397,8 +399,7 @@ insert_vap(struct vap_tree *tree, uint32
> * pre-'AFI explosion' deduplicated count.
> */
> void
> -aspa_insert_vaps(struct vap_tree *tree, struct aspa *aspa, size_t *vaps,
> - size_t *uniqs)
> +aspa_insert_vaps(struct vap_tree *tree, struct aspa *aspa, struct repo *rp)
> {
> size_t i;
> uint32_t cas, pas;
> @@ -407,24 +408,24 @@ aspa_insert_vaps(struct vap_tree *tree,
> cas = aspa->custasid;
> expires = aspa->expires;
>
> - *uniqs += aspa->providersz;
> + repo_stat_inc(rp, RTYPE_ASPA, TOTAL);
>
> for (i = 0; i < aspa->providersz; i++) {
> pas = aspa->providers[i].as;
>
> switch (aspa->providers[i].afi) {
> case AFI_IPV4:
> - insert_vap(tree, cas, pas, expires, AFI_IPV4);
> - (*vaps)++;
> + if (insert_vap(tree, cas, pas, expires, AFI_IPV4, rp))
> + repo_stat_inc(rp, RTYPE_ASPA, ONLY_IPV4);
> break;
> case AFI_IPV6:
> - insert_vap(tree, cas, pas, expires, AFI_IPV6);
> - (*vaps)++;
> + if (insert_vap(tree, cas, pas, expires, AFI_IPV6, rp))
> + repo_stat_inc(rp, RTYPE_ASPA, ONLY_IPV6);
> break;
> default:
> - insert_vap(tree, cas, pas, expires, AFI_IPV4);
> - insert_vap(tree, cas, pas, expires, AFI_IPV6);
> - *vaps += 2;
> + if (insert_vap(tree, cas, pas, expires, AFI_IPV4, rp) ||
> + insert_vap(tree, cas, pas, expires, AFI_IPV6, rp))
I don't think this is correct because of short-circuiting: if a new
AFI_IPV4 vap is inserted, insert_vap(AFI_IPV6) isn't called.
I'm not sure about the best way to fix this. | instead of || would work
but is a bit icky inside the if since it looks like a typo that someone
will 'fix' later. Perhaps something like
new = insert_vap(AFI_IPV4);
new |= insert_vap(AFI_IPV4);
if (new)
repo_stat_inc(rp, RTYPE_ASPA, BOTH);
> + repo_stat_inc(rp, RTYPE_ASPA, BOTH);
> break;
> }
> }
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.163
> diff -u -p -r1.163 extern.h
> --- extern.h 14 Dec 2022 10:34:49 -0000 1.163
> +++ extern.h 14 Dec 2022 11:04:14 -0000
> @@ -376,6 +376,7 @@ struct vrp {
> RB_ENTRY(vrp) entry;
> struct ip_addr addr;
> int talid; /* covered by which TAL */
> + unsigned int repoid;
> uint32_t asid;
> enum afi afi;
> unsigned char maxlength;
> @@ -493,6 +494,20 @@ struct entity {
> };
> TAILQ_HEAD(entityq, entity);
>
> +enum stype {
All other enums are "namespaced" with a prefix, I would prefer to do
this here as well.
> + OK,
> + FAIL,
> + INVALID,
> + STALE,
> + BGPSEC,
> + TOTAL,
> + UNIQUE,
> + DEC_UNIQUE,
> + ONLY_IPV4,
> + ONLY_IPV6,
> + BOTH,
> +};
> +
> struct repo;
> struct filepath;
> RB_HEAD(filepath_tree, filepath);
> @@ -501,41 +516,50 @@ RB_HEAD(filepath_tree, filepath);
> /*
> * Statistics collected during run-time.
> */
> +struct repostats {
> + uint32_t certs; /* certificates */
> + uint32_t certs_fail; /* invalid certificate */
> + uint32_t mfts; /* total number of manifests */
> + uint32_t mfts_fail; /* failing syntactic parse */
> + uint32_t mfts_stale; /* stale manifests */
> + uint32_t roas; /* route origin authorizations */
> + uint32_t roas_fail; /* failing syntactic parse */
> + uint32_t roas_invalid; /* invalid resources */
> + uint32_t aspas; /* ASPA objects */
> + uint32_t aspas_fail; /* ASPA objects failing syntactic parse */
> + uint32_t aspas_invalid; /* ASPAs with invalid customerASID */
> + uint32_t brks; /* number of BGPsec Router Key (BRK) certs */
> + uint32_t crls; /* revocation lists */
> + uint32_t gbrs; /* ghostbuster records */
> + uint32_t taks; /* signed TAL objects */
> + uint32_t vaps; /* total number of Validated ASPA Payloads */
> + uint32_t vaps_uniqs; /* total number of unique VAPs */
> + uint32_t vaps_pas; /* total number of providers */
> + uint32_t vaps_pas4; /* total number of IPv4 only providers */
> + uint32_t vaps_pas6; /* total number of IPv6 only providers */
> + uint32_t vrps; /* total number of Validated ROA Payloads */
> + uint32_t vrps_uniqs; /* number of unique vrps */
> + struct timespec sync_time; /* time to sync repo */
> +};
> +
> struct stats {
> - size_t tals; /* total number of locators */
> - size_t mfts; /* total number of manifests */
> - size_t mfts_fail; /* failing syntactic parse */
> - size_t mfts_stale; /* stale manifests */
> - size_t certs; /* certificates */
> - size_t certs_fail; /* invalid certificate */
> - size_t roas; /* route origin authorizations */
> - size_t roas_fail; /* failing syntactic parse */
> - size_t roas_invalid; /* invalid resources */
> - size_t repos; /* repositories */
> - size_t rsync_repos; /* synced rsync repositories */
> - size_t rsync_fails; /* failed rsync repositories */
> - size_t http_repos; /* synced http repositories */
> - size_t http_fails; /* failed http repositories */
> - size_t rrdp_repos; /* synced rrdp repositories */
> - size_t rrdp_fails; /* failed rrdp repositories */
> - size_t crls; /* revocation lists */
> - size_t gbrs; /* ghostbuster records */
> - size_t taks; /* signed TAL objects */
> - size_t aspas; /* ASPA objects */
> - size_t aspas_fail; /* ASPA objects failing syntactic parse */
> - size_t aspas_invalid; /* ASPAs with invalid customerASID */
> - size_t vaps; /* total number of Validated ASPA Payloads */
> - size_t vaps_uniqs; /* total number of unique VAPs */
> - size_t vrps; /* total number of vrps */
> - size_t uniqs; /* number of unique vrps */
> - size_t del_files; /* number of files removed in cleanup */
> - size_t extra_files; /* number of superfluous files */
> - size_t del_dirs; /* number of directories removed in cleanup */
> - size_t brks; /* number of BGPsec Router Key (BRK) certificates */
> - size_t skiplistentries; /* number of skiplist entries */
> - struct timespec elapsed_time;
> - struct timespec user_time;
> - struct timespec system_time;
> + uint32_t tals; /* total number of locators */
> + uint32_t repos; /* repositories */
> + uint32_t rsync_repos; /* synced rsync repositories */
> + uint32_t rsync_fails; /* failed rsync repositories */
> + uint32_t http_repos; /* synced http repositories */
> + uint32_t http_fails; /* failed http repositories */
> + uint32_t rrdp_repos; /* synced rrdp repositories */
> + uint32_t rrdp_fails; /* failed rrdp repositories */
> + uint32_t del_files; /* number of files removed in cleanup */
> + uint32_t extra_files; /* number of superfluous files */
> + uint32_t del_dirs; /* number of dirs removed in cleanup */
> + uint32_t skiplistentries; /* number of skiplist entries */
> +
> + struct repostats repo_stats;
> + struct timespec elapsed_time;
> + struct timespec user_time;
> + struct timespec system_time;
> };
>
> struct ibuf;
> @@ -547,6 +571,7 @@ extern int filemode;
> extern const char *tals[];
> extern const char *taldescs[];
> extern unsigned int talrepocnt[];
> +extern struct repostats talstats[];
> extern int talsz;
>
> /* Routines for RPKI entities. */
> @@ -580,8 +605,8 @@ void roa_free(struct roa *);
> struct roa *roa_parse(X509 **, const char *, const unsigned char *,
> size_t);
> struct roa *roa_read(struct ibuf *);
> -void roa_insert_vrps(struct vrp_tree *, struct roa *, size_t *,
> - size_t *);
> +void roa_insert_vrps(struct vrp_tree *, struct roa *,
> + struct repo *);
>
> void gbr_free(struct gbr *);
> struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
> @@ -602,8 +627,8 @@ struct tak *tak_read(struct ibuf *);
>
> void aspa_buffer(struct ibuf *, const struct aspa *);
> void aspa_free(struct aspa *);
> -void aspa_insert_vaps(struct vap_tree *, struct aspa *, size_t *,
> - size_t *);
> +void aspa_insert_vaps(struct vap_tree *, struct aspa *,
> + struct repo *);
> struct aspa *aspa_parse(X509 **, const char *, const unsigned char *,
> size_t);
> struct aspa *aspa_read(struct ibuf *);
> @@ -703,11 +728,19 @@ int rrdp_handle_file(unsigned int, enu
> char *repo_basedir(const struct repo *, int);
> unsigned int repo_id(const struct repo *);
> const char *repo_uri(const struct repo *);
> +void repo_fetch_uris(const struct repo *, const char **,
> + const char **);
> +int repo_synced(const struct repo *);
> +int repo_talid(const struct repo *);
> struct repo *ta_lookup(int, struct tal *);
> struct repo *repo_lookup(int, const char *, const char *);
> struct repo *repo_byid(unsigned int);
> int repo_queued(struct repo *, struct entity *);
> void repo_cleanup(struct filepath_tree *, int);
> +int repo_check_timeout(int);
> +void repo_stat_inc(struct repo *, enum rtype, enum stype);
> +void repo_stats_collect(void (*)(const struct repo *,
> + const struct repostats *, void *), void *);
> void repo_free(void);
>
> void rsync_finish(unsigned int, int);
> @@ -722,7 +755,6 @@ void rrdp_fetch(unsigned int, const ch
> struct rrdp_session *);
> void rrdp_abort(unsigned int);
> void rrdp_http_done(unsigned int, enum http_result, const char *);
> -int repo_check_timeout(int);
>
> /* Logging (though really used for OpenSSL errors). */
>
> @@ -797,6 +829,7 @@ extern int outformats;
> #define FORMAT_BIRD 0x02
> #define FORMAT_CSV 0x04
> #define FORMAT_JSON 0x08
> +#define FORMAT_OMETRIC 0x10
>
> int outputfiles(struct vrp_tree *v, struct brk_tree *b,
> struct vap_tree *, struct stats *);
> @@ -812,6 +845,8 @@ int output_bird2(FILE *, struct vrp_tr
> int output_csv(FILE *, struct vrp_tree *, struct brk_tree *,
> struct vap_tree *, struct stats *);
> int output_json(FILE *, struct vrp_tree *, struct brk_tree *,
> + struct vap_tree *, struct stats *);
> +int output_ometric(FILE *, struct vrp_tree *, struct brk_tree *,
> struct vap_tree *, struct stats *);
>
> void logx(const char *fmt, ...)
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.228
> diff -u -p -r1.228 main.c
> --- main.c 14 Dec 2022 10:34:49 -0000 1.228
> +++ main.c 14 Dec 2022 11:03:49 -0000
> @@ -51,6 +51,7 @@
> const char *tals[TALSZ_MAX];
> const char *taldescs[TALSZ_MAX];
> unsigned int talrepocnt[TALSZ_MAX];
> +struct repostats talstats[TALSZ_MAX];
> int talsz;
>
> size_t entity_queue;
> @@ -535,7 +536,9 @@ entity_process(struct ibuf *b, struct st
> struct mft *mft;
> struct roa *roa;
> struct aspa *aspa;
> + struct repo *rp;
> char *file;
> + unsigned int id;
> int c;
>
> /*
> @@ -556,6 +559,9 @@ entity_process(struct ibuf *b, struct st
> goto done;
> }
>
> + io_read_buf(b, &id, sizeof(id));
> + rp = repo_byid(id);
Can we assert(rp != NULL) here or should we error? For example, the
repo_id() call in roa_insert_vrps() could result in a crash otherwise.
> + repo_stat_inc(rp, type, OK);
> switch (type) {
> case RTYPE_TAL:
> st->tals++;
> @@ -564,10 +570,9 @@ entity_process(struct ibuf *b, struct st
> tal_free(tal);
> break;
> case RTYPE_CER:
> - st->certs++;
> io_read_buf(b, &c, sizeof(c));
> if (c == 0) {
> - st->certs_fail++;
> + repo_stat_inc(rp, type, FAIL);
> break;
> }
> cert = cert_read(b);
> @@ -577,65 +582,58 @@ entity_process(struct ibuf *b, struct st
> break;
> case CERT_PURPOSE_BGPSEC_ROUTER:
> cert_insert_brks(brktree, cert);
> - st->brks++;
> + repo_stat_inc(rp, type, BGPSEC);
> break;
> default:
> - st->certs_fail++;
> + errx(1, "unexpected cert purpose received");
> break;
> }
> cert_free(cert);
> break;
> case RTYPE_MFT:
> - st->mfts++;
> io_read_buf(b, &c, sizeof(c));
> if (c == 0) {
> - st->mfts_fail++;
> + repo_stat_inc(rp, type, FAIL);
> break;
> }
> mft = mft_read(b);
> if (!mft->stale)
> queue_add_from_mft(mft);
> else
> - st->mfts_stale++;
> + repo_stat_inc(rp, type, STALE);
> mft_free(mft);
> break;
> case RTYPE_CRL:
> - st->crls++;
> break;
> case RTYPE_ROA:
> - st->roas++;
> io_read_buf(b, &c, sizeof(c));
> if (c == 0) {
> - st->roas_fail++;
> + repo_stat_inc(rp, type, FAIL);
> break;
> }
> roa = roa_read(b);
> if (roa->valid)
> - roa_insert_vrps(tree, roa, &st->vrps, &st->uniqs);
> + roa_insert_vrps(tree, roa, rp);
> else
> - st->roas_invalid++;
> + repo_stat_inc(rp, type, INVALID);
> roa_free(roa);
> break;
> case RTYPE_GBR:
> - st->gbrs++;
> break;
> case RTYPE_ASPA:
> - st->aspas++;
> io_read_buf(b, &c, sizeof(c));
> if (c == 0) {
> - st->aspas_fail++;
> + repo_stat_inc(rp, type, FAIL);
> break;
> }
> aspa = aspa_read(b);
> if (aspa->valid)
> - aspa_insert_vaps(vaptree, aspa, &st->vaps,
> - &st->vaps_uniqs);
> + aspa_insert_vaps(vaptree, aspa, rp);
> else
> - st->aspas_invalid++;
> + repo_stat_inc(rp, type, INVALID);
> aspa_free(aspa);
> break;
> case RTYPE_TAK:
> - st->taks++;
> break;
> case RTYPE_FILE:
> break;
> @@ -703,6 +701,40 @@ rrdp_process(struct ibuf *b)
> }
> }
>
> +static void
> +sum_stats(const struct repo *rp, const struct repostats *in, void *arg)
> +{
> + struct repostats *out = arg;
> +
> + if (rp != NULL)
> + sum_stats(NULL, in, &talstats[repo_talid(rp)]);
> +
> + out->mfts += in->mfts;
> + out->mfts_fail += in->mfts_fail;
> + out->mfts_stale += in->mfts_stale;
> + out->certs += in->certs;
> + out->certs_fail += in->certs_fail;
> + out->roas += in->roas;
> + out->roas_fail += in->roas_fail;
> + out->roas_invalid += in->roas_invalid;
> + out->aspas += in->aspas;
> + out->aspas_fail += in->aspas_fail;
> + out->aspas_invalid += in->aspas_invalid;
> + out->brks += in->brks;
> + out->crls += in->crls;
> + out->gbrs += in->gbrs;
> + out->taks += in->taks;
> + out->vrps += in->vrps;
> + out->vrps_uniqs += in->vrps_uniqs;
> + out->vaps += in->vaps;
> + out->vaps_uniqs += in->vaps_uniqs;
> + out->vaps_pas += in->vaps_pas;
> + out->vaps_pas4 += in->vaps_pas4;
> + out->vaps_pas6 += in->vaps_pas6;
> +
> + timespecadd(&in->sync_time, &out->sync_time, &out->sync_time);
> +}
> +
> /*
> * Assign filenames ending in ".tal" in "/etc/rpki" into "tals",
> * returning the number of files found and filled-in.
> @@ -909,7 +941,7 @@ main(int argc, char *argv[])
> "proc exec unveil", NULL) == -1)
> err(1, "pledge");
>
> - while ((c = getopt(argc, argv, "b:Bcd:e:fH:jnorRs:S:t:T:vV")) != -1)
> + while ((c = getopt(argc, argv, "b:Bcd:e:fH:jmnorRs:S:t:T:vV")) != -1)
> switch (c) {
> case 'b':
> bind_addr = optarg;
> @@ -937,6 +969,9 @@ main(int argc, char *argv[])
> case 'j':
> outformats |= FORMAT_JSON;
> break;
> + case 'm':
> + outformats |= FORMAT_OMETRIC;
> + break;
> case 'n':
> noop = 1;
> break;
> @@ -1344,6 +1379,8 @@ main(int argc, char *argv[])
> if (fchdir(outdirfd) == -1)
> err(1, "fchdir output dir");
>
> + repo_stats_collect(sum_stats, &stats.repo_stats);
> +
> if (outputfiles(&vrps, &brks, &vaps, &stats))
> rc = 1;
>
> @@ -1352,26 +1389,31 @@ main(int argc, char *argv[])
> (long long)stats.elapsed_time.tv_sec,
> (long long)stats.user_time.tv_sec,
> (long long)stats.system_time.tv_sec);
> - printf("Skiplist entries: %zu\n", stats.skiplistentries);
> - printf("Route Origin Authorizations: %zu (%zu failed parse, %zu "
> - "invalid)\n", stats.roas, stats.roas_fail, stats.roas_invalid);
> - printf("AS Provider Attestations: %zu (%zu failed parse, %zu "
> - "invalid)\n", stats.aspas, stats.aspas_fail, stats.aspas_invalid);
> - printf("BGPsec Router Certificates: %zu\n", stats.brks);
> - printf("Certificates: %zu (%zu invalid)\n",
> - stats.certs, stats.certs_fail);
> - printf("Trust Anchor Locators: %zu (%zu invalid)\n",
> + printf("Skiplist entries: %u\n", stats.skiplistentries);
> + printf("Route Origin Authorizations: %u (%u failed parse, %u "
> + "invalid)\n", stats.repo_stats.roas, stats.repo_stats.roas_fail,
> + stats.repo_stats.roas_invalid);
> + printf("AS Provider Attestations: %u (%u failed parse, %u "
> + "invalid)\n", stats.repo_stats.aspas, stats.repo_stats.aspas_fail,
> + stats.repo_stats.aspas_invalid);
> + printf("BGPsec Router Certificates: %u\n", stats.repo_stats.brks);
> + printf("Certificates: %u (%u invalid)\n",
> + stats.repo_stats.certs, stats.repo_stats.certs_fail);
> + printf("Trust Anchor Locators: %u (%u invalid)\n",
> stats.tals, talsz - stats.tals);
> - printf("Manifests: %zu (%zu failed parse, %zu stale)\n",
> - stats.mfts, stats.mfts_fail, stats.mfts_stale);
> - printf("Certificate revocation lists: %zu\n", stats.crls);
> - printf("Ghostbuster records: %zu\n", stats.gbrs);
> - printf("Trust Anchor Keys: %zu\n", stats.taks);
> - printf("Repositories: %zu\n", stats.repos);
> - printf("Cleanup: removed %zu files, %zu directories, %zu superfluous\n",
> + printf("Manifests: %u (%u failed parse, %u stale)\n",
> + stats.repo_stats.mfts, stats.repo_stats.mfts_fail,
> + stats.repo_stats.mfts_stale);
> + printf("Certificate revocation lists: %u\n", stats.repo_stats.crls);
> + printf("Ghostbuster records: %u\n", stats.repo_stats.gbrs);
> + printf("Trust Anchor Keys: %u\n", stats.repo_stats.taks);
> + printf("Repositories: %u\n", stats.repos);
> + printf("Cleanup: removed %u files, %u directories, %u superfluous\n",
> stats.del_files, stats.del_dirs, stats.extra_files);
> - printf("VRP Entries: %zu (%zu unique)\n", stats.vrps, stats.uniqs);
> - printf("VAP Entries: %zu (%zu unique)\n", stats.vaps, stats.vaps_uniqs);
> + printf("VRP Entries: %u (%u unique)\n", stats.repo_stats.vrps,
> + stats.repo_stats.vrps_uniqs);
> + printf("VAP Entries: %u (%u unique)\n", stats.repo_stats.vaps,
> + stats.repo_stats.vaps_uniqs);
>
> /* Memory cleanup. */
> repo_free();
> @@ -1380,7 +1422,7 @@ main(int argc, char *argv[])
>
> usage:
> fprintf(stderr,
> - "usage: rpki-client [-BcjnoRrVv] [-b sourceaddr] [-d cachedir]"
> + "usage: rpki-client [-BcjmnoRrVv] [-b sourceaddr] [-d cachedir]"
> " [-e rsync_prog]\n"
> " [-H fqdn] [-S skiplist] [-s timeout] [-T table]"
> " [-t tal]\n"
> Index: ometric.c
> Index: ometric.h
> Index: output-json.c
[...]
> Index: output-ometric.c
> ===================================================================
> RCS file: output-ometric.c
> diff -N output-ometric.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ output-ometric.c 14 Dec 2022 08:40:54 -0000
> @@ -0,0 +1,212 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2022 Claudio Jeker <[email protected]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <err.h>
> +#include <limits.h>
> +#include <stdlib.h>
> +#include <stdio.h>
stdlib comes after stdio
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include "extern.h"
> +#include "ometric.h"
> +#include "version.h"
> +
> +struct ometric *rpki_info, *rpki_completion_time, *rpki_duration;
> +struct ometric *rpki_repo, *rpki_obj, *rpki_ta_obj;
> +struct ometric *rpki_repo_obj, *rpki_repo_duration, *rpki_repo_state;
> +
> +const char * const repo_states[2] = { "failed", "synced" };
static?
> +
> +static void
> +set_common_stats(const struct repostats *in, struct ometric *metric,
> + struct olabels *ol)
> +{
> + ometric_set_int_with_labels(metric, in->certs,
> + OKV("type", "state"), OKV("cert", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->certs_fail,
> + OKV("type", "state"), OKV("cert", "failed parse"), ol);
> +
> + ometric_set_int_with_labels(metric, in->mfts,
> + OKV("type", "state"), OKV("manifest", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->mfts_fail,
> + OKV("type", "state"), OKV("manifest", "failed parse"), ol);
> + ometric_set_int_with_labels(metric, in->mfts_stale,
> + OKV("type", "state"), OKV("manifest", "stale"), ol);
> +
> + ometric_set_int_with_labels(metric, in->roas,
> + OKV("type", "state"), OKV("roa", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->roas_fail,
> + OKV("type", "state"), OKV("roa", "failed parse"), ol);
> + ometric_set_int_with_labels(metric, in->roas_invalid,
> + OKV("type", "state"), OKV("roa", "invalid"), ol);
> +
> + ometric_set_int_with_labels(metric, in->aspas,
> + OKV("type", "state"), OKV("aspa", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->aspas_fail,
> + OKV("type", "state"), OKV("aspa", "failed parse"), ol);
> + ometric_set_int_with_labels(metric, in->aspas_invalid,
> + OKV("type", "state"), OKV("aspa", "invalid"), ol);
> +
> + ometric_set_int_with_labels(metric, in->brks,
> + OKV("type", "state"), OKV("router_key", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->crls,
> + OKV("type", "state"), OKV("crl", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->gbrs,
> + OKV("type", "state"), OKV("gbr", "valid"), ol);
> + ometric_set_int_with_labels(metric, in->taks,
> + OKV("type", "state"), OKV("tak", "valid"), ol);
> +
> + ometric_set_int_with_labels(metric, in->vrps,
> + OKV("type", "state"), OKV("vrp", "total"), ol);
> + ometric_set_int_with_labels(metric, in->vrps_uniqs,
> + OKV("type", "state"), OKV("vrp", "unique"), ol);
empty line?
> + ometric_set_int_with_labels(metric, in->vaps,
> + OKV("type", "state"), OKV("vap", "total"), ol);
> + ometric_set_int_with_labels(metric, in->vaps_uniqs,
> + OKV("type", "state"), OKV("vap", "unique"), ol);
> + ometric_set_int_with_labels(metric, in->vaps_pas,
> + OKV("type", "state"), OKV("vap porviders", "both"), ol);
> + ometric_set_int_with_labels(metric, in->vaps_pas4,
> + OKV("type", "state"), OKV("vap porviders", "IPv4 only"), ol);
> + ometric_set_int_with_labels(metric, in->vaps_pas6,
> + OKV("type", "state"), OKV("vap porviders", "IPv6 only"), ol);
porviders -> providers (3x)
> +
> +}
> +
> +static void
> +ta_stats(int id)
> +{
> + struct olabels *ol;
> + const char *keys[2] = { "name", NULL };
> + const char *values[2];
> +
> + values[0] = taldescs[id];
> + values[1] = NULL;
> +
> + ol = olabels_new(keys, values);
> + set_common_stats(&talstats[id], rpki_ta_obj, ol);
> + olabels_free(ol);
> +}
> +
> +static void
> +repo_stats(const struct repo *rp, const struct repostats *in, void *arg)
> +{
> + struct olabels *ol;
> + const char *keys[4] = { "name", "carepo", "notify", NULL };
> + const char *values[4];
> +
> + values[0] = taldescs[repo_talid(rp)];
> + repo_fetch_uris(rp, &values[1], &values[2]);
> + values[3] = NULL;
> +
> + ol = olabels_new(keys, values);
> + set_common_stats(in, rpki_repo_obj, ol);
> + ometric_set_timespec(rpki_repo_duration, &in->sync_time, ol);
> + ometric_set_state(rpki_repo_state, repo_states[repo_synced(rp)], ol);
> + olabels_free(ol);
> +}
> +
> +int
> +output_ometric(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks,
> + struct vap_tree *vaps, struct stats *st)
> +{
> + struct olabels *ol;
> + const char *keys[4] = { "nodename", "domainname", "release", NULL };
Surely there's a reason... I don't understand "nodename" vs "hostname".
> + const char *values[4];
> + char hostname[HOST_NAME_MAX + 1];
> + char *domainname;
> + struct timespec now_time;
> + int rv, i;
> +
> + rpki_info = ometric_new(OMT_INFO, "rpki_client",
> + "rpki-client information");
> + rpki_completion_time = ometric_new(OMT_GAUGE,
> + "rpki_client_job_completion_time",
> + "end of this run as epoch timestamp");
> +
> + rpki_repo = ometric_new(OMT_GAUGE, "rpki_client_repository",
> + "total number of repositories");
> + rpki_obj = ometric_new(OMT_GAUGE, "rpki_client_objects",
> + "total number of objects");
> +
> + rpki_duration = ometric_new(OMT_GAUGE, "rpki_client_duration",
> + "duration in seconds");
> +
> + rpki_ta_obj = ometric_new(OMT_GAUGE, "rpki_client_ta_objects",
> + "total number of objects per TAL");
> + rpki_repo_obj = ometric_new(OMT_GAUGE, "rpki_client_repository_objects",
> + "total number of objects per repository");
> + rpki_repo_duration = ometric_new(OMT_GAUGE,
> + "rpki_client_repository_duration",
> + "duration used to sync this repository in seconds");
> + rpki_repo_state = ometric_new_state(repo_states,
> + sizeof(repo_states) / sizeof(repo_states[0]),
> + "rpki_client_repository_state",
> + "repository state");
> +
> + /*
> + * Dump statistics
> + */
> + if (gethostname(hostname, sizeof(hostname)))
> + err(1, "gethostname");
> + if ((domainname = strchr(hostname, '.')))
> + *domainname++ = '\0';
> +
> + values[0] = hostname;
> + values[1] = domainname;
> + values[2] = RPKI_VERSION;
> + values[3] = NULL;
> +
> + ol = olabels_new(keys, values);
> + ometric_set_info(rpki_info, NULL, NULL, ol);
> + olabels_free(ol);
> +
> + repo_stats_collect(repo_stats, NULL);
> + for (i = 0; i < talsz; i++)
> + ta_stats(i);
> + set_common_stats(&st->repo_stats, rpki_obj, NULL);
> +
> + ometric_set_int(rpki_repo, st->repos, NULL);
> + ometric_set_int_with_labels(rpki_repo, st->rsync_repos,
> + OKV("type", "state"), OKV("rsync", "synced"), NULL);
> + ometric_set_int_with_labels(rpki_repo, st->rsync_fails,
> + OKV("type", "state"), OKV("rsync", "failed"), NULL);
> + ometric_set_int_with_labels(rpki_repo, st->http_repos,
> + OKV("type", "state"), OKV("http", "synced"), NULL);
> + ometric_set_int_with_labels(rpki_repo, st->http_fails,
> + OKV("type", "state"), OKV("http", "failed"), NULL);
> + ometric_set_int_with_labels(rpki_repo, st->rrdp_repos,
> + OKV("type", "state"), OKV("rrdp", "synced"), NULL);
> + ometric_set_int_with_labels(rpki_repo, st->rrdp_fails,
> + OKV("type", "state"), OKV("rrdp", "failed"), NULL);
> +
> + ometric_set_timespec_with_labels(rpki_duration, &st->elapsed_time,
> + OKV("type"), OKV("elapsed"), NULL);
> + ometric_set_timespec_with_labels(rpki_duration, &st->user_time,
> + OKV("type"), OKV("user"), NULL);
> + ometric_set_timespec_with_labels(rpki_duration, &st->system_time,
> + OKV("type"), OKV("system"), NULL);
> +
> + clock_gettime(CLOCK_REALTIME, &now_time);
> + ometric_set_timespec(rpki_completion_time, &now_time, NULL);
> +
> + rv = ometric_output_all(out);
> + ometric_free_all();
> +
> + return rv;
> +}
> Index: output.c
> Index: parser.c
[...]
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 repo.c
> --- repo.c 2 Sep 2022 21:56:45 -0000 1.39
> +++ repo.c 14 Dec 2022 08:42:10 -0000
> @@ -97,6 +97,8 @@ struct repo {
> const struct rsyncrepo *rsync;
> const struct tarepo *ta;
> struct entityq queue; /* files waiting for repo */
> + struct repostats stats;
> + struct timespec start_time;
> time_t alarm; /* sync timeout */
> int talid;
> unsigned int id; /* identifier */
> @@ -263,7 +265,7 @@ repo_mkpath(int fd, char *file)
> * Return the state of a repository.
> */
> static enum repo_state
> -repo_state(struct repo *rp)
> +repo_state(const struct repo *rp)
> {
> if (rp->ta)
> return rp->ta->state;
> @@ -283,24 +285,25 @@ static void
> repo_done(const void *vp, int ok)
> {
> struct repo *rp;
> + struct timespec flush_time;
>
> SLIST_FOREACH(rp, &repos, entry) {
> - if (vp == rp->ta)
> - entityq_flush(&rp->queue, rp);
> - if (vp == rp->rsync)
> - entityq_flush(&rp->queue, rp);
> - if (vp == rp->rrdp) {
> - if (!ok && !nofetch) {
> - /* try to fall back to rsync */
> - rp->rrdp = NULL;
> - rp->rsync = rsync_get(rp->repouri,
> - rp->basedir);
> - /* need to check if it was already loaded */
> - if (repo_state(rp) != REPO_LOADING)
> - entityq_flush(&rp->queue, rp);
> - } else
> - entityq_flush(&rp->queue, rp);
> + if (vp != rp->ta && vp != rp->rsync && vp != rp->rrdp)
> + continue;
> +
> + /* for rrdp try to fall back to rsync */
> + if (vp == rp->rrdp && !ok && !nofetch) {
> + rp->rrdp = NULL;
> + rp->rsync = rsync_get(rp->repouri,
> + rp->basedir);
could unwrap previous line
> + /* need to check if it was already loaded */
> + if (repo_state(rp) == REPO_LOADING)
> + continue;
> }
> +
> + entityq_flush(&rp->queue, rp);
> + clock_gettime(CLOCK_MONOTONIC, &flush_time);
> + timespecsub(&flush_time, &rp->start_time, &rp->stats.sync_time);
> }
> }
>
> @@ -600,6 +603,7 @@ repo_alloc(int talid)
> rp->alarm = getmonotime() + repo_timeout;
> TAILQ_INIT(&rp->queue);
> SLIST_INSERT_HEAD(&repos, rp, entry);
> + clock_gettime(CLOCK_MONOTONIC, &rp->start_time);
>
> stats.repos++;
> return rp;
> @@ -1179,6 +1183,39 @@ repo_uri(const struct repo *rp)
> return rp->repouri;
> }
>
> +/*
> + * Return the repository URI.
> + */
> +void
> +repo_fetch_uris(const struct repo *rp, const char **carepo,
> + const char **notifyuri)
> +{
> + *carepo = rp->repouri;
> + *notifyuri = rp->notifyuri;
> +}
> +
> +/*
> + * Return 1 if repository is synced else 0.
> + */
> +int
> +repo_synced(const struct repo *rp)
> +{
> + if (repo_state(rp) == REPO_DONE &&
> + !(rp->rrdp == NULL && rp->rsync == NULL && rp->ta == NULL))
> + return 1;
> + return 0;
> +}
> +
previous line is extra (with trailing tab)
> +
> +/*
> + * Return the repository tal ID.
> + */
> +int
> +repo_talid(const struct repo *rp)
> +{
> + return rp->talid;
> +}
> +
> int
> repo_queued(struct repo *rp, struct entity *p)
> {
> @@ -1260,6 +1297,110 @@ repo_check_timeout(int timeout)
> }
> }
> return timeout;
> +}
> +
> +/*
> + * Update stats object of repository depending on rtype and subtype.
> + */
> +void
> +repo_stat_inc(struct repo *rp, enum rtype type, enum stype subtype)
> +{
> + if (rp == NULL)
> + return;
> + switch (type) {
> + case RTYPE_CER:
> + if (subtype == OK)
> + rp->stats.certs++;
> + if (subtype == BGPSEC) {
> + rp->stats.certs--;
> + rp->stats.brks++;
> + }
subtype FAIL isn't handled but called from entity_process()
> + break;
> + case RTYPE_MFT:
> + if (subtype == OK)
> + rp->stats.mfts++;
> + if (subtype == FAIL)
> + rp->stats.mfts_fail++;
> + if (subtype == STALE)
> + rp->stats.mfts_stale++;
> + break;
> + case RTYPE_ROA:
> + switch (subtype) {
> + case OK:
> + rp->stats.roas++;
> + break;
> + case FAIL:
> + rp->stats.roas_fail++;
> + break;
> + case INVALID:
> + rp->stats.roas_invalid++;
> + break;
> + case TOTAL:
> + rp->stats.vrps++;
> + break;
> + case UNIQUE:
> + rp->stats.vrps_uniqs++;
> + break;
> + case DEC_UNIQUE:
> + rp->stats.vrps_uniqs--;
I wonder if we should at least warn on wraparound. This can't currently
happen, but what if we change the logic down the road?
> + break;
> + default:
> + break;
> + }
> + break;
> + case RTYPE_ASPA:
> + switch (subtype) {
> + case OK:
> + rp->stats.aspas++;
> + break;
> + case FAIL:
> + rp->stats.aspas_fail++;
> + break;
> + case INVALID:
> + rp->stats.aspas_invalid++;
> + break;
> + case TOTAL:
> + rp->stats.vaps++;
> + break;
> + case UNIQUE:
> + rp->stats.vaps_uniqs++;
> + break;
> + case BOTH:
> + rp->stats.vaps_pas++;
> + break;
> + case ONLY_IPV4:
> + rp->stats.vaps_pas4++;
> + break;
> + case ONLY_IPV6:
> + rp->stats.vaps_pas6++;
> + break;
> + default:
> + break;
> + }
> + break;
> + case RTYPE_CRL:
> + rp->stats.crls++;
> + break;
> + case RTYPE_GBR:
> + rp->stats.gbrs++;
> + break;
> + case RTYPE_TAK:
> + rp->stats.taks++;
> + break;
> + default:
> + break;
> + }
> +}
> +
> +void
> +repo_stats_collect(void (*cb)(const struct repo *rp, const struct repostats
> *,
replace '*rp' with '*' for consistency with the other arguments
> + void *), void *arg)
> +{
> + struct repo *rp;
> +
> + SLIST_FOREACH(rp, &repos, entry) {
> + cb(rp, &rp->stats, arg);
> + }
> }
>
> /*
> Index: roa.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 roa.c
> --- roa.c 29 Nov 2022 20:41:32 -0000 1.58
> +++ roa.c 6 Dec 2022 11:11:42 -0000
> @@ -360,8 +360,7 @@ roa_read(struct ibuf *b)
> * number of addresses.
> */
> void
> -roa_insert_vrps(struct vrp_tree *tree, struct roa *roa, size_t *vrps,
> - size_t *uniqs)
> +roa_insert_vrps(struct vrp_tree *tree, struct roa *roa, struct repo *rp)
> {
> struct vrp *v, *found;
> size_t i;
> @@ -374,6 +373,7 @@ roa_insert_vrps(struct vrp_tree *tree, s
> v->maxlength = roa->ips[i].maxlength;
> v->asid = roa->asid;
> v->talid = roa->talid;
> + v->repoid = repo_id(rp);
> v->expires = roa->expires;
>
> /*
> @@ -387,12 +387,17 @@ roa_insert_vrps(struct vrp_tree *tree, s
> /* update found with preferred data */
> found->talid = v->talid;
> found->expires = v->expires;
> + /* adjust unique count */
> + repo_stat_inc(repo_byid(found->repoid),
> + RTYPE_ROA, DEC_UNIQUE);
> + found->repoid = v->repoid;
> + repo_stat_inc(rp, RTYPE_ROA, UNIQUE);
> }
> free(v);
> } else
> - (*uniqs)++;
> + repo_stat_inc(rp, RTYPE_ROA, UNIQUE);
>
> - (*vrps)++;
> + repo_stat_inc(rp, RTYPE_ROA, TOTAL);
> }
> }
>
> Index: rpki-client.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> retrieving revision 1.81
> diff -u -p -r1.81 rpki-client.8
> --- rpki-client.8 26 Nov 2022 12:02:37 -0000 1.81
> +++ rpki-client.8 14 Dec 2022 09:19:30 -0000
> @@ -22,7 +22,7 @@
> .Nd RPKI validator to support BGP routing security
> .Sh SYNOPSIS
> .Nm
> -.Op Fl BcjnoRrVv
> +.Op Fl BcjmnoRrVv
> .Op Fl b Ar sourceaddr
> .Op Fl d Ar cachedir
> .Op Fl e Ar rsync_prog
> @@ -138,6 +138,10 @@ in the output directory as JSON object.
> See
> .Fl c
> for a description of the fields.
> +.It Fl m
> +Create output in the file
> +.Pa metrics
> +in the output directory in OpenMetrics format.
> .It Fl n
> Offline mode.
> Validate the contents of
>
Index: Makefile.inc
===================================================================
RCS file: /cvs/src/regress/usr.sbin/rpki-client/Makefile.inc,v
retrieving revision 1.27
diff -u -p -r1.27 Makefile.inc
--- Makefile.inc 26 Nov 2022 12:09:34 -0000 1.27
+++ Makefile.inc 14 Dec 2022 12:52:05 -0000
@@ -43,7 +43,7 @@ run-regress-test-mft: test-mft
./test-mft -v ${.CURDIR}/../mft/*.mft
SRCS_test-roa+= test-roa.c roa.c cms.c x509.c ip.c as.c io.c log.c \
- encoding.c print.c validate.c cert.c mft.c
+ encoding.c print.c validate.c cert.c mft.c repo-dummy.c
run-regress-test-roa: test-roa
./test-roa -v ${.CURDIR}/../roa/*.roa
@@ -68,7 +68,7 @@ run-regress-test-tal: test-tal
./test-tal -v ${.CURDIR}/../tal/*.tal
SRCS_test-aspa+= test-aspa.c aspa.c cms.c x509.c ip.c as.c io.c log.c \
- encoding.c print.c validate.c cert.c mft.c
+ encoding.c print.c validate.c cert.c mft.c repo-dummy.c
run-regress-test-aspa: test-aspa
./test-aspa -v ${.CURDIR}/../aspa/*.asa
Index: repo-dummy.c
===================================================================
RCS file: repo-dummy.c
diff -N repo-dummy.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ repo-dummy.c 14 Dec 2022 12:52:23 -0000
@@ -0,0 +1,24 @@
+/*
+ * Public domain
+ * dummy shim for some tests.
+ */
+
+#include "extern.h"
+
+void
+repo_stat_inc(struct repo *repo, enum rtype type, enum stype subtype)
+{
+ return;
+}
+
+struct repo *
+repo_byid(unsigned int id)
+{
+ return NULL;
+}
+
+unsigned int
+repo_id(const struct repo *repo)
+{
+ return 0;
+}
