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.

Sign up for GitHub

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

Jump to bottom

Add http server metricset to support push metrics via http #4770

Merged
merged 1 commit into from
Aug 1, 2017
Merged

Add http server metricset to support push metrics via http #4770

merged 1 commit into from
Aug 1, 2017

Conversation

vjsamuel
Copy link
Contributor

@vjsamuel vjsamuel commented Jul 27, 2017 •
edited
Loading

This PR allows users to configure a http server in order to send data. The restriction right now it only supports JSON.

Path parameters can be parsed into tags. Ex:
/foo/bar would result in

tag:  {
  foo: "bar",
}

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically on build-eu-00. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

exekias
exekias reviewed Jul 27, 2017
View reviewed changes
metricbeat/helper/server/http/http.go Outdated
}

func (h *HttpServer) handleFunc(writer http.ResponseWriter, req *http.Request) {
if req.Method == "POST" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should provide a friendly response if it's not POST, at least implement a GET message?

exekias
exekias reviewed Jul 27, 2017
View reviewed changes
metricbeat/module/http/_meta/config.yml Outdated
# paths:
# - path: "/foo"
# namespace: "foo"
# tags:
Copy link
Contributor

@exekias exekias Jul 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think it would make sense to change this behavior, use fields instead of tags, and put given fields in the final event, overriding any existing one, what do you think?

exekias
exekias reviewed Jul 27, 2017
View reviewed changes
CHANGELOG.asciidoc Outdated
@@ -52,7 +52,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta1...master[Check the HEAD di

- Add support to exclude labels from kubernetes pod metadata. {pull}4757[4757]
- Add graphite protocol metricbeat module. {pull}4734[4734]

- Add http server metricset to support push metrics via http. {pull}4770[4770]
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing newline after this

@vjsamuel vjsamuel force-pushed the http_push branch 5 times, most recently from 908e001 to b65150f Compare July 27, 2017 21:32
ruflin
ruflin reviewed Jul 31, 2017
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

What happens if the port is already taken? What happens when the same metricset is defined twice with the same port? What happens if reloading is used and part is already used? Trying to make sure we thought of the edge cases.

metricbeat/docs/fields.asciidoc Outdated


[float]
=== http.server.example
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably should not have "example" fields in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have removed this field from the asciidoc. during restart/multiple definitions we would error out with a critical message saying port is already in use.

@exekias
Copy link
Contributor

exekias commented Jul 31, 2017

ok to test

@exekias
Copy link
Contributor

exekias commented Jul 31, 2017

You may want to take a look to travis tests, the fail looks legit

@vjsamuel
Implementing graphite protocol metricbeat module (elastic#4734)
f821e13 
* Implementing graphite protocol metricbeat module
@exekias
Copy link
Contributor

exekias commented Jul 31, 2017

jenkins retest this

@exekias exekias merged commit eb92974 into elastic:master Aug 1, 2017
@exekias exekias added backport enhancement Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. and removed backport labels Aug 1, 2017
@vjsamuel vjsamuel deleted the http_push branch August 1, 2017 17:03
@tsg tsg removed the needs_backport PR is waiting to be backported to other branches. label Aug 24, 2017
exekias pushed a commit to exekias/beats that referenced this pull request Aug 24, 2017
@vjsamuel @exekias
Implementing graphite protocol metricbeat module (elastic#4734) (elas…
dc2ddd0 
…tic#4770)

* Implementing graphite protocol metricbeat module
(cherry picked from commit eb92974)
@exekias exekias mentioned this pull request Aug 24, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@exekias exekias exekias left review comments

@ruflin ruflin ruflin left review comments

Assignees
No one assigned
Labels
enhancement Metricbeat Metricbeat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vjsamuel @elasticmachine @exekias @ruflin @tsg