Adding beat module with stats metricset #12181

ycombinator · 2019-05-13T22:44:17Z

This PR adds a new beat module to Metricbeat. It also implements the first (of two) metricsets for this module, stats.

More specifically, this PR:

Introduces the beat module.
Implements the stats metricset for this module.
Implements the non-xpack code path for this metricset.
Implements the xpack code path for this metricset.
Introduces the modules.d/beat.yml and modules.d/beat-xpack.yml module configuration files.
Implements integration tests for the module.
Implements system tests for the module.

Testing this PR

X-Pack code path (for Stack Monitoring UI)

Build metricbeat with this PR.

cd $GOPATH/src/github.com/elastic/beats/metricbeat
mage build

Using Kibana > Console, clear out existing Beats monitoring data.
```
DELETE .monitoring-beats-*
```

Start Metricbeat with internal collection.

./metricbeat -e -E monitoring.enabled=true

Wait 10 seconds or so for monitoring data to be collected.

Using Kibana > Console, retrieve the latest document from the .monitoring-beats-* index and save it.

POST .monitoring-beats-*/_search?filter_path=hits.hits._source
{
  "query": {
    "bool": {
      "filter": {
        "term": {
          "type": "beats_stats"
        }
      }
    }
  },
  "sort": [
    {
      "timestamp": {
        "order": "desc"
      }
    }
  ]
}

Stop Metricbeat.
Using Kibana > Console, clear out existing Beats monitoring data.
```
DELETE .monitoring-beats-*
```
Start Metricbeat with HTTP API enabled for external collection.
```
./metricbeat -e -E http.enabled=true
```
In a new window enable the beats-xpack Metricbeat module for external collection and start this Metricbeat instance.
```
./metricbeat modules enable beat-xpack
./metricbeat -e
```
Wait 10 seconds or so for monitoring data to be collected.

Using Kibana > Console, retrieve the latest document from the .monitoring-beats-* index and save it.

POST .monitoring-beats-*/_search?filter_path=hits.hits._source
{
  "query": {
    "bool": {
      "filter": {
        "term": {
          "type": "beats_stats"
        }
      }
    }
  },
  "sort": [
    {
      "timestamp": {
        "order": "desc"
      }
    }
  ]
}

Compare the JSON document saved from internal collection (step 5) with the JSON document saved from external collection (step 11). You might want to use a tool like jsondiff.com for this purpose.

The only structural difference between the two documents should be that the externally-collected JSON document should have 7 extra top-level fields: @timestamp, event, ecs, agent, metricset, service, host. These are expected extra fields that are always inserted by Metricbeat. Other than these expected structural differences, the rest of the structure should be identical; however, values of corresponding fields may be different.

ruflin · 2019-05-15T10:41:22Z

With this the stats endpoint basically becomes a "documented" endpoint and the API and it's content should not change anymore. Perhaps a good time to revised the output we have to check for example if it follows ECS etc. At the same time we use this event already for the monitoring reporting, so we should not break too many things here?

metricbeat/module/beats/beats.go

metricbeat/module/beats/stats/data.go

sayden

Sorry for the early review. I'm stuck at the airport 😄

ycombinator · 2019-06-18T03:35:49Z

jenkins, test this

elasticmachine · 2019-06-18T03:36:22Z

Pinging @elastic/stack-monitoring

ycombinator · 2019-06-18T03:38:42Z

Reviewers: there is a LOT of new code / files in this PR. However, they are all in service of a single logical end-goal: introducing the beats module with the stats metricset within it. Still, if you'd prefer that I break up this PR into smaller PRs, I'm happy to do that — perhaps the implementation can be one PR and all the test code/files a follow up PR?

ycombinator · 2019-06-18T03:40:28Z

With this the stats endpoint basically becomes a "documented" endpoint and the API and it's content should not change anymore. Perhaps a good time to revised the output we have to check for example if it follows ECS etc. At the same time we use this event already for the monitoring reporting, so we should not break too many things here?

I would not change this API now, in a minor version. Perhaps we can do that in 8.0.0.

However, what I would do now is make sure the non-xpack code path of the beats/stats metricset indexes documents into metricbeat-* that follow ECS conventions. This can be checked by looking at the generated data.json file.

cachedout · 2019-06-18T11:38:44Z

I tested this according to the steps outlined by @ycombinator and it worked as described!

metricbeat/metricbeat.reference.yml

ruflin · 2019-06-19T07:02:31Z

metricbeat/module/beats/_meta/fields.yml

