Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add latency to index and node Elasticsearch stats #22625

Merged
merged 7 commits into from
Oct 23, 2018
+2,888 −6,631
Merged

Add latency to index and node Elasticsearch stats #22625

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
89efc5f
Add latency to index and node Elasticsearch stats
ruflin Sep 3, 2018
55200ca
cleanup
ruflin Oct 16, 2018
58165f4
fix title
ruflin Oct 19, 2018
25091e5
update snapshots
ruflin Oct 19, 2018
11dbd73
update fixture
ruflin Oct 23, 2018
6dcf3ee
one more fix
ruflin Oct 23, 2018
f57f825
Update fixture
chrisronline Oct 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/public/views/elasticsearch/index/advanced/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<div class="col-md-6"><monitoring-chart series="pageData.metrics.index_throttling"/></div>
<div class="col-md-6"><monitoring-chart series="pageData.metrics.index_disk"/></div>
<div class="col-md-6"><monitoring-chart series="pageData.metrics.index_segment_count"/></div>
<div class="col-md-6"><monitoring-chart series="pageData.metrics.index_latency"/></div>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisronline I'm trying to figure out how to set the title for this one. It seems to set the title from index_index_latency.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea the title is managed by the metric itself, so it's a little disconnected from the UI. You'll need to change it there

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where I'm stuck. index_latency itself is not a metric but the name of 2 metrics combined. Interestingly in the UI it shows the title of the first metric.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The corresponding Javascript file defines that combined "series" name when it's a combined chart.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that you figured that out after reading this. But we do grab the first title:

https://github.com/elastic/kibana/blob/master/x-pack/plugins/monitoring/public/components/chart/get_title.js#L13-L19

Generally speaking though, we define unique metrics per display.

</div>
</div>
</monitoring-main>
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/public/views/elasticsearch/node/advanced/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<div class="col-md-6"><monitoring-chart series="pageData.metrics.node_read_threads"/></div>
<div class="col-md-6"><monitoring-chart series="pageData.metrics.node_cgroup_cpu"/></div>
<div class="col-md-6"><monitoring-chart series="pageData.metrics.node_cgroup_stats"/></div>
<div class="col-md-6"><monitoring-chart series="pageData.metrics.node_latency"/></div>
</div>
</div>
</monitoring-main>
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/server/lib/metrics/elasticsearch/metrics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export const metrics = {
'Average latency for indexing documents, which is time it takes to index documents divided by number that were indexed. This considers any shard located on this node, including replicas.', // eslint-disable-line max-len
type: 'node'
}),
index_latency: new LatencyMetric({
index_index_latency: new LatencyMetric({
metric: 'index',
fieldSource: 'index_stats.primaries',
field: 'index_stats.primaries.indexing.index_total',
Expand Down Expand Up @@ -98,7 +98,7 @@ export const metrics = {
'Average latency for searching, which is time it takes to execute searches divided by number of searches submitted. This considers primary and replica shards.', // eslint-disable-line max-len
type: 'node'
}),
query_latency: new LatencyMetric({
index_query_latency: new LatencyMetric({
metric: 'query',
fieldSource: 'index_stats.total',
field: 'index_stats.total.search.query_total',
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/monitoring/server/routes/api/v1/elasticsearch/metric_set_index_detail.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ export const metricSet = {
{
keys: ['index_segment_count_total', 'index_segment_count_primaries'],
name: 'index_segment_count'
},
{
keys: ['index_index_latency', 'index_query_latency'],
name: 'index_latency'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisronline It's here that have have 2 metrics combined in 1 graph. Where do I modify the title of the graph. I tried to create an "empty" metric index_latency but this trick didn't work. It still picks the name of the first metric.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. It uses the title for the first metric found in the set, so in this case index_index_latency. That's what you're seeing right? That's the behavior I found as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but in this case the title should not be from the first metric. And there are quite a few examples in Advance where this is also not the case but I didn't find out so far how this is changed. @pickypg might share some insights here?

}
],
overview: [
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/monitoring/server/routes/api/v1/elasticsearch/metric_set_node_detail.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ export const metricSets = {
{
keys: ['node_cgroup_periods', 'node_cgroup_throttled_count'],
name: 'node_cgroup_stats'
},
{
keys: ['node_query_latency', 'node_index_latency'],
name: 'node_latency'
}
],
overview: [
Expand Down