From 51ac1bf6c7229b49a120ba40a73f80023dfbd9f5 Mon Sep 17 00:00:00 2001 From: dan pittman Date: Fri, 4 Dec 2015 11:50:11 -0800 Subject: [PATCH] fix #526: adds lookup for GetMetricVersions This commit adds a lookup to fetch all metrics of a given namespace without traversing to the leaves and collecting children. --- cmd/snapctl/commands.go | 6 ++++- cmd/snapctl/metric.go | 49 +++++++++++----------------------- control/control.go | 34 ++++++++++++++++++----- control/control_test.go | 14 ++++++---- control/metrics.go | 49 +++++++++++++++++++++------------- control/metrics_test.go | 5 ---- control/mttrie.go | 23 +++++----------- control/mttrie_test.go | 3 --- control/plugin_manager_test.go | 4 +-- mgmt/rest/client/metric.go | 49 ++++++++++++++++++++++++++++++++++ mgmt/rest/metric.go | 36 ++++++++++++++++++------- mgmt/rest/server.go | 1 + 12 files changed, 171 insertions(+), 102 deletions(-) diff --git a/cmd/snapctl/commands.go b/cmd/snapctl/commands.go index e556dee68..6d0c60e08 100644 --- a/cmd/snapctl/commands.go +++ b/cmd/snapctl/commands.go @@ -219,7 +219,11 @@ func printFields(tw *tabwriter.Writer, indent bool, width int, fields ...interfa argArray = append(argArray, strings.Repeat(" ", width)) } for i, field := range fields { - argArray = append(argArray, field) + if field != nil { + argArray = append(argArray, field) + } else { + argArray = append(argArray, "") + } if i < (len(fields) - 1) { argArray = append(argArray, "\t") } diff --git a/cmd/snapctl/metric.go b/cmd/snapctl/metric.go index 4d8be70cf..d5412df6b 100644 --- a/cmd/snapctl/metric.go +++ b/cmd/snapctl/metric.go @@ -54,8 +54,8 @@ func listMetrics(ctx *cli.Context) { /* NAMESPACE VERSION - /intel/mock/foo 1,2 - /intel/mock/bar 1 + /intel/mock/foo 1,2 + /intel/mock/bar 1 */ w := tabwriter.NewWriter(os.Stdout, 0, 8, 1, '\t', 0) metsByVer := make(map[string][]string) @@ -78,26 +78,23 @@ func listMetrics(ctx *cli.Context) { } func getMetric(ctx *cli.Context) { - ns := ctx.String("metric-namespace") - ver := ctx.Int("metric-version") - if ns == "" { - fmt.Println("namespace is required") + if !ctx.IsSet("metric-namespace") || !ctx.IsSet("metric-version") { + fmt.Println("namespace and version are required") + fmt.Println("") cli.ShowCommandHelp(ctx, ctx.Command.Name) return } - if ver == 0 { - ver = -1 - } - metrics := pClient.FetchMetrics(ns, ver) - if metrics.Err != nil { - fmt.Println(metrics.Err) + ns := ctx.String("metric-namespace") + ver := ctx.Int("metric-version") + metric := pClient.GetMetric(ns, ver) + if metric.Err != nil { + fmt.Println(metric.Err) return } - metric := metrics.Catalog[0] /* NAMESPACE VERSION LAST ADVERTISED TIME - /intel/mock/foo 2 Wed, 09 Sep 2015 10:01:04 PDT + /intel/mock/foo 2 Wed, 09 Sep 2015 10:01:04 PDT Rules for collecting /intel/mock/foo: @@ -109,28 +106,12 @@ func getMetric(ctx *cli.Context) { w := tabwriter.NewWriter(os.Stdout, 0, 8, 1, '\t', 0) printFields(w, false, 0, "NAMESPACE", "VERSION", "LAST ADVERTISED TIME") - printFields(w, false, 0, metric.Namespace, metric.Version, time.Unix(metric.LastAdvertisedTimestamp, 0).Format(time.RFC1123)) + printFields(w, false, 0, metric.Metric.Namespace, metric.Metric.Version, time.Unix(metric.Metric.LastAdvertisedTimestamp, 0).Format(time.RFC1123)) w.Flush() - fmt.Printf("\n Rules for collecting %s:\n\n", metric.Namespace) + fmt.Printf("\n Rules for collecting %s:\n\n", metric.Metric.Namespace) printFields(w, true, 6, "NAME", "TYPE", "DEFAULT", "REQUIRED", "MINIMUM", "MAXIMUM") - for _, rule := range metric.Policy { - var def_, min_, max_ interface{} - if rule.Default == nil { - def_ = "" - } else { - def_ = rule.Default - } - if rule.Minimum == nil { - min_ = "" - } else { - min_ = rule.Minimum - } - if rule.Maximum == nil { - max_ = "" - } else { - max_ = rule.Maximum - } - printFields(w, true, 6, rule.Name, rule.Type, def_, rule.Required, min_, max_) + for _, rule := range metric.Metric.Policy { + printFields(w, true, 6, rule.Name, rule.Type, rule.Default, rule.Required, rule.Minimum, rule.Maximum) } w.Flush() } diff --git a/control/control.go b/control/control.go index 78c3d2275..f398fa7c8 100644 --- a/control/control.go +++ b/control/control.go @@ -108,16 +108,17 @@ type managesPlugins interface { } type catalogsMetrics interface { - Get([]string, int) (*metricType, serror.SnapError) + Get([]string, int) (*metricType, error) Add(*metricType) AddLoadedMetricType(*loadedPlugin, core.Metric) RmUnloadedPluginMetrics(lp *loadedPlugin) - Fetch([]string) ([]*metricType, serror.SnapError) + GetVersions([]string) ([]*metricType, error) + Fetch([]string) ([]*metricType, error) Item() (string, []*metricType) Next() bool - Subscribe([]string, int) serror.SnapError - Unsubscribe([]string, int) serror.SnapError - GetPlugin([]string, int) (*loadedPlugin, serror.SnapError) + Subscribe([]string, int) error + Unsubscribe([]string, int) error + GetPlugin([]string, int) (*loadedPlugin, error) } type managesSigning interface { @@ -491,7 +492,10 @@ func (p *pluginControl) validateMetricTypeSubscription(mt core.RequestedMetric, m, err := p.metricCatalog.Get(mt.Namespace(), mt.Version()) if err != nil { - serrs = append(serrs, err) + serrs = append(serrs, serror.New(err, map[string]interface{}{ + "name": core.JoinNamespace(mt.Namespace()), + "version": mt.Version(), + })) return nil, serrs } @@ -551,7 +555,10 @@ func (p *pluginControl) gatherCollectors(mts []core.Metric) ([]core.Plugin, []se for _, mt := range mts { m, err := p.metricCatalog.Get(mt.Namespace(), mt.Version()) if err != nil { - serrs = append(serrs, err) + serrs = append(serrs, serror.New(err, map[string]interface{}{ + "name": core.JoinNamespace(mt.Namespace()), + "version": mt.Version(), + })) continue } // if the metric subscription is to version -1, we need to carry @@ -786,6 +793,19 @@ func (p *pluginControl) GetMetric(ns []string, ver int) (core.CatalogedMetric, e return p.metricCatalog.Get(ns, ver) } +func (p *pluginControl) GetMetricVersions(ns []string) ([]core.CatalogedMetric, error) { + mts, err := p.metricCatalog.GetVersions(ns) + if err != nil { + return nil, err + } + + rmts := make([]core.CatalogedMetric, len(mts)) + for i, m := range mts { + rmts[i] = m + } + return rmts, nil +} + func (p *pluginControl) MetricExists(mns []string, ver int) bool { _, err := p.metricCatalog.Get(mns, ver) if err == nil { diff --git a/control/control_test.go b/control/control_test.go index fe3bcb3db..51ee493d4 100644 --- a/control/control_test.go +++ b/control/control_test.go @@ -556,7 +556,7 @@ type mc struct { e int } -func (m *mc) Fetch(ns []string) ([]*metricType, serror.SnapError) { +func (m *mc) Fetch(ns []string) ([]*metricType, error) { if m.e == 2 { return nil, serror.New(errors.New("test")) } @@ -567,11 +567,15 @@ func (m *mc) resolvePlugin(mns []string, ver int) (*loadedPlugin, error) { return nil, nil } -func (m *mc) GetPlugin([]string, int) (*loadedPlugin, serror.SnapError) { +func (m *mc) GetPlugin([]string, int) (*loadedPlugin, error) { return nil, nil } -func (m *mc) Get(ns []string, ver int) (*metricType, serror.SnapError) { +func (m *mc) GetVersions([]string) ([]*metricType, error) { + return nil, nil +} + +func (m *mc) Get(ns []string, ver int) (*metricType, error) { if m.e == 1 { return &metricType{ policy: &mockCDProc{}, @@ -580,14 +584,14 @@ func (m *mc) Get(ns []string, ver int) (*metricType, serror.SnapError) { return nil, serror.New(errorMetricNotFound(ns)) } -func (m *mc) Subscribe(ns []string, ver int) serror.SnapError { +func (m *mc) Subscribe(ns []string, ver int) error { if ns[0] == "nf" { return serror.New(errorMetricNotFound(ns)) } return nil } -func (m *mc) Unsubscribe(ns []string, ver int) serror.SnapError { +func (m *mc) Unsubscribe(ns []string, ver int) error { if ns[0] == "nf" { return serror.New(errorMetricNotFound(ns)) } diff --git a/control/metrics.go b/control/metrics.go index da99c3049..1b7e23487 100644 --- a/control/metrics.go +++ b/control/metrics.go @@ -208,16 +208,23 @@ func (mc *metricCatalog) Add(m *metricType) { mc.tree.Add(m) } -// Get retrieves a loadedPlugin given a namespace and version. +// Get retrieves a metric given a namespace and version. // If provided a version of -1 the latest plugin will be returned. -func (mc *metricCatalog) Get(ns []string, version int) (*metricType, serror.SnapError) { +func (mc *metricCatalog) Get(ns []string, version int) (*metricType, error) { mc.mutex.Lock() defer mc.mutex.Unlock() return mc.get(ns, version) } +// GetVersions retrieves all versions of a given metric namespace. +func (mc *metricCatalog) GetVersions(ns []string) ([]*metricType, error) { + mc.mutex.Lock() + defer mc.mutex.Unlock() + return mc.getVersions(ns) +} + // Fetch transactionally retrieves all metrics which fall under namespace ns -func (mc *metricCatalog) Fetch(ns []string) ([]*metricType, serror.SnapError) { +func (mc *metricCatalog) Fetch(ns []string) ([]*metricType, error) { mc.mutex.Lock() defer mc.mutex.Unlock() @@ -260,7 +267,7 @@ func (mc *metricCatalog) Next() bool { } // Subscribe atomically increments a metric's subscription count in the table. -func (mc *metricCatalog) Subscribe(ns []string, version int) serror.SnapError { +func (mc *metricCatalog) Subscribe(ns []string, version int) error { mc.mutex.Lock() defer mc.mutex.Unlock() @@ -274,7 +281,7 @@ func (mc *metricCatalog) Subscribe(ns []string, version int) serror.SnapError { } // Unsubscribe atomically decrements a metric's count in the table -func (mc *metricCatalog) Unsubscribe(ns []string, version int) serror.SnapError { +func (mc *metricCatalog) Unsubscribe(ns []string, version int) error { mc.mutex.Lock() defer mc.mutex.Unlock() @@ -286,7 +293,7 @@ func (mc *metricCatalog) Unsubscribe(ns []string, version int) serror.SnapError return m.Unsubscribe() } -func (mc *metricCatalog) GetPlugin(mns []string, ver int) (*loadedPlugin, serror.SnapError) { +func (mc *metricCatalog) GetPlugin(mns []string, ver int) (*loadedPlugin, error) { m, err := mc.Get(mns, ver) if err != nil { return nil, err @@ -294,24 +301,17 @@ func (mc *metricCatalog) GetPlugin(mns []string, ver int) (*loadedPlugin, serror return m.Plugin, nil } -func (mc *metricCatalog) get(ns []string, ver int) (*metricType, serror.SnapError) { - mts, err := mc.tree.Get(ns) +func (mc *metricCatalog) get(ns []string, ver int) (*metricType, error) { + mts, err := mc.getVersions(ns) if err != nil { return nil, err } - if mts == nil { - return nil, serror.New(errMetricNotFound) - } + // a version IS given if ver > 0 { l, err := getVersion(mts, ver) if err != nil { - se := serror.New(errorMetricNotFound(ns, ver)) - se.SetFields(map[string]interface{}{ - "name": core.JoinNamespace(ns), - "version": ver, - }) - return nil, se + return nil, errorMetricNotFound(ns, ver) } return l, nil } @@ -319,6 +319,17 @@ func (mc *metricCatalog) get(ns []string, ver int) (*metricType, serror.SnapErro return getLatest(mts), nil } +func (mc *metricCatalog) getVersions(ns []string) ([]*metricType, error) { + mts, err := mc.tree.Get(ns) + if err != nil { + return nil, err + } + if mts == nil { + return nil, errMetricNotFound + } + return mts, nil +} + func getMetricKey(metric []string) string { return strings.Join(metric, ".") } @@ -343,11 +354,11 @@ func appendIfMissing(keys []string, ns string) []string { return append(keys, ns) } -func getVersion(c []*metricType, ver int) (*metricType, serror.SnapError) { +func getVersion(c []*metricType, ver int) (*metricType, error) { for _, m := range c { if m.Plugin.Version() == ver { return m, nil } } - return nil, serror.New(errMetricNotFound) + return nil, errMetricNotFound } diff --git a/control/metrics_test.go b/control/metrics_test.go index 05d51c168..735be2e77 100644 --- a/control/metrics_test.go +++ b/control/metrics_test.go @@ -140,8 +140,6 @@ func TestMetricCatalog(t *testing.T) { mc.Add(m35) _, err := mc.Get([]string{"foo", "bar"}, 7) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/foo/bar") - So(err.Fields()["version"], ShouldEqual, 7) }) }) Convey("metricCatalog.Table()", t, func() { @@ -164,7 +162,6 @@ func TestMetricCatalog(t *testing.T) { _mt, err := mc.Get(ns, -1) So(_mt, ShouldBeNil) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/test") }) }) Convey("metricCatalog.Next()", t, func() { @@ -243,7 +240,6 @@ func TestSubscribe(t *testing.T) { Convey("then it returns an error", func() { err := mc.Subscribe([]string{"test4"}, -1) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/test4") }) }) Convey("when the metric is in the table", t, func() { @@ -289,7 +285,6 @@ func TestUnsubscribe(t *testing.T) { Convey("then it returns metric not found error", func() { err := mc.Unsubscribe([]string{"test4"}, -1) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/test4") }) }) Convey("when the metric's count is already 0", t, func() { diff --git a/control/mttrie.go b/control/mttrie.go index 05e37135e..73ec3c363 100644 --- a/control/mttrie.go +++ b/control/mttrie.go @@ -22,9 +22,6 @@ package control import ( "errors" "fmt" - - "github.com/intelsdi-x/snap/core" - "github.com/intelsdi-x/snap/core/serror" ) /* @@ -146,7 +143,7 @@ func (mtt *mttNode) Add(mt *metricType) { // Collect collects all children below a given namespace // and concatenates their metric types into a single slice -func (mtt *mttNode) Fetch(ns []string) ([]*metricType, serror.SnapError) { +func (mtt *mttNode) Fetch(ns []string) ([]*metricType, error) { node, err := mtt.find(ns) if err != nil { return nil, err @@ -171,7 +168,7 @@ func (mtt *mttNode) Fetch(ns []string) ([]*metricType, serror.SnapError) { } // Remove removes all children below a given namespace -func (mtt *mttNode) Remove(ns []string) serror.SnapError { +func (mtt *mttNode) Remove(ns []string) error { _, err := mtt.find(ns) if err != nil { return err @@ -189,17 +186,13 @@ func (mtt *mttNode) Remove(ns []string) serror.SnapError { // Get works like fetch, but only returns the MT at the given node // and does not gather the node's children. -func (mtt *mttNode) Get(ns []string) ([]*metricType, serror.SnapError) { +func (mtt *mttNode) Get(ns []string) ([]*metricType, error) { node, err := mtt.find(ns) if err != nil { return nil, err } if node.mts == nil { - se := serror.New(errorMetricNotFound(ns)) - se.SetFields(map[string]interface{}{ - "name": core.JoinNamespace(ns), - }) - return nil, se + return nil, errorMetricNotFound(ns) } var mts []*metricType for _, mt := range node.mts { @@ -226,14 +219,10 @@ func (mtt *mttNode) walk(ns []string) (*mttNode, int) { return parent, len(ns) } -func (mtt *mttNode) find(ns []string) (*mttNode, serror.SnapError) { +func (mtt *mttNode) find(ns []string) (*mttNode, error) { node, index := mtt.walk(ns) if index != len(ns) { - se := serror.New(errorMetricNotFound(ns)) - se.SetFields(map[string]interface{}{ - "name": core.JoinNamespace(ns), - }) - return nil, se + return nil, errorMetricNotFound(ns) } return node, nil } diff --git a/control/mttrie_test.go b/control/mttrie_test.go index 5de5492f2..148a376ec 100644 --- a/control/mttrie_test.go +++ b/control/mttrie_test.go @@ -104,7 +104,6 @@ func TestTrie(t *testing.T) { _, err := trie.Fetch([]string{"not", "present"}) So(err, ShouldNotBeNil) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/not/present") }) Convey("Fetch with error: depth exceeded", func() { lp := new(loadedPlugin) @@ -114,7 +113,6 @@ func TestTrie(t *testing.T) { _, err := trie.Fetch([]string{"intel", "foo", "bar", "baz"}) So(err, ShouldNotBeNil) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/intel/foo/bar/baz") }) }) @@ -138,7 +136,6 @@ func TestTrie(t *testing.T) { n, err := trie.Get([]string{"intel"}) So(n, ShouldBeNil) So(err.Error(), ShouldContainSubstring, "Metric not found:") - So(err.Fields()["name"], ShouldEqual, "/intel") }) }) } diff --git a/control/plugin_manager_test.go b/control/plugin_manager_test.go index fac02eada..68fb509eb 100644 --- a/control/plugin_manager_test.go +++ b/control/plugin_manager_test.go @@ -121,11 +121,11 @@ func TestLoadPlugin(t *testing.T) { cfg.Plugins.Collector.Plugins["mock2"] = newPluginConfigItem(optAddPluginConfigItem("test", ctypes.ConfigValueBool{Value: true})) p := newPluginManager(OptSetPluginConfig(cfg.Plugins)) p.SetMetricCatalog(newMetricCatalog()) - lp, err := loadPlugin(p, PluginPath) + lp, serr := loadPlugin(p, PluginPath) So(lp, ShouldHaveSameTypeAs, new(loadedPlugin)) So(p.all(), ShouldNotBeEmpty) - So(err, ShouldBeNil) + So(serr, ShouldBeNil) So(len(p.all()), ShouldBeGreaterThan, 0) mts, err := p.metricCatalog.Fetch([]string{}) So(err, ShouldBeNil) diff --git a/mgmt/rest/client/metric.go b/mgmt/rest/client/metric.go index 9b642c6ee..4a1106993 100644 --- a/mgmt/rest/client/metric.go +++ b/mgmt/rest/client/metric.go @@ -77,12 +77,61 @@ func (c *Client) FetchMetrics(ns string, ver int) *GetMetricsResult { return r } +// GetMetricVersions retrieves all versions of a metric at a given namespace. +func (c *Client) GetMetricVersions(ns string) *GetMetricsResult { + r := &GetMetricsResult{} + q := fmt.Sprintf("/metrics%s", ns) + resp, err := c.do("GET", q, ContentTypeJSON) + if err != nil { + return &GetMetricsResult{Err: err} + } + + switch resp.Meta.Type { + case rbody.MetricsReturnedType: + mc := resp.Body.(*rbody.MetricsReturned) + r.Catalog = convertCatalog(mc) + case rbody.ErrorType: + r.Err = resp.Body.(*rbody.Error) + default: + r.Err = ErrAPIResponseMetaType + } + return r +} + +// GetMetric retrieves a metric at a given namespace and version. +// If the version is < 1, the latest version is returned. +func (c *Client) GetMetric(ns string, ver int) *GetMetricResult { + r := &GetMetricResult{} + q := fmt.Sprintf("/metrics%s?ver=%d", ns, ver) + resp, err := c.do("GET", q, ContentTypeJSON) + if err != nil { + return &GetMetricResult{Err: err} + } + + switch resp.Meta.Type { + case rbody.MetricReturnedType: + mc := resp.Body.(*rbody.MetricReturned) + r.Metric = mc.Metric + case rbody.ErrorType: + r.Err = resp.Body.(*rbody.Error) + default: + r.Err = ErrAPIResponseMetaType + } + return r +} + // GetMetricsResult is the response from snap/client on a GetMetricCatalog call. type GetMetricsResult struct { Catalog []*rbody.Metric Err error } +// GetMetricResult is the response from snap/client on a GetMetricCatalog call. +type GetMetricResult struct { + Metric *rbody.Metric + Err error +} + // Len returns the slice's length of GetMetricsResult.Catalog. func (g *GetMetricsResult) Len() int { return len(g.Catalog) diff --git a/mgmt/rest/metric.go b/mgmt/rest/metric.go index 62486d94c..2cea94287 100644 --- a/mgmt/rest/metric.go +++ b/mgmt/rest/metric.go @@ -60,26 +60,44 @@ func (s *Server) getMetricsFromTree(w http.ResponseWriter, r *http.Request, para ) q := r.URL.Query() v := q.Get("ver") - if v == "" { - ver = -1 - } else { - ver, err = strconv.Atoi(v) + + if ns[len(ns)-1] == "*" { + if v == "" { + ver = -1 + } else { + ver, err = strconv.Atoi(v) + if err != nil { + respond(400, rbody.FromError(err), w) + return + } + } + + mets, err := s.mm.FetchMetrics(ns[:len(ns)-1], ver) if err != nil { - respond(400, rbody.FromError(err), w) + respond(404, rbody.FromError(err), w) return } + respondWithMetrics(r.Host, mets, w) + return } - if ns[len(ns)-1] == "*" { - mets, err := s.mm.FetchMetrics(ns[:len(ns)-1], ver) + // If no version was given, get all that fall at this namespace. + if v == "" { + mts, err := s.mm.GetMetricVersions(ns) if err != nil { respond(404, rbody.FromError(err), w) return } - respondWithMetrics(r.Host, mets, w) + respondWithMetrics(r.Host, mts, w) return } + // if an explicit version is given, get that single one. + ver, err = strconv.Atoi(v) + if err != nil { + respond(400, rbody.FromError(err), w) + return + } mt, err := s.mm.GetMetric(ns, ver) if err != nil { respond(404, rbody.FromError(err), w) @@ -139,5 +157,5 @@ func respondWithMetrics(host string, mets []core.CatalogedMetric, w http.Respons } func catalogedMetricURI(host string, mt core.CatalogedMetric) string { - return fmt.Sprintf("%s://%s/v1/metrics%s?version=%d", protocolPrefix, host, core.JoinNamespace(mt.Namespace()), mt.Version()) + return fmt.Sprintf("%s://%s/v1/metrics%s?ver=%d", protocolPrefix, host, core.JoinNamespace(mt.Namespace()), mt.Version()) } diff --git a/mgmt/rest/server.go b/mgmt/rest/server.go index b5ffb5e8b..5026ff4b3 100644 --- a/mgmt/rest/server.go +++ b/mgmt/rest/server.go @@ -57,6 +57,7 @@ var ( type managesMetrics interface { MetricCatalog() ([]core.CatalogedMetric, error) FetchMetrics([]string, int) ([]core.CatalogedMetric, error) + GetMetricVersions([]string) ([]core.CatalogedMetric, error) GetMetric([]string, int) (core.CatalogedMetric, error) Load(*core.RequestedPlugin) (core.CatalogedPlugin, serror.SnapError) Unload(core.Plugin) (core.CatalogedPlugin, serror.SnapError)