On Wed, Dec 14, 2022 at 06:40:19PM +0100, Claudio Jeker wrote:
> On Wed, Dec 14, 2022 at 05:54:54PM +0100, Theo Buehler wrote:
> > 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.
> 
> Thanks.
> 

...

> > > + 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.
> 
> I added an assert and added an if (rp != NULL) check in roa_insert_vrps().
  
Had to remove the assert() it triggered immediatly.

assertion "rp != NULL" failed: file main.c, line 564, function entity_process

I think tal files have no repoid set and so this fails for at least them.

-- 
:wq Claudio

Reply via email to