Refactor http.server metricset #7100
Conversation
ruflin
added
in progress
| @@ -77,6 +77,7 @@ func (p *metricProcessor) Process(event server.Event) (common.MapStr, error) { | |||
| return nil, errors.New(fmt.Sprintf("Unsupported Content-Type: %s", contentType)) | |||
| } | |||
|
|
|||
| // QUESTION: What is the exact expected behaviour here if no list of paths is set | |||
There was a problem hiding this comment.
@vjsamuel Could you comment here? It seems right now if I just run it the events end up under http/http. Not sure if that is expected.
There was a problem hiding this comment.
func defaultHttpServerConfig() HttpServerConfig {
return HttpServerConfig{
DefaultPath: PathConfig{
Path: "/",
Namespace: "http",
},
}
}
it just exposes an API on / where you can send the data and the default namespace would be http
There was a problem hiding this comment.
Ok, at the moment the default is http/http so this is a bug I assume.
ruflin
force-pushed
the
http-server-push-reporter-v2
branch
2 times, most recently
from
ca5173a
to
6d39dee
Compare
ruflin
changed the title
|
@vjsamuel As I already touched the metricset I cleaned it up a bit which also means some breaking changes. Please have a look. |
ruflin
force-pushed
the
http-server-push-reporter-v2
branch
from
6d39dee
to
5b0b400
Compare
| Paths []PathConfig `config:"paths"` | ||
| DefaultPath PathConfig `config:"default_path"` | ||
| Paths []PathConfig `config:"server.paths"` | ||
| DefaultPath PathConfig `config:"server.default_path"` |
There was a problem hiding this comment.
LGTM, if you want to do a softer deprecation you could still read old settings before these, but I don't think this is a hard requirement.
| port: "8080" | ||
| server.paths: | ||
| - path: "/foo" | ||
| namespace: "foo" |
metricbeat/tests/system/test_http.py
Outdated
|
|
||
| self.wait_until(lambda: self.log_contains("Starting http server on localhost:8080")) | ||
|
|
||
| requests.post('http://localhost:8080/', json={'hello': 'world'}, headers={'Content-Type': 'application/json'}) |
There was a problem hiding this comment.
This request should go to the value in $HTTP_HOST instead of to localhost.
|
PR updated with review comment. |
|
Failure in travis could be related https://travis-ci.org/elastic/beats/jobs/383064425#L4420 |
ruflin
force-pushed
the
http-server-push-reporter-v2
branch
from
aa8feac
to
44358cf
Compare
This PR cleans up the http.server metricset which fixes a bug but also has breaking changes. Breaking changes: * Before events were put under `http.http` prefix by default which was not intended. Now it is `http.server` by default. * Config options for the metricset are prefixed now by `server.` as we have for other metricsets. Further changes * Metricset now uses PushReporterV2 interface * System test added to confirm data structure * Add more docs
This PR cleans up the http.server metricset which fixes a bug but also has breaking changes.
Breaking changes:
http.httpprefix by default which was not intended. Now it ishttp.serverby default.server.as we have for other metricsets.Further changes