New module http #4156

christiangalsterer · 2017-05-01T07:08:30Z

Initial commit after creating a new PR

christiangalsterer · 2017-05-01T07:09:29Z

@ruflin , here the new PR, as replacement for PR #4092

elasticmachine · 2017-05-01T07:12: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-05-01T07:12:08Z

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.

ruflin

@christiangalsterer I left a few comments on the PR. I'm also ok to merge it as is and take over the review comments in a second part if you prefer it.

ruflin · 2017-05-01T07:40:44Z

metricbeat/docs/modules/http.asciidoc

+Http module is a [Metricbeat](https://www.elastic.co/products/beats/metricbeat) used to call HTTP endpoints.
+Multiple endpoints can be configured which are polled in a regular interval and the result is shipped to the configured output channel.
+
+Httpbeat is inspired by the Logstash [http_poller](https://www.elastic.co/guide/en/logstash/current/plugins-inputs-http_poller.html) input filter but doesn't require that the endpoint is reachable by Logstash as the Metricbeat module pushes the data to the configured outpust channels, e.g. Logstash or Elasticsearch.


s/outpust/outputs/

ok, will fix

ruflin · 2017-05-01T07:41:38Z

metricbeat/docs/modules/http.asciidoc

+Httpbeat is inspired by the Logstash [http_poller](https://www.elastic.co/guide/en/logstash/current/plugins-inputs-http_poller.html) input filter but doesn't require that the endpoint is reachable by Logstash as the Metricbeat module pushes the data to the configured outpust channels, e.g. Logstash or Elasticsearch.
+This is often necessary in security restricted network setups, where Logstash is not able to reach all servers. Instead the server to be monitored itself has Metricbeat installed and can send the data or a collector server has Metricbeat installed which is deployed in the secured network environment and can reach all servers to be monitored.
+
+Example use cases are:


I suggest to remove these examples as we cover these by its own modules (except spring-boot), but that should come as a module soon.

ok, will fix

ruflin · 2017-05-01T07:42:12Z

metricbeat/metricbeat.full.yml

+#-------------------------------- http Module --------------------------------
+- module: http
+  metricsets: ["json"]
+  enabled: true


This should be set to enabled: false by default.

ok, will fix

ruflin · 2017-05-01T07:43:07Z

metricbeat/module/http/_meta/Dockerfile

@@ -0,0 +1,11 @@
+# Tomcat is started to fetch Jolokia metrics from it


I'm thinking if we could use a less "heavy" container for the http testing?

Any suggestions? I reused the container Jolokia container as it was already there, so that no additional image needs to be build and an additional to be started.

I was thinking of having our own very small web service in go that exposes and http endpoint and sends json to make it clear it is a "dynamic" metricset. Here we have something similar for dropwizard: https://github.com/elastic/beats/tree/master/metricbeat/module/dropwizard/_meta/test Except having a http service will hopefully only be a few lines of code and then in the container go run ... can be directly used.

ruflin · 2017-05-01T07:55:11Z

metricbeat/module/http/json/json.go

+		return nil, err
+	}
+
+	event := common.MapStr{


I would change this code as following. Note: code will not work directly probably because of some type conversion that is needed.

event := jsonBody event["_namespace"] = m.namespace if m.config.request.enabled { event[mb.ModuleData]["request"] = common.MapStr{ "headers": m.getHeaders(response.Request.Header), "method": response.Request.Method, "body": m.body, } } if m.config.response.enabled { event[mb.ModuleData]["response"] = common.MapStr{ "headers": m.getHeaders(response.Request.Header), "method": response.Request.Method, "body": m.body, } }

The thinking behind this is as following:

Httpbeat json is used either for self build http endpoints or for endpoints like spring which are not supported yet by Metricbeat. In these cases the json body contains all the relevant data. So in most cases the response header details is not too relevant if it is a 200.

The json document should not be nested under body as we already have the namespace to put it under its separate namespace.

There are definitively use cases where request and response can become interesting, so we should have a config option to enable this. Be aware that these use cases will also overlap potentially with heartbeat and heartbeat could be the better tool for this use case.

ok, will fix. W.r.t to heartbeat I definitely see also the overlap.

one more question: Do you think we should add the custom config struct to the MetricSet (your example) or dedicated fields for each item (current approach)?

For enable / disable it should be config options. Not sure what you mean by dedicated fields?

- Initial commit after creating a new PR

christiangalsterer · 2017-05-01T13:52:48Z

Addressed most PR comments, excepts for the Docker container.

ruflin

Thanks a lot. LGTM. Will go through it in detail again later.

ruflin · 2017-05-01T14:41:23Z

metricbeat/module/http/json/json.go

+		Body            string `config:"body"`
+		RequestEnabled  bool   `config:"request.enabled"`
+		ResponseEnabled bool   `config:"response.enabled"`
+	}{}


We probably should define the "defaults" here in a follow up PR.

Yes I agree.

ruflin · 2017-05-01T15:15:34Z

jenkins, test it

ruflin · 2017-05-02T08:35:56Z

@christiangalsterer Merged. Thanks a lot for the contribution.

ruflin · 2017-05-02T09:11:22Z

@christiangalsterer I created here a minor follow up PR: #4170

tsg · 2017-05-02T11:15:46Z

metricbeat/docs/modules/http.asciidoc

+[[metricbeat-module-http]]
+== http Module
+
+Http module is a [Metricbeat](https://www.elastic.co/products/beats/metricbeat) used to call arbitrary HTTP endpoints for which not a dedicated metricbeat is available.


Missing "module" after "a Metricbeat"?

also there's an extra "not".

Opps sorry, I intended to comment on #4170. Ignore these.

This is a follow up PR for elastic#4156 * Add environment with small golang web service * Remove not needed body fields * Add defaults to configs * Add system tests * Update config with all config options

This is a follow up PR for #4156 * Add environment with small golang web service * Remove not needed body fields * Add defaults to configs * Add system tests * Update config with all config options

christiangalsterer mentioned this pull request May 1, 2017

Httpmetricbeat #4092

Closed

ruflin reviewed May 1, 2017

View reviewed changes

ruflin added Metricbeat Metricbeat review labels May 1, 2017

New module http

2508f4c

- Initial commit after creating a new PR

christiangalsterer force-pushed the httpmodule branch from 78af38c to 2508f4c Compare May 1, 2017 13:51

ruflin reviewed May 1, 2017

View reviewed changes

ruflin merged commit b764e32 into elastic:master May 2, 2017

ruflin mentioned this pull request May 2, 2017

Http module improvements #4170

Merged

tsg reviewed May 2, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New module http #4156

New module http #4156

christiangalsterer commented May 1, 2017

christiangalsterer commented May 1, 2017

elasticmachine commented May 1, 2017

elasticmachine commented May 1, 2017

ruflin left a comment

ruflin May 1, 2017

christiangalsterer May 1, 2017

ruflin May 1, 2017

christiangalsterer May 1, 2017

ruflin May 1, 2017

christiangalsterer May 1, 2017

ruflin May 1, 2017

christiangalsterer May 1, 2017

ruflin May 1, 2017

ruflin May 1, 2017

christiangalsterer May 1, 2017

christiangalsterer May 1, 2017

ruflin May 1, 2017

christiangalsterer commented May 1, 2017

ruflin left a comment

ruflin May 1, 2017

christiangalsterer May 1, 2017

ruflin commented May 1, 2017

ruflin commented May 2, 2017

ruflin commented May 2, 2017

tsg May 2, 2017

tsg May 2, 2017

tsg May 2, 2017

		@@ -0,0 +1,11 @@
		# Tomcat is started to fetch Jolokia metrics from it

New module http #4156

New module http #4156

Conversation

christiangalsterer commented May 1, 2017

christiangalsterer commented May 1, 2017

elasticmachine commented May 1, 2017

elasticmachine commented May 1, 2017

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

christiangalsterer commented May 1, 2017

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

ruflin commented May 1, 2017

ruflin commented May 2, 2017

ruflin commented May 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment