Skip to content

Adding query APIs for metricsets and modules from metricbeat registry #4102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 25, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ https://github.com/elastic/beats/compare/v5.1.1...master[Check the HEAD diff]
- Add new MetricSet interfaces for developers (`Closer`, `ReportingFetcher`, and `PushMetricSet`). {pull}3908[3908]
- Add kubelet module {pull}3916[3916]
- Add dropwizard module {pull}4022[4022]
- Adding query APIs for metricsets and modules from metricbeat registry {pull}4102[4102]

*Packetbeat*
- Add `fields` and `fields_under_root` to packetbeat protocols configurations. {pull}3518[3518]
Expand Down
38 changes: 37 additions & 1 deletion metricbeat/mb/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"sort"
"strings"
"sync"

"github.com/elastic/beats/libbeat/logp"
)
Expand Down Expand Up @@ -46,6 +47,8 @@ type metricSetFactoryInfo struct {
// Register contains the factory functions for creating new Modules and new
// MetricSets.
type Register struct {
//Lock to control concurrent read/writes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment needs a space after the //.

sync.RWMutex
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think embedding the mutex is not a good choice here because the methods will be become part of Register's public API. Users should never need to lock/unlock the Register because that is handled internally.

This reminds me, can you add to the godoc for Register that "It is safe for concurrent usage."`.

// A map of module name to ModuleFactory.
modules map[string]ModuleFactory
// A map of module name to nested map of MetricSet name to metricSetFactoryInfo.
Expand Down Expand Up @@ -144,9 +147,42 @@ func (r *Register) metricSetFactory(module, name string) (MetricSetFactory, Host
return info.factory, info.hostParser, nil
}

//Modules returns the list of module names that are registered
func (r *Register) Modules() []string {
r.RLock()
defer r.RUnlock()

modules := []string{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to make([]string, 0, len(r.modules)) since it knows the capacity requirement a priori.

for module := range r.modules {
modules = append(modules, module)
}

return modules
}

//MetricSets returns the list of metricsets registered for a given module
func (r *Register) MetricSets(module string) []string {
r.RLock()
defer r.RUnlock()

metricsets := []string{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


sets, ok := r.metricSets[module]
if ok {
for name := range sets {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once it reaches this point, it can allocate the slice with the metricsets = make([]string, 0, len(sets).

metricsets = append(metricsets, name)
}
}

return metricsets
}

// String return a string representation of the registered ModuleFactory's and
// MetricSetFactory's.
func (r Register) String() string {
func (r *Register) String() string {
r.RLock()
defer r.RUnlock()

var modules []string
for module := range r.modules {
modules = append(modules, module)
Expand Down
24 changes: 24 additions & 0 deletions metricbeat/mb/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,27 @@ func TestMetricSetFactory(t *testing.T) {
assert.NotNil(t, hp) // Can't compare functions in Go so just check for non-nil.
})
}

func TestMetricSetQuery(t *testing.T) {
registry := NewRegister()
err := registry.AddMetricSet(moduleName, metricSetName, fakeMetricSetFactory)
if err != nil {
t.Fatal(err)
}

metricsets := registry.MetricSets(moduleName)
assert.Equal(t, len(metricsets), 1)
assert.Equal(t, metricsets[0], metricSetName)

metricsets = registry.MetricSets("foo")
assert.Equal(t, len(metricsets), 0)
}

func TestModuleQuery(t *testing.T) {
registry := NewRegister()
registry.modules[moduleName] = fakeModuleFactory

modules := registry.Modules()
assert.Equal(t, len(modules), 1)
assert.Equal(t, modules[0], moduleName)
}