admin: Add Chart Catalog infrastructure #27505
Conversation
|
This change is |
|
It looks like maybe you haven't included some new files? Without looking too much into the specifics, I wonder if this is too focused on the display of the charts? Along the same lines of the direction I was pushing on the previous PR, I think this should be articulated as a catalog of metrics without respect to the manner in which they are displayed. The latter introduces a whole set of thorny issues that can be avoided by treating this as organizational, not presentational. |
sploiselle
requested review from
spencerkimball and
vilterp
and removed request for
danhhz,
couchand and
mrtracy
|
@spencerkimball Here's the actual "catalog" onto which you should be able to graft this notion of "components." Obviously, let me know if there's a more optimal way to structure this to facilitate that or if there's anyone else I should tag to review it. @vilterp Do you mind taking a look at this given that it does interface with the admin UI in some capacity? |
pkg/ts/catalog/catalog_generator.go
Outdated
| // Organization identifies the sections in which to place the chart. | ||
| // (Sections are created ad hoc as ChartSections whenever they're identified in | ||
| // a chartDescription.) | ||
| // Inner array is Level 0 (req), Level 1 (req), Level 2 (opt). |
There was a problem hiding this comment.
Not sure what these levels mean. Could you augment the comment?
There was a problem hiding this comment.
Added much more context to the general structure of the project.
|
@spencerkimball If there are any metrics that don't appear in at least one chart, there is a test that fails. Hoping that doesn't disincentivize creating metrics in the first place, but seems reasonable to ensure that the catalog doesn't wither in the face of progress. |
Create internal mechanisms to define and serve a predefined set of charts that could be used by the Admin UI. Release note: none
|
bors r+ |
27505: admin: Add Chart Catalog infrastructure r=sploiselle a=sploiselle To support creating a catalog of predefined charts to aid in debugging, this PR introduces the internal mechanisms by which we can define and generate a catalog of charts. This PR does not contain an exhaustive catalog, but instead only an illustrative subset of the catalog to demonstrate the code's ergonomics. I will add the remainder of the catalog once this code is reviewed. The provided tests will also be expanded on in a subsequent commit. @danhhz The library of consts that you proposed grew really unwieldy given the size of the final catalog (~150 distinct values). I'd love to make sure this is as easy as possible to use, though, so please include any other suggestions. @mrtracy Can I get you to take a peek and ensure that `pkg/ts/catalog/chart_catalog.go`'s [`chartDefaultsPerMetricType`](https://github.com/cockroachdb/cockroach/compare/master...sploiselle:chart-catalog-endpoint?expand=1#diff-fc032d10a16db3caf6e19e81a2549edeR57) has sensible defaults? Release note: none Co-authored-by: Sean Loiselle <himself@seanloiselle.com>
Build succeeded |
To support creating a catalog of predefined charts to aid in debugging, this PR introduces the
internal mechanisms by which we can define and generate a catalog of charts.
This PR does not contain an exhaustive catalog, but instead only an illustrative subset of the
catalog to demonstrate the code's ergonomics. I will add the remainder of the catalog once
this code is reviewed.
The provided tests will also be expanded on in a subsequent commit.
@danhhz The library of consts that you proposed grew really unwieldy given the size of the final catalog (~150 distinct values). I'd love to make sure this is as easy as possible to use, though, so please include any other suggestions.
@mrtracy Can I get you to take a peek and ensure that
pkg/ts/catalog/chart_catalog.go'schartDefaultsPerMetricTypehas sensible defaults?Release note: none