Httpmetricbeat #4092

christiangalsterer · 2017-04-23T19:28:46Z

here a very first commit.
There are no tests yet and I haven't run manual tests.

Actually I get a strange error when trying to run metricbeat and also in some tests, that the process are immediately killed, e.g. with "Killed: 9".

I'm running on Mac with golang 1.7.4 and my other beats are running fine. Are there any special settings needed for metricbeat on a mac?

Shall I open an issue for tracking/reference for the new metricbeat?

Added first request/event

elasticmachine · 2017-04-23T19:32:02Z

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

elasticmachine · 2017-04-23T19:32:07Z

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

christiangalsterer · 2017-04-23T20:03:35Z

Forget about my comment on build issues. It is solved by updating to gaoling 1.8.1 (should have checked beforehand).

ruflin · 2017-04-23T20:04:26Z

I had a similar issue with 1.8 which is a golang/ os x issue. 1.8.1 should solve it.

vjsamuel · 2017-04-24T03:52:01Z

metricbeat/module/http/json/json.go

+	http := helper.NewHTTP(base)
+	http.SetMethod(config.Method)
+	http.SetBody([]byte(config.Body))
+	for key, value := range config.Headers {


headers are unpacked and added to the http request here:
https://github.com/elastic/beats/blob/master/metricbeat/helper/http.go#L63

True, saw that as well, but then forgot to change before committing.

ruflin

Thanks a lot for the contribution. I'm really looking forward to have the http module as part of metricbeat.

I left a few comments below. Could you also add an entry in the CHANGELOG.asciidoc?

ruflin · 2017-04-24T06:37:32Z

metricbeat/module/http/json/json.go

+
+	event := common.MapStr{}
+
+	event["request"] = common.MapStr{


As these are based on the configs set, not sure if we should really add the request to each document. What would be the use case here?

ruflin · 2017-04-24T06:39:19Z

metricbeat/module/http/json/json.go

+// Part of new is also setting up the configuration by processing additional
+// configuration entries if needed.
+func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
+


Could you add here a log message like logp.Beta("The http json metricset is in beta.").

Yes will do.

ruflin · 2017-04-24T06:39:56Z

metricbeat/module/http/json/json.go

+type MetricSet struct {
+	mb.BaseMetricSet
+	http    *helper.HTTP
+	headers map[string]string


In case we do not add these fields (see comment below) to the event, they are probably not needed in the object.

ruflin · 2017-04-24T06:41:48Z

metricbeat/module/http/json/json.go

+		"body":        responseBody,
+	}
+
+	return event, nil


The json metricset is a dynamic metricset. That means it can be used multiple times with different fields as output. To make sure these fields to not conflict, we need a namespace which can be configure. As an example see here the jmx metricset: https://github.com/elastic/beats/blob/master/metricbeat/module/jolokia/jmx/jmx.go#L93 So event are put into http.namespace.*.

True, thanks for the hint. Is namespace here the correct concept, or is it to use different document_types?

Namespace is the one to use here. We don't use document type, especially as it will be removed in 6.0

ok, will make the changes accordingly.

ruflin · 2017-04-24T06:42:21Z

metricbeat/module/http/json/json.go

+	event["response"] = common.MapStr{
+		"status_code": response.StatusCode,
+		"headers":     m.getHeaders(response.Header),
+		"body":        responseBody,


As this is JSON, don't we need to decode it here?

Yes definitely.

One more note on the body: I would probably put the decode json directly on the top level and not into a body namespace.

ruflin · 2017-04-24T06:45:48Z

metricbeat/module/http/json/json.go

+
+	event["response"] = common.MapStr{
+		"status_code": response.StatusCode,
+		"headers":     m.getHeaders(response.Header),


As status_code and response header will be the same for all http metricsets, I was thinking to add these on the module lever for example under a response namespace. How to do that for example see docker: https://github.com/elastic/beats/blob/master/metricbeat/module/docker/cpu/data.go#L18

That would mean these are always under http.response.*.

I'm not sure if we should enable the headers by default because in the case of json I assume in most use cases people are not too interested in the headers (happy to be convinced otherwise). So I would make it configurable with a config option. As we already have headers in the config file, we could make it response.headers.enabled.

There is definitely room for improvement to move some of the information to the module layer. Idea was to get something working and getting familiar with Metricbeat implementation.
W.r.t if we should add the header I don't see an issue here to always add them

Happy to merge the PR rater soonish and do improvements on top of it.

For the headers, I probably miss the use case.

Update of the Contributing guide was missed in elastic#4033.

This allows if a second request is needed with a different path in a metricset to reuse the http helper and hostparser and only overwrite the path temporarly.

In some cases, a metricset does not return an event but also not an error. This can be the case if a metricset for a distributed system should only send events for the master. All metricbeat instances are running on all nodes, but only one node should send the events and the other skip the event. This changes makes this behaviour possible.

Fix race in go-metrics adapter, if registry is updated concurrently from multiple go routines.

This was accidentially removed in elastic#4055

…issue#199 Warn Beats users not to do multiline handling in Logstash

* Rename package name, packages in add_could_metadata and add_locale should follow the name of their current folder

Add perfmon raw counter values calculations This PR adds functions to calculate performance counters from raw values. These functions should serve more as internal apis.

* Remove kibana from the short config and comment it out by default

* Move kubernetes processor to root processors folder * Restart watch call on error * Flag kubernetes proccesor as beta * Fix config validation for `in_cluster` cases * Add kubernetes metadata fields * Put kubernetes processor fields under a common namespace

…elastic#4102) * Adding query APIs for metricsets and modules from metricbeat registry * Added locking to make `mb.Register` thread-safe.

Expand double wildcards into standard glob patterns, up to a maximum depth of 8 levels after the wildcard. Resolves elastic#2084

…stic#4111) * Support Alibaba Cloud provider for add_cloud_metadata processor * Add note for Alibaba Cloud provider in add_cloud_metadata processor documentation.

See PR elastic#4091 for detailed list of event format changes and sample events. - export transport.DialWith - update heartbeat look: - add look.Status - add some missing godoc - rewrite heartbeat dialchain package - simplify package - have connection layers add standardized connection data to the final event - add some more helpers - add some godocs - update event format. See PR elastic#4091 - add more detailed duration measure to HTTP module - update job settings - introduce explicit structs for job settings - job settings can contain static monitor fields to be added to every event - helpers can add more static monitor fields to settings - update fields.yml structure - update kibana dashboard

…4115)

Add community beats topic to devguide

Added first request/event

added first config for testing

Moved fields to modules Update (some) documentation Open: - complete documentation

# Conflicts: # metricbeat/docs/modules/http.asciidoc # metricbeat/module/http/_meta/config.yml # metricbeat/module/http/_meta/fields.yml # metricbeat/module/http/json/_meta/fields.yml # metricbeat/module/http/json/json.go

christiangalsterer · 2017-04-30T06:46:52Z

'make collect' doesn't seem to update the generated doc files (anymore). I did already a fresh clone, but no luck. This is why the "make check" currently fails ans the docs don't match anymore.

christiangalsterer · 2017-04-30T07:14:39Z

intellij had some refresh issues causing some bad merges/rebases. I will fix that later today, sorry for any inconveniences caused by this.

ruflin · 2017-04-30T08:13:44Z

use make update instead of make collect

christiangalsterer · 2017-04-30T08:21:30Z

Thanks for the quick reply. If you want to to create a new PR as the current is cluttered with merge commits I'm happy to do so.

I will add more documentation later today. In any case thanks for any review comments in advance.

ruflin · 2017-04-30T08:38:41Z

will probably not be able to review it today. you could also squash the commits and the force push to the sam branch.

christiangalsterer · 2017-04-30T16:04:11Z

sure no problem, enjoy the long weekend. Will also squash the changes into a single commit

christiangalsterer · 2017-05-01T06:24:33Z

I added PR #4155 for the confusion on make collect vs make update.

ruflin · 2017-05-01T06:34:38Z

@christiangalsterer Any chance to push the squashed changes? Right now it is kind of tricky to review as the diff is huge because of all the merges. Probably you also have to rebase on master. As you merged previously I worry that you will have quite a few conflicts :-( In general I recommend to always rebase on master and not merge master.

christiangalsterer · 2017-05-01T06:48:23Z

@ruflin: Sure thing. Will squash the changes and I always use rebase normally, but somehow IntelliJ played some bad tricks ;-)

christiangalsterer · 2017-05-01T06:49:52Z

Actually maybe I close this PR and create a new one, to have a cleaner state, would you be ok with this?

ruflin · 2017-05-01T06:51:08Z

sure, feel free to open a new one.

christiangalsterer · 2017-05-01T07:10:13Z

Replaced by #4156.

dedemorton and others added 5 commits April 21, 2017 14:06

Warn Beats users not to do multiline handling in Logstash

e97377d

Initial commit http metricbeat module

4f061e2

Initial commit http metricbeat module

e750f90

Add first fields information

1977b33

Added first request/event

Add first fields information

2242883

Added first request/event

christiangalsterer added 2 commits April 23, 2017 22:04

Updated config and docu files.

c442602

Fix fmt findings

621f763

vjsamuel reviewed Apr 24, 2017

View reviewed changes

ruflin reviewed Apr 24, 2017

View reviewed changes

ruflin added in progress Pull request is currently in progress. Metricbeat Metricbeat module review labels Apr 24, 2017

ruflin and others added 13 commits April 24, 2017 09:54

Update CONTRIBUTING.md to Golang 1.8.1 (elastic#4094)

282d81d

Update of the Contributing guide was missed in elastic#4033.

Allow to overwrite uri in http helper (elastic#4084)

3efb9db

This allows if a second request is needed with a different path in a metricset to reuse the http helper and hostparser and only overwrite the path temporarly.

Fix race in go-metrics adapater (elastic#4098)

38e04b4

Fix race in go-metrics adapter, if registry is updated concurrently from multiple go routines.

Set 10000 field limit also for ES v5 (elastic#4079)

fafe088

This was accidentially removed in elastic#4055

Merge pull request elastic#4086 from dedemorton/logstash_input_beats_…

4aed040

…issue#199 Warn Beats users not to do multiline handling in Logstash

File restructure by providers for add_cloud_metadata (elastic#4088)

2bc0a20

* Rename package name, packages in add_could_metadata and add_locale should follow the name of their current folder

Add perfmon raw counter values calculations (elastic#3972)

de11679

Add perfmon raw counter values calculations This PR adds functions to calculate performance counters from raw values. These functions should serve more as internal apis.

Add x-pack monitoring note to kibana/es docs (elastic#4012)

af60958

* Remove kibana from the short config and comment it out by default

Adding query APIs for metricsets and modules from metricbeat registry (…

354fdd6

…elastic#4102) * Adding query APIs for metricsets and modules from metricbeat registry * Added locking to make `mb.Register` thread-safe.

Suggest GOPATH in ~/go (elastic#4110)

ca2e449

filebeat: expand double wildcards in prospector (elastic#3980)

aaee997

Expand double wildcards into standard glob patterns, up to a maximum depth of 8 levels after the wildcard. Resolves elastic#2084

athom and others added 14 commits April 27, 2017 09:27

Support Alibaba Cloud provider for add_cloud_metadata proccessor (ela…

1344e4f

…stic#4111) * Support Alibaba Cloud provider for add_cloud_metadata processor * Add note for Alibaba Cloud provider in add_cloud_metadata processor documentation.

Add community beats topic to devguide

b9f1b50

Add example dashboards using the time series visual builder (elastic#…

b845687

…4115)

Merge pull request elastic#4134 from dedemorton/add_commbeats

ea763e2

Add community beats topic to devguide

Initial commit http metricbeat module

946bfd8

Initial commit http metricbeat module

92ee85e

Add first fields information

81da4f6

Added first request/event

Add first fields information

2c84adf

Added first request/event

Updated config and docu files.

521c11c

Fix fmt findings

9d69fd6

Fixed review findings

47a046c

added first config for testing

Added integration tess

a3bbadb

Moved fields to modules Update (some) documentation Open: - complete documentation

Generated docucmentation

1fca1b0

Updated documentation and added entry to changelog

8eb085f

christiangalsterer closed this May 1, 2017

christiangalsterer mentioned this pull request May 1, 2017

New module http #4156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Httpmetricbeat #4092

Httpmetricbeat #4092

christiangalsterer commented Apr 23, 2017

elasticmachine commented Apr 23, 2017

elasticmachine commented Apr 23, 2017

christiangalsterer commented Apr 23, 2017

ruflin commented Apr 23, 2017

vjsamuel Apr 24, 2017

christiangalsterer Apr 24, 2017

ruflin left a comment

ruflin Apr 24, 2017

ruflin Apr 24, 2017

christiangalsterer Apr 24, 2017

ruflin Apr 24, 2017

ruflin Apr 24, 2017

christiangalsterer Apr 24, 2017

ruflin Apr 24, 2017

christiangalsterer Apr 24, 2017

ruflin Apr 24, 2017

christiangalsterer Apr 24, 2017

ruflin Apr 25, 2017

ruflin Apr 24, 2017

christiangalsterer Apr 24, 2017

ruflin Apr 24, 2017

christiangalsterer commented Apr 30, 2017

christiangalsterer commented Apr 30, 2017

ruflin commented Apr 30, 2017

christiangalsterer commented Apr 30, 2017

ruflin commented Apr 30, 2017

christiangalsterer commented Apr 30, 2017

christiangalsterer commented May 1, 2017

ruflin commented May 1, 2017

christiangalsterer commented May 1, 2017

christiangalsterer commented May 1, 2017

ruflin commented May 1, 2017

christiangalsterer commented May 1, 2017

Httpmetricbeat #4092

Httpmetricbeat #4092

Conversation

christiangalsterer commented Apr 23, 2017

elasticmachine commented Apr 23, 2017

elasticmachine commented Apr 23, 2017

christiangalsterer commented Apr 23, 2017

ruflin commented Apr 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christiangalsterer commented Apr 30, 2017

christiangalsterer commented Apr 30, 2017

ruflin commented Apr 30, 2017

christiangalsterer commented Apr 30, 2017

ruflin commented Apr 30, 2017

christiangalsterer commented Apr 30, 2017

christiangalsterer commented May 1, 2017

ruflin commented May 1, 2017

christiangalsterer commented May 1, 2017

christiangalsterer commented May 1, 2017

ruflin commented May 1, 2017

christiangalsterer commented May 1, 2017