[ML] Return statistics about forecasts as part of the jobsstats and usage API #31647

hendrikmuhs · 2018-06-28T09:43:36Z

This change adds stats about forecasts, to the jobstats api as well as xpack/_usage. The following information is collected:

_xpack/ml/anomaly_detectors/{jobid|_all}/_stats:

total number of forecasts
memory statistics (mean/min/max)
runtime statistics
record statistics
counts by status

_xpack/usage

collected by job status as well as overall (_all):
- total number of forecasts
- number of jobs that have at least 1 forecast
- memory, runtime, record statistics
- counts by status

Fixes #31395

Implementation notes:

reused existing functionality but had to move classes to core due to dependencies
added functionality to use aggregations for the collection of statistics

Todo:

check documentation/specs (https://www.elastic.co/guide/en/elasticsearch/reference/current/index.html)
schema updates for consumers of above API's

elasticmachine · 2018-06-28T09:43:38Z

Pinging @elastic/ml-core

hendrikmuhs · 2018-06-28T09:52:34Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/stats/StatsAccumulator.java

+/**
+ * Helper class to collect min, max, avg and total statistics for a quantity
+ */
+public class StatsAccumulator implements Writeable {


this has been moved from o.e.x.ml.utils

hendrikmuhs · 2018-06-28T12:11:49Z

Examples:

_xpack/ml/anomaly_detectors/{job|_all}/_stats

_xpack/ml/anomaly_detectors/{job|_all}/_stats no forecast

Note: No statistics, only the total counter showing 0 forecasts for this jobs

_xpack/usage

Note: jobs (better name anyone?) is the unique count of jobs that have at least 1 forecast. E.g. in this example count == 4, jobs == 3, so 3 out of 4 ML jobs have at least 1 forecast
jobs is omitted if it is 1.

davidkyle

Looks great, seems like naming is the hardest thing.

davidkyle · 2018-06-28T12:02:21Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetJobsStatsAction.java

                this.jobId = Objects.requireNonNull(jobId);
                this.dataCounts = Objects.requireNonNull(dataCounts);
                this.modelSizeStats = modelSizeStats;
+                this.forecastStats = forecastStats;


Also add this to the JobStats(StreamInput), writeTo(StreamOutput) with BWC checks and hashCode & equals

davidkyle · 2018-06-28T12:41:41Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/stats/ForecastStats.java

+            builder.field(Fields.RUNTIME, runtimeStats.asMap());
+            builder.field(Fields.STATUSES, statusCounts.asMap());
+        }
+        if (jobsWithAtleastOneForecast > 1) {


TOTAL is the total number of forecasts made and JOBS is the number of jobs that have been forecasted and it's only written if 2 or more jobs have been forecasted? It may not be understood that the absence of this field and the presence of the other fields means only 1 job has been forecasted.

I agree, this magic is a bit strange. The reason is that I wanted to not show this field for the jobs stats API but only for the usage stats.

davidkyle · 2018-06-28T12:55:25Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearningFeatureSet.java

@@ -215,24 +219,29 @@ private void addJobsUsage(GetJobsStatsAction.Response response) {
                        js -> new StatsAccumulator()).add(detectorsCount);
                modelSizeStatsByState.computeIfAbsent(jobState,
                        js -> new StatsAccumulator()).add(modelSize);
+                forecastStatsByState.computeIfAbsent(jobState,
+                        js -> new ForecastStats()).merge(jobStats.getForecastStats());


Is this not the equivalent to jobStats.getForecastStats()?

not sure if I get the question, only in case the key does not exist yet it is equivalent to jobStats.getForecastStats(). I think I can simplify this using the Map::merge function in this case.

Sorry my mistake I didn't match the brackets properly. I thought the code was new ForecastStats().merge(jobStats.getForecastStats() i.e. merging an empty new object I now see that is not the case

davidkyle · 2018-06-28T13:05:16Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/stats/ForecastStats.java

+        }
+        total += other.total;
+        if (other.total > 0) {
+            ++jobsWithAtleastOneForecast;


fs = new ForecastStats().merge(a.merge(b.merge(c.merge(someStats))))
After this fs. jobsWithAtleastOneForecast would have a value of 1 regardless of what has been merged into should the value of other. jobsWithAtleastOneForecast be used or have I misunderstood this completely.

I think you are right, it's not used like this right now, but that's a bad excuse for not fixing it. it should simply be jobsWithAtleastOneForecast +=other.jobsWithAtleastOneForecast, the if is superflous.

davidkyle · 2018-06-28T13:07:51Z

Note: jobs (better name anyone?) is the unique count of jobs that have at least 1 forecast. E.g. in this example count == 4, jobs == 3, so 3 out of 4 ML jobs have at least 1 forecast
jobs is omitted if it is 1.

I didn't see this before I started the review. How about forecasted_jobs?

hendrikmuhs · 2018-06-29T08:41:17Z

I changed the fieldname as proposed to forecasted_jobs and removed the magic about writing it or not. It's now always written, for stats I still avoid writing them if there isn't any forecast.

davidkyle

Code changes LGTM

as well as xpack/_usage.

dimitris-athanasiou

LGTM2

hendrikmuhs · 2018-07-03T11:12:15Z

@lcawl

Would you mind quickly reviewing: f9804dc ?

lcawl

I added a few suggestions, but otherwise LGTM

…sage API (#31647) This change adds stats about forecasts, to the jobstats api as well as xpack/_usage. The following information is collected: _xpack/ml/anomaly_detectors/{jobid|_all}/_stats: - total number of forecasts - memory statistics (mean/min/max) - runtime statistics - record statistics - counts by status _xpack/usage - collected by job status as well as overall (_all): - total number of forecasts - number of jobs that have at least 1 forecast - memory, runtime, record statistics - counts by status Fixes #31395

* 6.x: Fix not waiting for Netty ThreadDeathWatcher in IT (#31758) (#31789) [Docs] Correct default window_size (#31582) S3 fixture should report 404 on unknown bucket (#31782) [ML] Limit ML filter items to 10K (#31731) Fixture for Minio testing (#31688) [ML] Return statistics about forecasts as part of the jobsstats and usage API (#31647) [DOCS] Add missing get mappings docs to HLRC (#31765) [DOCS] Starting Elasticsearch (#31701) Fix coerce validation_method in GeoBoundingBoxQueryBuilder (#31747) Painless: Complete Removal of Painless Type (#31699) Consolidate watcher setting update registration (#31762) [DOCS] Adds empty 6.3.1 release notes page ingest: Introduction of a bytes processor (#31733) [test] don't run bats tests for suse boxes (#31749) Add analyze API to high-level rest client (#31577) Implemented XContent serialisation for GetIndexResponse (#31675) [DOCS] Typos DOC: Add examples to the SQL docs (#31633) Add support for AWS session tokens (#30414) Watcher: Reenable start/stop yaml tests (#31754) JDBC: Fix stackoverflow on getObject and timestamp conversion (#31735) Support multiple system store types (#31650) Add write*Blob option to replace existing blob (#31729) Split CircuitBreaker-related tests (#31659) Painless: Add Context Docs (#31190) Docs: Remove missing reference Migrate scripted metric aggregation scripts to ScriptContext design (#30111) Watcher: Fix chain input toXcontent serialization (#31721) Remove _all example (#31711) rest-high-level: added get cluster settings (#31706) Docs: Match the examples in the description (#31710) [Docs] Correct typos (#31720) Extend allowed characters for grok field names (#21745) (#31653) (#31722) [DOCS] Check for Windows and *nix file paths (#31648) [ML] Validate ML filter_id (#31535) Fix gradle4.8 deprecation warnings (#31654) Update numbers to reflect 4-byte UTF-8-encoded characters (#27083)

* master: [ML] Rate limit established model memory updates (#31768) [Docs] Correct default window_size (#31582) S3 fixture should report 404 on unknown bucket (#31782) Detach Transport from TransportService (#31727) [ML] Limit ML filter items to 10K (#31731) [ML] Return statistics about forecasts as part of the jobsstats and usage API (#31647) Fixture for Minio testing (#31688) [DOCS] Add missing get mappings docs to HLRC (#31765) [DOCS] Starting Elasticsearch (#31701) Painless: Complete Removal of Painless Type (#31699) Fix not waiting for Netty ThreadDeathWatcher in IT (#31758) Consolidate watcher setting update registration (#31762) Build: re-enabled bwc (#31769) ingest: Introduction of a bytes processor (#31733) Fix coerce validation_method in GeoBoundingBoxQueryBuilder (#31747) Add analyze API to high-level rest client (#31577) [DOCS] Typos DOC: Add examples to the SQL docs (#31633) Add support for AWS session tokens (#30414) Watcher: Reenable start/stop yaml tests (#31754) Implemented XContent serialisation for GetIndexResponse (#31675) JDBC: Fix stackoverflow on getObject and timestamp conversion (#31735) resolveHasher defaults to NOOP (#31723) Account for XContent overhead in in-flight breaker Split CircuitBreaker-related tests (#31659) Add write*Blob option to replace existing blob (#31729) Painless: Add Context Docs (#31190) Watcher: Fix chain input toXcontent serialization (#31721) Docs: Match the examples in the description (#31710) rest-high-level: added get cluster settings (#31706) [Docs] Correct typos (#31720) Clean up double semicolon code typos (#31687) [DOCS] Check for Windows and *nix file paths (#31648) [ML] Validate ML filter_id (#31535) Revert long lines Fix TransportChangePasswordActionTests

Add forecasts (Number of forecasts for an ml jobs) as column to the ML Jobs monitoring page. Related to: elastic/elasticsearch#31647

…ic#20758) Add forecasts (Number of forecasts for an ml jobs) as column to the ML Jobs monitoring page. Related to: elastic/elasticsearch#31647

… (#20889) Add forecasts (Number of forecasts for an ml jobs) as column to the ML Jobs monitoring page. Related to: elastic/elasticsearch#31647

hendrikmuhs added >enhancement discuss WIP v7.0.0 :ml Machine learning v6.4.0 labels Jun 28, 2018

hendrikmuhs commented Jun 28, 2018

View reviewed changes

davidkyle reviewed Jun 28, 2018

View reviewed changes

davidkyle approved these changes Jun 29, 2018

View reviewed changes

hendrikmuhs added team-discuss and removed WIP discuss labels Jun 29, 2018

hendrikmuhs force-pushed the forecaststats branch from f6d246a to 7d4ccd7 Compare July 2, 2018 12:00

Hendrik Muhs added 10 commits July 3, 2018 10:53

add ForecastStats to job stats

0b4bef4

implement stats collection of forecasts, extending the getjobstats api

3eceac9

as well as xpack/_usage.

fix stylecheck issues

11ff017

add forecastStats Writable interface implementation with BWC layer

1b93000

change field to forecasted_jobs, always write it and fix chained merges

b10eb4c

add a test for forecast by status for the usage endpoint

a0f6ed5

add documentation about forecast statistics shown as part of jobcounts.

f9804dc

fix NPE when search fails

d6dd365

fix checkstyle failure

d1fa2fc

fix JobStatsMonitoringDocTests

d1ed68d

hendrikmuhs force-pushed the forecaststats branch from 1aeaa30 to d1ed68d Compare July 3, 2018 08:53

dimitris-athanasiou approved these changes Jul 3, 2018

View reviewed changes

lcawl approved these changes Jul 3, 2018

View reviewed changes

improve documentation

5f91cbc

hendrikmuhs merged commit e9f8442 into elastic:master Jul 4, 2018

hendrikmuhs mentioned this pull request Jul 4, 2018

[ML] _xpack/usage should return stats about forecasting #31395

Closed

hendrikmuhs mentioned this pull request Jul 5, 2018

[ML] add duration statistics to forecast stats #31812

Closed

hendrikmuhs mentioned this pull request Jul 13, 2018

[Monitoring/ML] add number of forecasts to monitoring overview elastic/kibana#20758

Merged

Mpdreamz mentioned this pull request Sep 25, 2018

[meta] 6.4.0 release elastic/elasticsearch-net#3397

Closed

89 tasks

hendrikmuhs deleted the forecaststats branch October 18, 2018 08:30

Mpdreamz mentioned this pull request Oct 22, 2018

[meta] 6.5.0 Release elastic/elasticsearch-net#3457

Closed

codebrain mentioned this pull request Jan 28, 2019

[meta] 6.6.0 Release elastic/elasticsearch-net#3552

Closed

48 tasks

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

codebrain mentioned this pull request Mar 12, 2019

Return statistics about forecasts as part of the job stats and usage API elastic/elasticsearch-net#3593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Return statistics about forecasts as part of the jobsstats and usage API #31647

[ML] Return statistics about forecasts as part of the jobsstats and usage API #31647

hendrikmuhs commented Jun 28, 2018 •

edited

Loading

elasticmachine commented Jun 28, 2018

hendrikmuhs Jun 28, 2018

hendrikmuhs commented Jun 28, 2018

davidkyle left a comment

davidkyle Jun 28, 2018

davidkyle Jun 28, 2018

hendrikmuhs Jun 28, 2018

davidkyle Jun 28, 2018

hendrikmuhs Jun 28, 2018

davidkyle Jun 29, 2018

davidkyle Jun 28, 2018

hendrikmuhs Jun 28, 2018

davidkyle commented Jun 28, 2018

hendrikmuhs commented Jun 29, 2018

davidkyle left a comment

dimitris-athanasiou left a comment

hendrikmuhs commented Jul 3, 2018

lcawl left a comment

[ML] Return statistics about forecasts as part of the jobsstats and usage API #31647

[ML] Return statistics about forecasts as part of the jobsstats and usage API #31647

Conversation

hendrikmuhs commented Jun 28, 2018 • edited Loading

_xpack/ml/anomaly_detectors/{jobid|_all}/_stats:

_xpack/usage

elasticmachine commented Jun 28, 2018

Choose a reason for hiding this comment

hendrikmuhs commented Jun 28, 2018

_xpack/ml/anomaly_detectors/{job|_all}/_stats

_xpack/ml/anomaly_detectors/{job|_all}/_stats no forecast

_xpack/usage

davidkyle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidkyle commented Jun 28, 2018

hendrikmuhs commented Jun 29, 2018

davidkyle left a comment

Choose a reason for hiding this comment

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

hendrikmuhs commented Jul 3, 2018

lcawl left a comment

Choose a reason for hiding this comment

hendrikmuhs commented Jun 28, 2018 •

edited

Loading