+  settings: ["ssl", "http"]
+  short_config: false
+  fields:
+    - name: beats


Same as above, beat.* instead?

ruflin · 2019-06-19T07:02:56Z

metricbeat/module/beats/beats_integration_test.go

+	}
+}
+
+func TestData(t *testing.T) {


Why not use the new testing mechanism here with the golden files?

Sorry, I think I already forgot about this 😞. Could you point me to an example module that's using this?

this should help: https://github.com/elastic/beats/tree/master/metricbeat/mb/testing/data

Okay, I started to use this new testdata framework in 627ecf2bb. However, as you will see the generated stats.800.json-expected.json file is bad. It has a error.message field with value as "HTTP error 404 in : 404 Not Found".

Can you help me figure out what I'm doing wrong? Better yet, can you help me figure out how to debug what's going on with this new framework 🎣?

Okay, I did some digging and I think I know why this is happening. The beats/stats metricset makes 2 HTTP API calls: GET / and GET /stats.

I'm investigating now how to mock both these calls with the testdata framework.

AFAICT, there's no way currently for the testdata framework to handle the scenario where a metricset needs to call multiple HTTP APIs. Can you confirm this?

If this is true, I'd rather not hold up this PR (and the one coming after it for the beat/state metricset: #12615) for this enhancement to be implemented in the new test framework. Once the enhancement is there I can make a follow up PR to use it with these metricsets. WDYT?

I can confirm unfortunately, see #11425 SGTM to move forward.

metricbeat/docker-compose.yml

metricbeat/metricbeat.reference.yml

metricbeat/module/beat/_meta/config-xpack.yml

metricbeat/module/beat/_meta/config.reference.yml

metricbeat/module/beat/_meta/config.yml

metricbeat/docker-compose.yml

metricbeat/modules.d/beat-xpack.yml.disabled

x-pack/metricbeat/metricbeat.reference.yml

Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>

ycombinator · 2019-06-21T04:30:10Z

I've merged #12621 and rebased this PR on master now. @ruflin this PR is ready for another round of code review. In particular, you might want to see #12181 (comment). Thanks!

ruflin · 2019-06-21T13:34:49Z

Skimmed through it and looks good to me. What I didn't check in detail is naming fields. It could be an opportunity to rethink how we report beats metrics (structure), at the same time this should not hold back this.

ruflin

Changelog?

ycombinator · 2019-06-21T14:17:27Z

Re: API change (naming of fields), I mentioned it earlier in the PR (#12181 (comment)):

I would not change this API now, in a minor version. Perhaps we can do that in 8.0.0.

However, what I would do now is make sure the non-xpack code path of the beat/stats metricset indexes documents into metricbeat-* that follow ECS conventions. This can be checked by looking at the generated data.json file.

ycombinator · 2019-06-21T14:19:22Z

@ruflin I'm thinking I should wait to include the CHANGELOG entry until the follow up PR to this one is also merged: #12615. Then we will have a complete beat module. WDYT?

ruflin mentioned this pull request May 13, 2019

Monitor the Elastic Stack with Metricbeat #7035

Closed

39 tasks

sayden reviewed May 19, 2019

View reviewed changes

metricbeat/module/beats/beats.go Outdated Show resolved Hide resolved

sayden reviewed May 19, 2019

View reviewed changes

metricbeat/module/beats/stats/data.go Outdated Show resolved Hide resolved

sayden reviewed May 19, 2019

View reviewed changes

ycombinator force-pushed the mb-beats-stats branch from 1a8cd7d to 5addb99 Compare June 1, 2019 12:05

ycombinator force-pushed the mb-beats-stats branch from 5addb99 to add1452 Compare June 17, 2019 22:21

ycombinator requested review from ruflin and cachedout June 18, 2019 03:35

ycombinator added Metricbeat Metricbeat module needs_backport PR is waiting to be backported to other branches. review Feature:Stack Monitoring v7.3.0 v8.0.0 labels Jun 18, 2019

ycombinator marked this pull request as ready for review June 18, 2019 03:41

ycombinator requested review from a team as code owners June 18, 2019 03:41

ruflin reviewed Jun 19, 2019

View reviewed changes

ycombinator changed the title ~~Adding beats module with stats metricset~~ Adding beat module with stats metricset Jun 19, 2019

ruflin reviewed Jun 19, 2019

View reviewed changes

ycombinator commented Jun 19, 2019

View reviewed changes

metricbeat/docker-compose.yml Outdated Show resolved Hide resolved

ycombinator commented Jun 19, 2019

View reviewed changes

metricbeat/modules.d/beat-xpack.yml.disabled Outdated Show resolved Hide resolved

ycombinator commented Jun 19, 2019

View reviewed changes

x-pack/metricbeat/metricbeat.reference.yml Outdated Show resolved Hide resolved

ycombinator and others added 16 commits June 20, 2019 17:54

Update metricbeat/module/beat/_meta/config.yml

3189e4c

Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>

Update metricbeat/docker-compose.yml

a9d2fec

Update metricbeat/modules.d/beat-xpack.yml.disabled

56ebe7c

Update x-pack/metricbeat/metricbeat.reference.yml

aa56839

Update metricbeat/modules.d/beat.yml.disabled

3198cd3

Renaming system test

1626576

Regenerating files

d09091b

Trying to use the new testdata framework

82fcefb

Renaming service to avoid naming collision

06faf32

Adding back data_test.go

71a8cb0

Fixing HTTP server hostname

ea37564

Trying localhost

9f8f2d0

Fixing inconsistencies

48a696d

Fixing typo

9041773

Make runtime optional as it is absent in older versions of Beats

d7c128b

Updates from make update

6e4b903

ycombinator force-pushed the mb-beats-stats branch from 9b31e7c to 6e4b903 Compare June 21, 2019 00:54

ycombinator removed the request for review from a team June 21, 2019 01:11

ruflin reviewed Jun 21, 2019

View reviewed changes

ruflin approved these changes Jun 21, 2019

View reviewed changes

ycombinator merged commit d29c533 into elastic:master Jun 21, 2019

This was referenced Jun 23, 2019

Removing files since new testdata framework cannot be used yet #12644

Merged

Docs for monitoring Beats with Metricbeat #12647

Closed

andrewkroh mentioned this pull request Jun 26, 2019

Update regexp so make release-manager-snapshot passes again. #12696

Merged

ycombinator deleted the mb-beats-stats branch December 25, 2019 11:18

ycombinator removed the needs_backport PR is waiting to be backported to other branches. label Jan 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding beat module with stats metricset #12181

Adding beat module with stats metricset #12181

ycombinator commented May 13, 2019 •

edited

Loading

ruflin commented May 15, 2019

sayden left a comment

ycombinator commented Jun 18, 2019

elasticmachine commented Jun 18, 2019

ycombinator commented Jun 18, 2019

ycombinator commented Jun 18, 2019

cachedout commented Jun 18, 2019

ruflin Jun 19, 2019

ycombinator Jun 19, 2019

ruflin Jun 19, 2019

ycombinator Jun 19, 2019

ruflin Jun 19, 2019

ycombinator Jun 19, 2019 •

edited

Loading

ycombinator Jun 19, 2019

ycombinator Jun 19, 2019 •

edited

Loading

ruflin Jun 21, 2019

ycombinator commented Jun 21, 2019

ruflin commented Jun 21, 2019

ruflin left a comment

ycombinator commented Jun 21, 2019

ycombinator commented Jun 21, 2019

Adding beat module with stats metricset #12181

Adding beat module with stats metricset #12181

Conversation

ycombinator commented May 13, 2019 • edited Loading

Testing this PR

X-Pack code path (for Stack Monitoring UI)

ruflin commented May 15, 2019

sayden left a comment

Choose a reason for hiding this comment

ycombinator commented Jun 18, 2019

elasticmachine commented Jun 18, 2019

ycombinator commented Jun 18, 2019

ycombinator commented Jun 18, 2019

cachedout commented Jun 18, 2019

ruflin Jun 19, 2019

Choose a reason for hiding this comment

ycombinator Jun 19, 2019

Choose a reason for hiding this comment

ruflin Jun 19, 2019

Choose a reason for hiding this comment

ycombinator Jun 19, 2019

Choose a reason for hiding this comment

ruflin Jun 19, 2019

Choose a reason for hiding this comment

ycombinator Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

ycombinator Jun 19, 2019

Choose a reason for hiding this comment

ycombinator Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

ruflin Jun 21, 2019

Choose a reason for hiding this comment

ycombinator commented Jun 21, 2019

ruflin commented Jun 21, 2019

ruflin left a comment

Choose a reason for hiding this comment

ycombinator commented Jun 21, 2019

ycombinator commented Jun 21, 2019

ycombinator commented May 13, 2019 •

edited

Loading

ycombinator Jun 19, 2019 •

edited

Loading

ycombinator Jun 19, 2019 •

edited

Loading