Skip to content

Commit a5b8b9b

Browse files
chrischdichrischdi
committed
metrics: fixes to markers, adds proper godocs, enables help text generation
1 parent bb189ef commit a5b8b9b

13 files changed

+315
-157
lines changed

‎pkg/metrics/markers/gvk.go

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package markers
1818
import (
1919
"sigs.k8s.io/controller-tools/pkg/markers"
2020

21-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
21+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2222
)
2323

2424
const (
@@ -35,26 +35,18 @@ func init() {
3535
)
3636
}
3737

38-
// gvkMarker implements ResourceMarker to opt-in metric generation for a gvk and configure a name prefix.
38+
// +controllertools:marker:generateHelp:category=Metrics
39+
40+
// gvkMarker enables the creation of a custom resource configuration entry and uses the given prefix for the metrics if configured.
3941
type gvkMarker struct {
42+
// NamePrefix specifies the prefix for all metrics of this resource.
43+
// Note: This field directly maps to the metricNamePrefix field in the resource's custom resource configuration.
4044
NamePrefix string `marker:"namePrefix,optional"`
4145
}
4246

4347
var _ ResourceMarker = gvkMarker{}
4448

45-
// Help prints the help information for the gvkMarker.
46-
func (gvkMarker) Help() *markers.DefinitionHelp {
47-
return &markers.DefinitionHelp{
48-
Category: "Metrics",
49-
DetailedHelp: markers.DetailedHelp{
50-
Summary: "enables the creation of a customresourcestate Resource for the CRD and uses the given prefix for the metrics if configured.",
51-
Details: "",
52-
},
53-
FieldHelp: map[string]markers.DetailedHelp{},
54-
}
55-
}
56-
57-
func (n gvkMarker) ApplyToResource(resource *customresourcestate.Resource) error {
49+
func (n gvkMarker) ApplyToResource(resource *config.Resource) error {
5850
if n.NamePrefix != "" {
5951
resource.MetricNamePrefix = &n.NamePrefix
6052
}

‎pkg/metrics/markers/helper.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
"k8s.io/client-go/util/jsonpath"
2222
ctrlmarkers "sigs.k8s.io/controller-tools/pkg/markers"
2323

24-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
24+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2525
)
2626

2727
type markerDefinitionWithHelp struct {
@@ -88,12 +88,12 @@ func (j jsonPath) Parse() ([]string, error) {
8888
return ret, nil
8989
}
9090

91-
func newMetricMeta(basePath []string, j jsonPath, jsonLabelsFromPath map[string]jsonPath) (customresourcestate.MetricMeta, error) {
91+
func newMetricMeta(basePath []string, j jsonPath, jsonLabelsFromPath map[string]jsonPath) (config.MetricMeta, error) {
9292
path := basePath
9393
if j != "" {
9494
valueFrom, err := j.Parse()
9595
if err != nil {
96-
return customresourcestate.MetricMeta{}, fmt.Errorf("failed to parse JSONPath %q", j)
96+
return config.MetricMeta{}, fmt.Errorf("failed to parse JSONPath %q", j)
9797
}
9898
if len(valueFrom) > 0 {
9999
path = append(path, valueFrom...)
@@ -107,13 +107,13 @@ func newMetricMeta(basePath []string, j jsonPath, jsonLabelsFromPath map[string]
107107
if v != "." {
108108
path, err = v.Parse()
109109
if err != nil {
110-
return customresourcestate.MetricMeta{}, fmt.Errorf("failed to parse JSONPath %q", v)
110+
return config.MetricMeta{}, fmt.Errorf("failed to parse JSONPath %q", v)
111111
}
112112
}
113113
labelsFromPath[k] = path
114114
}
115115

116-
return customresourcestate.MetricMeta{
116+
return config.MetricMeta{
117117
Path: path,
118118
LabelsFromPath: labelsFromPath,
119119
}, nil

‎pkg/metrics/markers/helper_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"reflect"
2020
"testing"
2121

22-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
22+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2323
)
2424

2525
func Test_jsonPath_Parse(t *testing.T) {
@@ -78,14 +78,14 @@ func Test_newMetricMeta(t *testing.T) {
7878
basePath []string
7979
j jsonPath
8080
jsonLabelsFromPath map[string]jsonPath
81-
want customresourcestate.MetricMeta
81+
want config.MetricMeta
8282
}{
8383
{
8484
name: "with basePath and jsonpath, without jsonLabelsFromPath",
8585
basePath: []string{"foo"},
8686
j: jsonPath(".bar"),
8787
jsonLabelsFromPath: map[string]jsonPath{},
88-
want: customresourcestate.MetricMeta{
88+
want: config.MetricMeta{
8989
Path: []string{"foo", "bar"},
9090
LabelsFromPath: map[string][]string{},
9191
},
@@ -95,7 +95,7 @@ func Test_newMetricMeta(t *testing.T) {
9595
basePath: []string{"foo"},
9696
j: jsonPath(".bar"),
9797
jsonLabelsFromPath: map[string]jsonPath{"some": ".label.from.path"},
98-
want: customresourcestate.MetricMeta{
98+
want: config.MetricMeta{
9999
Path: []string{"foo", "bar"},
100100
LabelsFromPath: map[string][]string{
101101
"some": {"label", "from", "path"},
@@ -107,7 +107,7 @@ func Test_newMetricMeta(t *testing.T) {
107107
basePath: []string{},
108108
j: jsonPath(""),
109109
jsonLabelsFromPath: map[string]jsonPath{},
110-
want: customresourcestate.MetricMeta{
110+
want: config.MetricMeta{
111111
Path: []string{},
112112
LabelsFromPath: map[string][]string{},
113113
},

‎pkg/metrics/markers/labelfrompath.go

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121

2222
"sigs.k8s.io/controller-tools/pkg/markers"
2323

24-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
24+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2525
)
2626

2727
const (
@@ -38,28 +38,19 @@ func init() {
3838
)
3939
}
4040

41-
// labelFromPathMarker is the marker to configure a labelFromPath for a gvk.
41+
// +controllertools:marker:generateHelp:category=Metrics
42+
43+
// labelFromPathMarker specifies additional labels for all metrics of this field or type.
4244
type labelFromPathMarker struct {
43-
// +Metrics:labelFromPath:name=<string>,JSONPath=<string> on API type struct
44-
Name string
45+
// Name specifies the name of the label.
46+
Name string
47+
// JSONPath specifies the relative path to the value for the label.
4548
JSONPath jsonPath `marker:"JSONPath"`
4649
}
4750

4851
var _ ResourceMarker = labelFromPathMarker{}
4952

50-
// Help prints the help information for the LabelFromPathMarker.
51-
func (labelFromPathMarker) Help() *markers.DefinitionHelp {
52-
return &markers.DefinitionHelp{
53-
Category: "Metrics",
54-
DetailedHelp: markers.DetailedHelp{
55-
Summary: "adds an additional label to all metrics of this field or type with a value from the given JSONPath.",
56-
Details: "",
57-
},
58-
FieldHelp: map[string]markers.DetailedHelp{},
59-
}
60-
}
61-
62-
func (n labelFromPathMarker) ApplyToResource(resource *customresourcestate.Resource) error {
53+
func (n labelFromPathMarker) ApplyToResource(resource *config.Resource) error {
6354
if resource == nil {
6455
return errors.New("expected resource to not be nil")
6556
}

‎pkg/metrics/markers/labelfrompath_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"reflect"
2020
"testing"
2121

22-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
22+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2323
)
2424

2525
func Test_labelFromPathMarker_ApplyToResource(t *testing.T) {
@@ -30,8 +30,8 @@ func Test_labelFromPathMarker_ApplyToResource(t *testing.T) {
3030
tests := []struct {
3131
name string
3232
fields fields
33-
resource *customresourcestate.Resource
34-
wantResource *customresourcestate.Resource
33+
resource *config.Resource
34+
wantResource *config.Resource
3535
wantErr bool
3636
}{
3737
{
@@ -40,9 +40,9 @@ func Test_labelFromPathMarker_ApplyToResource(t *testing.T) {
4040
Name: "foo",
4141
JSONPath: ".bar",
4242
},
43-
resource: &customresourcestate.Resource{},
44-
wantResource: &customresourcestate.Resource{
45-
Labels: customresourcestate.Labels{
43+
resource: &config.Resource{},
44+
wantResource: &config.Resource{
45+
Labels: config.Labels{
4646
LabelsFromPath: map[string][]string{
4747
"foo": {"bar"},
4848
},
@@ -56,15 +56,15 @@ func Test_labelFromPathMarker_ApplyToResource(t *testing.T) {
5656
Name: "foo",
5757
JSONPath: ".bar",
5858
},
59-
resource: &customresourcestate.Resource{
60-
Labels: customresourcestate.Labels{
59+
resource: &config.Resource{
60+
Labels: config.Labels{
6161
LabelsFromPath: map[string][]string{
6262
"foo": {"other"},
6363
},
6464
},
6565
},
66-
wantResource: &customresourcestate.Resource{
67-
Labels: customresourcestate.Labels{
66+
wantResource: &config.Resource{
67+
Labels: config.Labels{
6868
LabelsFromPath: map[string][]string{
6969
"foo": {"other"},
7070
},
@@ -78,15 +78,15 @@ func Test_labelFromPathMarker_ApplyToResource(t *testing.T) {
7878
Name: "foo",
7979
JSONPath: ".bar",
8080
},
81-
resource: &customresourcestate.Resource{
82-
Labels: customresourcestate.Labels{
81+
resource: &config.Resource{
82+
Labels: config.Labels{
8383
LabelsFromPath: map[string][]string{
8484
"foo": {"other", "path"},
8585
},
8686
},
8787
},
88-
wantResource: &customresourcestate.Resource{
89-
Labels: customresourcestate.Labels{
88+
wantResource: &config.Resource{
89+
Labels: config.Labels{
9090
LabelsFromPath: map[string][]string{
9191
"foo": {"other", "path"},
9292
},
@@ -100,8 +100,8 @@ func Test_labelFromPathMarker_ApplyToResource(t *testing.T) {
100100
Name: "foo",
101101
JSONPath: "{.bar}",
102102
},
103-
resource: &customresourcestate.Resource{},
104-
wantResource: &customresourcestate.Resource{},
103+
resource: &config.Resource{},
104+
wantResource: &config.Resource{},
105105
wantErr: true,
106106
},
107107
{

‎pkg/metrics/markers/markers.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package markers
1818
import (
1919
"sigs.k8s.io/controller-tools/pkg/markers"
2020

21-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
21+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2222
)
2323

2424
var (
@@ -36,12 +36,12 @@ var (
3636
type ResourceMarker interface {
3737
// ApplyToCRD applies this marker to the given CRD, in the given version
3838
// within that CRD. It's called after everything else in the CRD is populated.
39-
ApplyToResource(resource *customresourcestate.Resource) error
39+
ApplyToResource(resource *config.Resource) error
4040
}
4141

4242
// LocalGeneratorMarker is a marker that creates a custom resource metric generator.
4343
type LocalGeneratorMarker interface {
4444
// ApplyToCRD applies this marker to the given CRD, in the given version
4545
// within that CRD. It's called after everything else in the CRD is populated.
46-
ToGenerator(basePath ...string) (*customresourcestate.Generator, error)
46+
ToGenerator(basePath ...string) (*config.Generator, error)
4747
}

‎pkg/metrics/markers/metric_gauge.go

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020

2121
"sigs.k8s.io/controller-tools/pkg/markers"
2222

23-
customresourcestate "sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
23+
"sigs.k8s.io/controller-tools/pkg/metrics/internal/config"
2424
)
2525

2626
const (
@@ -31,43 +31,47 @@ func init() {
3131
MarkerDefinitions = append(
3232
MarkerDefinitions,
3333
must(markers.MakeDefinition(gaugeMarkerName, markers.DescribesField, gaugeMarker{})).
34-
help(gaugeMarker{}.help()),
34+
help(gaugeMarker{}.Help()),
3535
must(markers.MakeDefinition(gaugeMarkerName, markers.DescribesType, gaugeMarker{})).
36-
help(gaugeMarker{}.help()),
36+
help(gaugeMarker{}.Help()),
3737
)
3838
}
3939

40-
// gaugeMarker implements localGeneratorMarker to generate a gauge type metric.
40+
// +controllertools:marker:generateHelp:category=Metric type Gauge
41+
42+
// gaugeMarker defines a Gauge metric and uses the implicit path to the field joined by the provided JSONPath as path for the metric configuration.
43+
// Gauge is a metric which targets a Path that may be a single value, array, or object.
44+
// Arrays and objects will generate a metric per element and requre ValueFrom to be set.
45+
// Ref: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#gauge
4146
type gaugeMarker struct {
47+
// Keys from the Generator struct.
48+
49+
// Name specifies the Name of the metric.
4250
Name string
43-
Help string `marker:"help,optional"`
51+
// MetricHelp specifies the help text for the metric.
52+
MetricHelp string `marker:"help,optional"`
4453

4554
// Keys from the MetricMeta struct.
4655

56+
// LabelsFromPath specifies additional labels where the value is taken from the given JSONPath.
4757
LabelsFromPath map[string]jsonPath `marker:"labelsFromPath,optional"`
48-
JSONPath jsonPath `marker:"JSONPath,optional"`
58+
// JSONPath specifies the relative path from this marker.
59+
// Note: This field get's appended to the path field in the custom resource configuration.
60+
JSONPath jsonPath `marker:"JSONPath,optional"`
4961

5062
// Keys from the MetricGauge struct.
5163

52-
ValueFrom *jsonPath `marker:"valueFrom,optional"`
53-
LabelFromKey string `marker:"labelFromKey,optional"`
54-
NilIsZero bool `marker:"nilIsZero,optional"`
64+
// ValueFrom specifies the JSONPath to a numeric field that will be the metric value.
65+
ValueFrom *jsonPath `marker:"valueFrom,optional"`
66+
// LabelFromKey specifies a label which will be added to the metric having the object's key as value.
67+
LabelFromKey string `marker:"labelFromKey,optional"`
68+
// NilIsZero specifies to treat a not-existing field as zero value.
69+
NilIsZero bool `marker:"nilIsZero,optional"`
5570
}
5671

5772
var _ LocalGeneratorMarker = &gaugeMarker{}
5873

59-
func (gaugeMarker) help() *markers.DefinitionHelp {
60-
return &markers.DefinitionHelp{
61-
Category: "Metrics",
62-
DetailedHelp: markers.DetailedHelp{
63-
Summary: "Defines a Gauge metric and uses the implicit path to the field joined by the provided JSONPath as path for the metric configuration.",
64-
Details: "",
65-
},
66-
FieldHelp: map[string]markers.DetailedHelp{},
67-
}
68-
}
69-
70-
func (g gaugeMarker) ToGenerator(basePath ...string) (*customresourcestate.Generator, error) {
74+
func (g gaugeMarker) ToGenerator(basePath ...string) (*config.Generator, error) {
7175
var err error
7276
var valueFrom []string
7377
if g.ValueFrom != nil {
@@ -82,12 +86,12 @@ func (g gaugeMarker) ToGenerator(basePath ...string) (*customresourcestate.Gener
8286
return nil, err
8387
}
8488

85-
return &customresourcestate.Generator{
89+
return &config.Generator{
8690
Name: g.Name,
87-
Help: g.Help,
88-
Each: customresourcestate.Metric{
89-
Type: customresourcestate.MetricTypeGauge,
90-
Gauge: &customresourcestate.MetricGauge{
91+
Help: g.MetricHelp,
92+
Each: config.Metric{
93+
Type: config.MetricTypeGauge,
94+
Gauge: &config.MetricGauge{
9195
NilIsZero: g.NilIsZero,
9296
MetricMeta: meta,
9397
LabelFromKey: g.LabelFromKey,

0 commit comments

Comments
 (0)