-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Updates to status API, re-align status page #10180
Conversation
src/server/status/metrics.js
Outdated
collection_time_in_millis: config.get('ops.interval'), | ||
uptime_in_millis: process.uptime() * 1000, | ||
process: { | ||
heap: { |
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.
Do we want to nest this under memory
as monitoring has done?
src/server/status/metrics.js
Outdated
const timestamp = new Date().toISOString(); | ||
kbnServer.metrics = { | ||
last_updated: timestamp, | ||
collection_time_in_millis: config.get('ops.interval'), |
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.
I really dislike shorthand, but it appears this is the standard set in ES. Thoughts on collection_interval_in_millis
? I feel like we can make it a bit more descriptive.
src/server/status/index.js
Outdated
status: kbnServer.status.toJSON() | ||
}; | ||
|
||
if (config.get('status.metrics')) { |
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.
Is there a time which we would not want to return the metrics? This could cause problems with beats consuming it if they can't rely on it being returned.
In 5.x we will need to provide a config to toggle the format introduced in this PR.
aaf4cc0
to
f2239b3
Compare
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.
LGTM
Tests failed on this |
I'm getting this error in browser console when I visit the status page:
|
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.
This is looking good other than the bug in the console log.
src/server/status/metrics.js
Outdated
}, | ||
os: { | ||
cpu: { | ||
load: { |
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.
Can we change this to load_average
to align with ES?
src/server/status/metrics.js
Outdated
process: { | ||
mem: { | ||
heap: { | ||
total_in_bytes: _.get(event, 'psmem.heapTotal'), |
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.
Can we change this to heap_max_in_bytes
and nest under mem
to align with ES?
src/server/status/metrics.js
Outdated
mem: { | ||
heap: { | ||
total_in_bytes: _.get(event, 'psmem.heapTotal'), | ||
used_in_bytes: _.get(event, 'psmem.heapUsed') |
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.
Can we change this to heap_used_in_bytes
and nest under mem
to align with ES?
@epixa, this is ready for another look. |
If you're both comfortable with this, I am. I gave it a pretty thorough review before, and the recent changes are minimal. |
@ruflin Do you want to look at this API response before we merge it? |
Here is an example output of {
"name":"tsmalley",
"uuid":"43469b79-7cfa-46fb-a187-387f2d1252d3",
"version":{
"number":"6.0.0-alpha1",
"build_hash":"6cb7fec4e154faa0a4a3fee4b33dfef91b9870d9",
"build_number":8467,
"build_snapshot":false
},
"status":{
"overall":{
"state":"red",
"title":"Red",
"nickname":"Danger Will Robinson! Danger!",
"icon":"danger",
"since":"2017-02-15T22:05:09.916Z"
},
"statuses":[
{
"id":"ui settings",
"state":"red",
"icon":"danger",
"message":"Elasticsearch plugin is red",
"since":"2017-02-15T22:05:10.198Z"
},
{
"id":"plugin:kibana@6.0.0-alpha1",
"state":"green",
"icon":"success",
"message":"Ready",
"since":"2017-02-15T22:05:09.820Z"
},
{
"id":"plugin:elasticsearch@6.0.0-alpha1",
"state":"red",
"icon":"danger",
"message":"Unable to connect to Elasticsearch at http://localhost:9200.",
"since":"2017-02-15T22:05:09.916Z"
},
{
"id":"plugin:console@6.0.0-alpha1",
"state":"green",
"icon":"success",
"message":"Ready",
"since":"2017-02-15T22:05:09.907Z"
},
{
"id":"plugin:timelion@6.0.0-alpha1",
"state":"green",
"icon":"success",
"message":"Ready",
"since":"2017-02-15T22:05:10.189Z"
}
]
},
"metrics":{
"last_updated":"2017-02-15T22:05:23.605Z",
"collection_interval_in_millis":5000,
"uptime_in_millis":19416,
"process":{
"mem":{
"heap_max_in_bytes":201592832,
"heap_used_in_bytes":174029824
}
},
"os":{
"cpu":{
"load_average":{
"1m":4.51513671875,
"5m":3.46142578125,
"15m":3.46142578125
}
}
},
"response_times":{
"avg_in_millis":null,
"max_in_millis":0
},
"requests":{
"total":0,
"disconnects":0,
"status_codes":{
}
},
"concurrent_connections":2
}
} |
@epixa @tylersmalley LGTM. Most important part for me is that there are no arrays (beside the Thanks for all the work on this one. |
before:
after: