-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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.
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
[Metricbeat] Migrate php_fpm to ECS #10366
Changes from 4 commits
67d4fba
e4ae0a3
abdd1a6
d8b6eaa
a1e4a79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,25 @@ | ||
{ | ||
"@timestamp": "2018-09-25T16:47:04.998Z", | ||
"@timestamp": "2017-10-12T08:05:34.853Z", | ||
"agent": { | ||
"hostname": "host.example.com", | ||
"name": "host.example.com" | ||
}, | ||
"event": { | ||
"dataset": "php_fpm.process", | ||
"duration": 115000, | ||
"module": "php_fpm" | ||
}, | ||
"http": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, looks pretty good. One comment about method, but that's it |
||
"request": { | ||
"method": "get" | ||
}, | ||
"response": { | ||
"body": { | ||
"bytes": 0 | ||
} | ||
} | ||
}, | ||
"metricset": { | ||
"module": "php_fpm", | ||
"host": "phpfpm:81", | ||
"rtt": 24681267, | ||
"name": "process" | ||
}, | ||
"php_fpm": { | ||
|
@@ -13,25 +29,25 @@ | |
"process": { | ||
"last_request_cpu": 0, | ||
"last_request_memory": 2097152, | ||
"request_duration": 1404, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will revert this change as the two durations are different: https://github.com/elastic/beats/pull/10366/files#r251730563 |
||
"request_duration": 204, | ||
"requests": 6, | ||
"script": "-", | ||
"pid": 6, | ||
"start_time": 1537974596, | ||
"user": "-", | ||
"state": "Idle", | ||
"start_since": 3946, | ||
"requests": 28, | ||
"request_method": "GET", | ||
"request_uri": "/status?json=", | ||
"content_length": 0 | ||
"start_since": 128, | ||
"start_time": 1548769887, | ||
"state": "Idle" | ||
} | ||
}, | ||
"beat": { | ||
"name": "host.example.com", | ||
"hostname": "host.example.com", | ||
"version": "7.0.0-alpha1" | ||
"process": { | ||
"pid": 17 | ||
}, | ||
"service": { | ||
"address": "127.0.0.1:81", | ||
"type": "php_fpm" | ||
}, | ||
"url": { | ||
"original": "/status?full=\u0026json=" | ||
}, | ||
"host": { | ||
"name": "DESKTOP-RFOOE09" | ||
"user": { | ||
"name": "-" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ package process | |
|
||
import ( | ||
"encoding/json" | ||
"strings" | ||
|
||
"github.com/elastic/beats/metricbeat/mb" | ||
|
||
|
@@ -55,22 +56,40 @@ func eventsMapping(r mb.ReporterV2, content []byte) { | |
} | ||
//remapping process details to match the naming format | ||
for _, process := range status.Processes { | ||
proc := common.MapStr{ | ||
"pid": process.PID, | ||
"state": process.State, | ||
"start_time": process.StartTime, | ||
"start_since": process.StartSince, | ||
"requests": process.Requests, | ||
"request_duration": process.RequestDuration, | ||
"request_method": process.RequestMethod, | ||
"request_uri": process.RequestURI, | ||
"content_length": process.ContentLength, | ||
"user": process.User, | ||
"script": process.Script, | ||
"last_request_cpu": process.LastRequestCPU, | ||
"last_request_memory": process.LastRequestMemory, | ||
event := mb.Event{ | ||
RootFields: common.MapStr{ | ||
"http": common.MapStr{ | ||
"request": common.MapStr{ | ||
"method": strings.ToLower(process.RequestMethod), | ||
}, | ||
"response": common.MapStr{ | ||
"body": common.MapStr{ | ||
"bytes": process.ContentLength, | ||
}, | ||
}, | ||
}, | ||
"user": common.MapStr{ | ||
"name": process.User, | ||
}, | ||
"process": common.MapStr{ | ||
"pid": process.PID, | ||
}, | ||
"url": common.MapStr{ | ||
"original": process.RequestURI, | ||
}, | ||
}, | ||
MetricSetFields: common.MapStr{ | ||
"state": process.State, | ||
"start_time": process.StartTime, | ||
"start_since": process.StartSince, | ||
"requests": process.Requests, | ||
"request_duration": process.RequestDuration, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Request duration should go to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed. |
||
"script": process.Script, | ||
"last_request_cpu": process.LastRequestCPU, | ||
"last_request_memory": process.LastRequestMemory, | ||
}, | ||
} | ||
event := mb.Event{MetricSetFields: proc} | ||
|
||
event.ModuleFields = common.MapStr{} | ||
event.ModuleFields.Put("pool.name", status.Name) | ||
r.Event(event) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.