[Metricbeat] Migrate php_fpm to ECS #10366
Conversation
ruflin
added
module
review
refactoring
Metricbeat
ruflin
changed the title
| "duration": 115000, | ||
| "module": "php_fpm" | ||
| }, | ||
| "http": { |
There was a problem hiding this comment.
@webmat Could you have a look if this is what you have expected? (check the full data.json)
There was a problem hiding this comment.
Yes, looks pretty good. One comment about method, but that's it
| "duration": 115000, | ||
| "module": "php_fpm" | ||
| }, | ||
| "http": { |
There was a problem hiding this comment.
Yes, looks pretty good. One comment about method, but that's it
| "start_time": process.StartTime, | ||
| "start_since": process.StartSince, | ||
| "requests": process.Requests, | ||
| "request_duration": process.RequestDuration, |
There was a problem hiding this comment.
Request duration should go to event.duration
This makes sure proper integration tests are run.
ruflin
force-pushed
the
php-fpm-reporter
branch
from
cb70d94 to
abdd1a6
Compare
| }, | ||
| "event": { | ||
| "dataset": "php_fpm.process", | ||
| "duration": 115000, |
There was a problem hiding this comment.
@webmat I just realised now that we have here 2 values for event.duration. The event duration from metricbeat which is how long it took to get the event and the event duration reported by the metricset. Probably that means for consistency that the php_fpm duration should stay inside php_fpm?
There was a problem hiding this comment.
Depends on the semantics of the whole metricset. If there's 1 event per http request served, I think event.duration should be the http request duration.
If rather this is more of a sampling (e.g. get metrics every 10s, and report on the last http request), then the Metricbeat event duration is the one that should go to event.duration.
| - from: php_fpm.status.request_duration | ||
| to: event.duration | ||
| alias: true | ||
| beat: metricbeat |
There was a problem hiding this comment.
both entries removed.
| @@ -12,26 +28,25 @@ | |||
| }, | |||
| "process": { | |||
| "last_request_cpu": 0, | |||
| "last_request_memory": 2097152, | |||
| "request_duration": 1404, | |||
There was a problem hiding this comment.
I think this was the request duration of the request processed by this process, that in this case happens to be the request to the status endpoint, but it could be any other thing.
There was a problem hiding this comment.
I will revert this change as the two durations are different: https://github.com/elastic/beats/pull/10366/files#r251730563
|
PR updated. |
Add also TestFetch methods to make sure proper integration tests are run.