On Wed, Mar 29, 2023 at 10:13:22AM +0200, Theo Buehler wrote:
> On Wed, Mar 29, 2023 at 09:51:56AM +0200, Claudio Jeker wrote:
> > In the metrics file the TA is currently reported as:
> > rpki_client_repository_objects{type="cert",state="valid",name="ripe",carepo="ripe"}
> >  1
> > 
> > The carepo which is a rsync URI for other repositories is just the TAL
> > name. Instead it may be better to display the first URI from the TAL,
> > like:
> > 
> > rpki_client_repository_objects{type="cert",state="valid",name="ripe",carepo="https://rpki.ripe.net/ta/ripe-ncc-ta.cer"}
> >  1
> > 
> > Other objects point to the base name of the repository:
> > rpki_client_repository_objects{type="vrp",state="unique",name="ripe",carepo="rsync://rpki.ripe.net/repository",notify="https://rrdp.ripe.net/notification.xml"}
> >  189062
> > 
> > I think this adds a bit of consistency to the metrics output.
> 
> Isn't it a bit strange that repouri contains something that isn't a URI?
> Feels like a trap.

Good point. There is no great reason for this apart from the fact that TAL
URI are strange. So maybe the following diff is better. 

My quick testrun did not show any difference apart from the URI in the
metrics file.
-- 
:wq Claudio

Index: repo.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
retrieving revision 1.41
diff -u -p -r1.41 repo.c
--- repo.c      28 Dec 2022 21:30:18 -0000      1.41
+++ repo.c      29 Mar 2023 15:47:45 -0000
@@ -1012,18 +1012,18 @@ ta_lookup(int id, struct tal *tal)
 
        /* Look up in repository table. (Lookup should actually fail here) */
        SLIST_FOREACH(rp, &repos, entry) {
-               if (strcmp(rp->repouri, tal->descr) == 0)
+               if (strcmp(rp->repouri, tal->uri[0]) == 0)
                        return rp;
        }
 
        rp = repo_alloc(id);
        rp->basedir = repo_dir(tal->descr, "ta", 0);
-       if ((rp->repouri = strdup(tal->descr)) == NULL)
+       if ((rp->repouri = strdup(tal->uri[0])) == NULL)
                err(1, NULL);
 
        /* check if sync disabled ... */
        if (noop) {
-               logx("ta/%s: using cache", rp->repouri);
+               logx("%s: using cache", rp->basedir);
                entityq_flush(&rp->queue, rp);
                return rp;
        }

Reply via email to