Skip to content
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

Index metricset for elasticsearch Metricbeat module #6881

Merged
merged 4 commits into from Apr 20, 2018

Conversation

Projects
None yet
5 participants
@ruflin
Copy link
Collaborator

commented Apr 17, 2018

This adds the index metricset to the Elasticsearch module. For now it only contains very basic metrics and is marked experimental.

This PR is used to create the basic for adding tests with the fetch reporter v2 interface and also generate data with it.

It also adds basic utility functions to fetch Master or Node information in the Elasticsearch module.

return clusterStruct.MasterNode, nil
}

func GetInfo(http *helper.HTTP, uri string) (*Info, error) {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

exported function GetInfo should have comment or be unexported

func GetClusterID(http *helper.HTTP, uri string, nodeID string) (string, error) {

// Check if cluster id already cached. If yes, return it.
if clusterId, ok := clusterIDCache[nodeID]; ok {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

var clusterId should be clusterID

// Global clusterIdCache. Assumption is that the same node id never can belong to a different cluster id
var clusterIDCache = map[string]string{}

type Info struct {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

exported type Info should have comment or be unexported

return r.events
}

func (r *CapturingReporterV2) GetErrors() []error {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

exported method CapturingReporterV2.GetErrors should have comment or be unexported

r.errs = append(r.errs, err)
return true
}

func (r *CapturingReporterV2) GetEvents() []mb.Event {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

exported method CapturingReporterV2.GetEvents should have comment or be unexported

events []mb.Event
errs []error
}

func (r *capturingReporterV2) Event(event mb.Event) bool {
func (r *CapturingReporterV2) Event(event mb.Event) bool {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

exported method CapturingReporterV2.Event should have comment or be unexported

@@ -151,25 +151,33 @@ func NewReportingMetricSetV2(t testing.TB, config interface{}) mb.ReportingMetri
return reportingMetricSetV2
}

type capturingReporterV2 struct {
type CapturingReporterV2 struct {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

exported type CapturingReporterV2 should have comment or be unexported

@@ -57,6 +57,28 @@ func WriteEvents(f mb.EventsFetcher, t testing.TB) error {
return nil
}

// WriteEvents fetches events and writes the first event to a ./_meta/data.json

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 17, 2018

comment on exported function WriteEventsReporterV2 should be of the form "WriteEventsReporterV2 ..."

@exekias
Copy link
Member

left a comment

This is a great addition to the module, I left some minor comments/questions

metricsets:
#- "index"
- "node"
- "node_stats"

This comment has been minimized.

Copy link
@exekias

exekias Apr 17, 2018

Member

you can drop " chars here

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 18, 2018

Author Collaborator

done

}

func eventMapping(node map[string]interface{}) common.MapStr {
event, _ := schema.Apply(node)

This comment has been minimized.

Copy link
@exekias

exekias Apr 17, 2018

Member

Perhaps we want to stop/warn on error here?

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 18, 2018

Author Collaborator

The error handling is partially broken here. We skip it in all the other modules too. We should revisit how we do error handling during parsing events.

}

func getNodeName(http *helper.HTTP, uri string) (string, error) {

This comment has been minimized.

Copy link
@exekias

exekias Apr 17, 2018

Member

There are a few methods with a starting empty line, it would be nice to remove them

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 18, 2018

Author Collaborator

removed all of them.

}

// Not master, no event sent
if !isMaster {

This comment has been minimized.

Copy link
@exekias

exekias Apr 17, 2018

Member

I wonder what will be the behavior when monitoring from outside the cluster? for instance a cluster in Elastic Cloud

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 18, 2018

Author Collaborator

This will not work properly at the moment. We will have to introduce additional config options for this. See also discussion here: #6807 (comment)

I'm thinking that these will potentially be module config options which apply to all metricsets but I'm not 100% sure yet on what the exact behaviour / options will be so I ignored it for now.

return port
}

func GetConfig24(metricset string) map[string]interface{} {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 18, 2018

exported function GetConfig24 should have comment or be unexported

return host
}

func GetEnvPort24() string {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 18, 2018

exported function GetEnvPort24 should have comment or be unexported

@@ -27,3 +27,29 @@ func GetConfig(metricset string) map[string]interface{} {
"hosts": []string{GetEnvHost() + ":" + GetEnvPort()},
}
}

func GetEnvHost24() string {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Apr 18, 2018

exported function GetEnvHost24 should have comment or be unexported

@ruflin ruflin force-pushed the ruflin:es-index-metricset branch from 0303367 to d826677 Apr 19, 2018

@ruflin ruflin removed the in progress label Apr 19, 2018

@exekias
Copy link
Member

left a comment

WFG

@exekias

This comment has been minimized.

Copy link
Member

commented Apr 19, 2018

Tests are failing, I think it's related to ELASTICSEARCH_HOST env vars

@ruflin ruflin force-pushed the ruflin:es-index-metricset branch from 5cc0087 to 70d26a4 Apr 19, 2018

Index metricset for elasticsearch Metricbeat module
This adds the index metricset to the Elasticsearch module. For now it only contains very basic metrics and is marked experimental.

This PR is used to create the basic for adding tests with the fetch reporter v2 interface and also generate data with it.

It also adds basic utility functions to fetch Master or Node information in the Elasticsearch module.

@ruflin ruflin force-pushed the ruflin:es-index-metricset branch from 70d26a4 to 1581858 Apr 19, 2018

@@ -1,5 +1,3 @@
=== Elasticsearch node_stats metricset

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 19, 2018

Author Collaborator

This is cleanup and should not have been here. First though was this cause the doc build issue.

==== Fields

For a description of each field in the metricset, see the
<<exported-fields-elasticsearch,exported fields>> section.

This comment has been minimized.

Copy link
@ph

ph Apr 19, 2018

Member

This ID doesn't to exist exported-fields-elasticsearch?

This comment has been minimized.

This comment has been minimized.

Copy link
@dedemorton

dedemorton Apr 19, 2018

Contributor

@exekias I really think the error here is related to how our tests are written, not the doc source. I tested the files offline and they build without errors. I think it's safe to ignore the error messages here.

@exekias

This comment has been minimized.

Copy link
Member

commented Apr 19, 2018

jenkins, retest this please

@exekias
Copy link
Member

left a comment

Ready for merge

ruflin added some commits Apr 20, 2018

@exekias exekias merged commit 7db33f2 into elastic:master Apr 20, 2018

3 of 4 checks passed

beats-ci Build started sha1 is merged.
Details
CLA Commit author has signed the CLA
Details
Hound No violations found. Woof!
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.