[Metricbeat] Migrate HTTP json Metricset to use ReporterV2 interface#10960
[Metricbeat] Migrate HTTP json Metricset to use ReporterV2 interface#10960sayden merged 5 commits intoelastic:masterfrom
Conversation
b183a40 to
ca973ca
Compare
metricbeat/module/http/json/json.go
Outdated
metricbeat/module/http/json/json.go
Outdated
metricbeat/module/http/json/json.go
Outdated
metricbeat/tests/system/test_http.py
Outdated
There was a problem hiding this comment.
As the namespace is set to test, this should stay under test. Seems like your change does not respect this config anymore but should.
There was a problem hiding this comment.
Will this pass? Shouldn't this be under test?
metricbeat/module/http/json/json.go
Outdated
There was a problem hiding this comment.
you need to report here the event under the configured namespace. Same below.
|
I set this to |
|
Waiting for #11660 to be merged to rebase |
ca973ca to
7f9fcbb
Compare
4918fc3 to
a22fa8c
Compare
| var event common.MapStr | ||
|
|
||
| if m.deDotEnabled { | ||
| event = common.DeDotJSON(jsonBody).(common.MapStr) |
There was a problem hiding this comment.
Not part of this PR so we should follow up but afterwards but I wonder if this type conversion is safe or if we should check it. Also 2 lines below.
There was a problem hiding this comment.
Do you want me to open a follow up PR to discuss it or better to leave it in an issue to discuss?
|
|
||
| return mb.Event{ | ||
| MetricSetFields: event, | ||
| Namespace: "http." + m.namespace, |
There was a problem hiding this comment.
Just a thought (not related to this PR): We should probably rename this to dataset instead of namespace as that is what it is in the event in the end.
metricbeat/module/http/json/json.go
Outdated
| events = append(events, event) | ||
|
|
||
| if reported := reporter.Event(event); !reported { | ||
| return errors.New("error reporting event") |
There was a problem hiding this comment.
This is related to the discussion we need to have in #11666
In general I don't think we should report an error here as it normally means the metricset or beat was stopped on purpose (which is not an error).
metricbeat/module/http/json/json.go
Outdated
| for _, h := range v { | ||
| value += h + " ," | ||
| if reported := reporter.Event(event); !reported { | ||
| return errors.Errorf("error reporting event: %#v", event) |
metricbeat/tests/system/test_http.py
Outdated
There was a problem hiding this comment.
Will this pass? Shouldn't this be under test?
ruflin
left a comment
There was a problem hiding this comment.
LGTM, assuming tests pass ;-)
Refer to #10774 for more info
In this case, a slight change in system tests was needed. I'm not sure of what it was originally doing.