[ML] Fix datafeed start time is incorrect when the job has trailing empty buckets #71976
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ML] Change to using earliestStartTimestampMs
qn895
added
release_note:fix
:ml
Feature:Anomaly Detection
|
Pinging @elastic/ml-ui (:ml) |
droberts195
reviewed
Jul 16, 2020
| bucketSpan?: Duration | null | undefined | ||
| ): number | undefined { | ||
| if (latestRecordTimestamp !== undefined && latestBucketTimestamp !== undefined) { | ||
| // if bucket span is availble (e.g. 15m) add it to the latest bucket timestamp in ms |
There was a problem hiding this comment.
Suggested change
| // if bucket span is availble (e.g. 15m) add it to the latest bucket timestamp in ms | |
| // if bucket span is available (e.g. 15m) add it to the latest bucket timestamp in ms |
| ): number | undefined { | ||
| if (latestRecordTimestamp !== undefined && latestBucketTimestamp !== undefined) { | ||
| // if bucket span is availble (e.g. 15m) add it to the latest bucket timestamp in ms | ||
| const latestBucketStartTime = bucketSpan |
There was a problem hiding this comment.
latestBucketStartTime might be a confusing name for future readers of this code, as bucket timestamps are the bucket start time, so latestBucketTimestamp is the start time of the latest bucket we have results for. When you add on the bucket span you're getting the start time of the first bucket we don't have results for.
It might be clearer to rename latestBucketStartTime in this code to adjustedBucketStartTime.
|
@elasticmachine merge upstream |
jgowdyelastic
approved these changes
Jul 16, 2020
| @@ -171,6 +172,11 @@ export function jobsProvider(mlClusterClient: ILegacyScopedClusterClient) { | |||
| description: job.description || '', | |||
| groups: Array.isArray(job.groups) ? job.groups.sort() : [], | |||
| processed_record_count: job.data_counts?.processed_record_count, | |||
| earliestStartTimestampMs: getEarliestDatafeedStartTime( | |||
| dataCounts?.latest_record_timestamp as number, | |||
There was a problem hiding this comment.
it doesn't look like the number type assertions are needed here.
they're also not need for the call to getLatestDataOrBucketTimestamp
There was a problem hiding this comment.
I removed the casting check for both here 09ae890 since they are both not needed.
…com/qn895/kibana into incorrect-datafeed-start-time-69537
💚 Build SucceededBuild metricspage load bundle size
History
To update your PR or re-run it, just comment with: |
qn895
added a commit
to qn895/kibana
that referenced
this pull request
Jul 16, 2020
…mpty buckets (elastic#71976) Co-authored-by: Elastic Machine <[email protected]>
qn895
added a commit
that referenced
this pull request
Jul 16, 2020
…ling empty buckets (#71976) (#72144) Co-authored-by: Elastic Machine <[email protected]>
qn895
added a commit
that referenced
this pull request
Jul 16, 2020
…ling empty buckets (#71976) (#72145) Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jul 17, 2020
* master: (214 commits) replacing hard coded links for ela.st (elastic#72240) skip flaky suite (elastic#60865) chore(NA): teardown dynamic dll plugin (elastic#72096) Register navLink actions for declared applications (elastic#72109) Fix value for process.hash.sha256 draggable (elastic#72142) Call setupIngest before fleet_install tests (elastic#72214) [Security Solution][Detections] Better toast errors (elastic#72205) skip flaky suite (elastic#64696) [Security Solution][Detections] Disable exceptions for Threshold and ML rules (elastic#72137) [Security Solution][Detections,Lists] Miscellaneous post-FF fixes (elastic#71990) [baseline/capture] use high-memory nodes with ramDisks (elastic#71894) skip flaky suite (elastic#77207) [Maps] Fix issue preventing TMS from rendering correctly (elastic#71946) using test_user with minimum privs (elastic#71988) Fixed Webhook connector doesn't retain added HTTP header settings (elastic#71924) [Ingest Manager] Do not show enrolling and unenrolling agents as online in agent counters (elastic#71921) [Maps] fix 'New Map' from getting added to recently accessed (elastic#72125) [Visualizations] Pass 'aggs' parameter to custom request handlers (elastic#71423) [Monitoring] Out of the box alert tweaks (elastic#71942) [ML] Fix datafeed start time is incorrect when the job has trailing empty buckets (elastic#71976) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses #69537 which corrects the earliest start/continue time from
dataCounts?.latest_record_timestamptomax(latest_record_timestamp, latest_bucket.timestamp + bucket_span).Example below is the
gallery1_1594832288_800_7370job with bucket_span of1hwhere:So the earliest available timestamp is 21:00
Before
After
Checklist
Delete any items that are not applicable to this PR.