Skip to content

Metricbeat test modules#4656

Merged
tsg merged 7 commits intoelastic:masterfrom
exekias:test-modules
Jul 21, 2017
Merged

Metricbeat test modules#4656
tsg merged 7 commits intoelastic:masterfrom
exekias:test-modules

Conversation

@exekias
Copy link
Copy Markdown
Contributor

@exekias exekias commented Jul 12, 2017

Add metricbeat test modules command, it goes over all configured metricsets and runs them, reporting the result back to the user:

X1 :: elastic/beats/metricbeat ‹test-modules*› » ./metricbeat test modules        
system...
  cpu...OK
    result: 
    {
     "cores": 4,
     "idle": {
      "pct": 0
     },
     "iowait": {
      "pct": 0
     },
     "irq": {
      "pct": 0
     },
     "nice": {
      "pct": 0
     },
     "softirq": {
      "pct": 0
     },
     "steal": {
      "pct": 0
     },
     "system": {
      "pct": 0
     },
     "user": {
      "pct": 0
     }
    }

nginx...
  stubstatus...
    error... ERROR error making http request: Get http://127.0.0.1/server-status: dial tcp 127.0.0.1:80: getsockopt: connection refused

Filtering by module and metricset:

X1 :: elastic/beats/metricbeat ‹test-modules*› » ./metricbeat test modules system process_summary
system...
  process_summary...OK
    result: 
    {
     "_namespace": "process.summary",
     "idle": 0,
     "running": 1,
     "sleeping": 236,
     "stopped": 0,
     "total": 238,
     "unknown": 1,
     "zombie": 0
    }

X1 :: elastic/beats/metricbeat ‹test-modules*› » ./metricbeat test modules nginx                 
nginx...
  stubstatus...
    error... ERROR error making http request: Get http://127.0.0.1/server-status: dial tcp 127.0.0.1:80: getsockopt: connection refused

TODO:

  • Make it work for dynamic modules
  • Tests

@exekias exekias changed the title [WIP] Metricbeat Test modules [WIP] Metricbeat test modules Jul 12, 2017
@exekias exekias added enhancement Metricbeat Metricbeat needs_docs in progress Pull request is currently in progress. labels Jul 12, 2017
@exekias
Copy link
Copy Markdown
Contributor Author

exekias commented Jul 13, 2017

jenkins retest this

@exekias exekias force-pushed the test-modules branch 3 times, most recently from 36c49e6 to 4838798 Compare July 17, 2017 14:42
@exekias exekias added review and removed in progress Pull request is currently in progress. labels Jul 17, 2017
exekias added 2 commits July 17, 2017 16:55
It goes over all configured modules and runs them, reporting the
resulting event to the user
@exekias exekias changed the title [WIP] Metricbeat test modules Metricbeat test modules Jul 17, 2017
@exekias
Copy link
Copy Markdown
Contributor Author

exekias commented Jul 17, 2017

jenkins retest this please

Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM, I just left a few questions.

}

// MetricSets return the list of metricsets of the module
func (mw *Wrapper) MetricSets() []*metricSetWrapper {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this return []mb.MetricSet instead? (Returning an unexported type from an exported method can be annoying to users.)

Copy link
Copy Markdown
Contributor Author

@exekias exekias Jul 19, 2017

Choose a reason for hiding this comment

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

That was my first though too, but then I would need to reimplement fetch, which is not a big deal but could cause issues when we add new metricset types, wdyt?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then that seems OK.


type reporter interface {
StartFetchTimer()
Done() <-chan struct{}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to embed mb.PushReporter in reporter rather than duplicating the method signatures?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

totally, thanks for the tip :)

by embedding `mb.PushReporter` in it
@exekias
Copy link
Copy Markdown
Contributor Author

exekias commented Jul 20, 2017

fixed conflicts with master, all tests passing now :)


config, err := b.BeatConfig()
if err != nil {
fmt.Fprintf(os.Stderr, "Error initializing beat: %s\n", err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The three "Error initializing beat" message above might get confusing at some point.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I merged the PR, this is minor and can be handled in a future PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, as I have been adding new commands I feel the need for a common cmdline util to handle initialization and output

@tsg tsg merged commit c1bd89b into elastic:master Jul 21, 2017
@dedemorton dedemorton mentioned this pull request Jul 25, 2017
42 tasks
@dedemorton
Copy link
Copy Markdown
Contributor

Removing needs_docs label because the command is documented here: https://www.elastic.co/guide/en/beats/metricbeat/master/command-line-options.html#test-command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants