From 7c023e6e7e913da8071412f5d97e6ed43a18bcc2 Mon Sep 17 00:00:00 2001 From: Joel Unzain Date: Mon, 20 Jul 2020 16:28:22 -0700 Subject: [PATCH] Add new labels to hardware_model metric (#498) * Add new labels to metric * Update tests * Release notes. --- CHANGELOG.md | 12 +++++++-- convey/conveymetric/metric.go | 41 +++++++++++++++++------------- convey/conveymetric/metric_test.go | 20 +++++++-------- device/manager.go | 13 +++++++--- device/manager_test.go | 2 +- device/metrics.go | 2 +- 6 files changed, 56 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61004587..d192055e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,13 +5,20 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] + + +## [v1.10.4] ### Fixed -- added default port for creating a xresolver connection [#499](https://github.com/xmidt-org/webpa-common/pull/499) +- Added default port for creating a xresolver connection. [#499](https://github.com/xmidt-org/webpa-common/pull/499) + +### Added +- Add partner and firmware label to existing `hardware_model` gauge. [#498](https://github.com/xmidt-org/webpa-common/pull/498) ### Changed - Update word from blacklist to denylist. [#495](https://github.com/xmidt-org/webpa-common/pull/495) - Update references to the main branch. [#497](https://github.com/xmidt-org/webpa-common/pull/497) + ## [v1.10.3] ### Fixed - Fixed xresolver failing to create route when default port is used. [#494](https://github.com/xmidt-org/webpa-common/pull/494) @@ -116,7 +123,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - The first official release. We will be better about documenting changes moving forward. -[Unreleased]: https://github.com/xmidt-org/webpa-common/compare/v1.10.3...HEAD +[Unreleased]: https://github.com/xmidt-org/webpa-common/compare/v1.10.4...HEAD +[v1.10.4]: https://github.com/xmidt-org/webpa-common/compare/v1.10.3...v1.10.4 [v1.10.3]: https://github.com/xmidt-org/webpa-common/compare/v1.10.2...v1.10.3 [v1.10.2]: https://github.com/xmidt-org/webpa-common/compare/v1.10.1...v1.10.2 [v1.10.1]: https://github.com/xmidt-org/webpa-common/compare/v1.10.0...v1.10.1 diff --git a/convey/conveymetric/metric.go b/convey/conveymetric/metric.go index d3e7bca8..e08508e2 100644 --- a/convey/conveymetric/metric.go +++ b/convey/conveymetric/metric.go @@ -5,46 +5,53 @@ import ( "github.com/xmidt-org/webpa-common/convey" ) -// UnknownLabel is a constant for when key/tag can not be found in the C JSON -const UnknownLabel = "unknown" +// UnknownLabelValue is a constant for when key/tag can not be found in the C JSON. +const UnknownLabelValue = "unknown" -// Closure will be returned after Update(), this should be used to update the struct, aka decrement the count +// Closure will be returned after Update(), this should be used to update the struct, aka decrement the count. type Closure func() -// Interface provides a way of updating an internal resource +// TagLabelPair is a convenient structure for inputs to create a new convey metric. +type TagLabelPair struct { + Tag string + Label string +} + +// Interface provides a way of updating an internal resource. type Interface interface { // Update takes the convey JSON to update internal struct, and return a closure to update the struct again, or an // error // // Note: Closure should only be called once. - Update(data convey.C) (Closure, error) + Update(data convey.C, labelPairs ...string) (Closure, error) } // NewConveyMetric produces an Interface where gauge is the internal structure to update, tag is the key in the C JSON // to update the gauge, and label is the `key` for the gauge cardinality. // // Note: The Gauge must have the label as one of the constant labels, (aka. the name of the gauge) -func NewConveyMetric(gauge metrics.Gauge, tag string, label string) Interface { +func NewConveyMetric(gauge metrics.Gauge, pairs ...TagLabelPair) Interface { return &cMetric{ - tag: tag, - label: label, + pairs: pairs, gauge: gauge, } } // cMetric is the internal Interface implementation type cMetric struct { - tag string - label string + pairs []TagLabelPair gauge metrics.Gauge } -func (m *cMetric) Update(data convey.C) (Closure, error) { - key := UnknownLabel - if item, ok := data[m.tag].(string); ok { - key = item +func (m *cMetric) Update(data convey.C, baseLabelPairs ...string) (Closure, error) { + labelPairs := baseLabelPairs + for _, pair := range m.pairs { + labelValue := UnknownLabelValue + if item, ok := data[pair.Tag].(string); ok { + labelValue = item + } + labelPairs = append(labelPairs, pair.Label, labelValue) } - - m.gauge.With(m.label, key).Add(1.0) - return func() { m.gauge.With(m.label, key).Add(-1.0) }, nil + m.gauge.With(labelPairs...).Add(1.0) + return func() { m.gauge.With(labelPairs...).Add(-1.0) }, nil } diff --git a/convey/conveymetric/metric_test.go b/convey/conveymetric/metric_test.go index bf1209a4..164b0872 100644 --- a/convey/conveymetric/metric_test.go +++ b/convey/conveymetric/metric_test.go @@ -1,10 +1,11 @@ package conveymetric import ( + "testing" + "github.com/xmidt-org/webpa-common/convey" "github.com/xmidt-org/webpa-common/xmetrics" "github.com/xmidt-org/webpa-common/xmetrics/xmetricstest" - "testing" "github.com/stretchr/testify/assert" ) @@ -12,25 +13,24 @@ import ( func TestConveyMetric(t *testing.T) { assert := assert.New(t) - //namespace, subsystem, name := "test", "basic", "hardware" - gauge := xmetricstest.NewGauge("hardware") - conveyMetric := NewConveyMetric(gauge, "hw-model", "model") + conveyMetric := NewConveyMetric(gauge, []TagLabelPair{{"hw-model", "model"}, {"fw-name", "firmware"}}...) - dec, err := conveyMetric.Update(convey.C{"data": "neat", "hw-model": "hardware123abc"}) + dec, err := conveyMetric.Update(convey.C{"data": "neat", "hw-model": "hardware123abc", "fw-name": "firmware-xyz"}) assert.NoError(err) - assert.Equal(float64(1), gauge.With("model", "hardware123abc").(xmetrics.Valuer).Value()) + assert.Equal(float64(1), gauge.With("model", "hardware123abc", "firmware", "firmware-xyz").(xmetrics.Valuer).Value()) // remove the update dec() - assert.Equal(float64(0), gauge.With("model", "hardware123abc").(xmetrics.Valuer).Value()) + assert.Equal(float64(0), gauge.With("model", "hardware123abc", "firmware", "firmware-xyz").(xmetrics.Valuer).Value()) // try with no `hw_model` - dec, err = conveyMetric.Update(convey.C{"data": "neat"}) - assert.Equal(float64(1), gauge.With("model", UnknownLabel).(xmetrics.Valuer).Value()) + dec, err = conveyMetric.Update(convey.C{"data": "neat", "fw-name": "firmware-abc"}) + t.Logf("%v+", gauge) + assert.Equal(float64(1), gauge.With("model", UnknownLabelValue, "firmware", "firmware-abc").(xmetrics.Valuer).Value()) // remove the update dec() - assert.Equal(float64(0), gauge.With("model", UnknownLabel).(xmetrics.Valuer).Value()) + assert.Equal(float64(0), gauge.With("model", UnknownLabelValue, "firmware", "firmware-abc").(xmetrics.Valuer).Value()) } diff --git a/device/manager.go b/device/manager.go index b60c9556..63905fac 100644 --- a/device/manager.go +++ b/device/manager.go @@ -103,7 +103,15 @@ func NewManager(o *Options) Manager { Limit: o.maxDevices(), Measures: measures, }), - conveyHWMetric: conveymetric.NewConveyMetric(measures.Models, "hw-model", "model"), + conveyHWMetric: conveymetric.NewConveyMetric(measures.Models, []conveymetric.TagLabelPair{ + { + Tag: "hw-model", + Label: "model", + }, + { + Tag: "fw-name", + Label: "firmware", + }}...), deviceMessageQueueSize: o.deviceMessageQueueSize(), pingPeriod: o.pingPeriod(), @@ -208,8 +216,7 @@ func (m *manager) Connect(response http.ResponseWriter, request *http.Request, r d.errorLog.Log(logging.MessageKey(), "unable to marshal the convey header", logging.ErrorKey(), err) } } - - metricClosure, err := m.conveyHWMetric.Update(cvy) + metricClosure, err := m.conveyHWMetric.Update(cvy, "partnerid", metadata.PartnerIDClaim()) if err != nil { d.errorLog.Log(logging.MessageKey(), "failed to update convey metrics", logging.ErrorKey(), err) } diff --git a/device/manager_test.go b/device/manager_test.go index fe091125..2ba35c1c 100644 --- a/device/manager_test.go +++ b/device/manager_test.go @@ -386,7 +386,7 @@ func TestGaugeCardinality(t *testing.T) { assert.NoError(err) assert.NotPanics(func() { - dec, err := m.(*manager).conveyHWMetric.Update(convey.C{"hw-model": "cardinality", "model": "f"}) + dec, err := m.(*manager).conveyHWMetric.Update(convey.C{"hw-model": "cardinality", "fw-name": "firmware-number", "model": "f"}, "partnerid", "comcast") assert.NoError(err) dec() }) diff --git a/device/metrics.go b/device/metrics.go index ca25c027..282e65a5 100644 --- a/device/metrics.go +++ b/device/metrics.go @@ -56,7 +56,7 @@ func Metrics() []xmetrics.Metric { { Name: ModelGauge, Type: "gauge", - LabelNames: []string{"model"}, + LabelNames: []string{"model", "partnerid", "firmware"}, }, } }