Skip to content

[ML] Fix zoom missing in Anomaly detection URLs - #86182

Merged
qn895 merged 9 commits into
elastic:masterfrom
qn895:ml-fix-zoom-not-working
Dec 22, 2020
Merged

[ML] Fix zoom missing in Anomaly detection URLs#86182
qn895 merged 9 commits into
elastic:masterfrom
qn895:ml-fix-zoom-not-working

Conversation

@qn895

@qn895 qn895 commented Dec 16, 2020

Copy link
Copy Markdown
Member

Summary

This PR fixes #86141 where the zoom time range set is missing when navigating from the Anomaly Explorer to the Single Metric Viewer. It also fixes not being able to ctrl+click/right click to open in new tab previously.

Checklist

@qn895 qn895 self-assigned this Dec 16, 2020
@qn895
qn895 requested a review from a team as a code owner December 16, 2020 20:17
@elasticmachine

elasticmachine commented Dec 16, 2020

Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

@qn895 qn895 added bug Fixes for quality problems that affect the customer experience release_note:fix and removed release_note:fix labels Dec 16, 2020
darnautov
darnautov approved these changes Dec 17, 2020
View reviewed changes

@darnautov darnautov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, just need to update the URL generator unit tests to make sure zoom is set as expected

Comment thread x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container.js Outdated
Comment thread x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container.js
@darnautov

darnautov commented Dec 17, 2020

Copy link
Copy Markdown
Contributor

@qn895 @peteharverson I just checked the latest 7.10.2 build, this issue is presented there as well.

peteharverson
peteharverson reviewed Dec 17, 2020
View reviewed changes

@peteharverson peteharverson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Zoom is now set correctly, but added a couple of comments of possible things to fix.

Comment thread x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container.js
};
}, [mlUrlGenerator]);

const addToRecentlyAccessed = useCallback(() => {

@peteharverson peteharverson Dec 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not related to the changes in this PR, but looks like the link added to the recently accessed list is wrong. For me it adds a link to (note vlq/app/ml is duplicated) e.g.

http://localhost:5601/vlq/app/ml/vlq/app/ml/timeseriesexplorer?_g=(ml%3A(jobIds%3A!(gallery_mean_bytes_15m))%2CrefreshInterval%3A(display%3AOff%2Cpause%3A!t%2Cvalue%3A0)%2Ctime%3A(from%3A%272019-08-01T00%3A03%3A40.000Z%27%2Cmode%3Aabsolute%2Cto%3A%272020-12-16T11%3A33%3A45.000Z%27))&_a=(timeseriesexplorer%3A(mlTimeSeriesExplorer%3A(detectorIndex%3A0%2Centities%3A(method%3APOST))%2Cquery%3A(query_string%3A(query_string%3A(analyze_wildcard%3A!t%2Cquery%3A%27*%27)))))

@qn895 qn895 Dec 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed here 7f82298 (#86182). Seems like this is also an issue on 7.10.2 so I'll be making a backport to 7.10 as well.

Comment thread x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container.js Outdated
@qn895 qn895 mentioned this pull request Dec 17, 2020
Merged
1 task done
@qn895

qn895 commented Dec 18, 2020

Copy link
Copy Markdown
Member Author

I just checked the latest 7.10.2 build, this issue is presented there as well.

Thanks Dima, I have created a PR for 7.10 here.

peteharverson
peteharverson approved these changes Dec 22, 2020
View reviewed changes

@peteharverson peteharverson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested latest edits and LGTM

@qn895

qn895 commented Dec 22, 2020

Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@kibanamachine

kibanamachine commented Dec 22, 2020

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 7.0MB 7.0MB +2.2KB

Distributable file count

id before after diff
default 47144 47904 +760

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 72.2KB 72.3KB +69.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@qn895
qn895 merged commit 4c17faa into elastic:master Dec 22, 2020
@qn895
qn895 deleted the ml-fix-zoom-not-working branch December 22, 2020 18:12
@qn895 qn895 mentioned this pull request Dec 22, 2020
Merged
qn895 added a commit to qn895/kibana that referenced this pull request Dec 22, 2020
@qn895 @kibanamachine
[ML] Fix zoom missing in Anomaly detection URLs (elastic#86182)
1b83318 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@qn895 qn895 mentioned this pull request Dec 22, 2020
Merged
qn895 added a commit to qn895/kibana that referenced this pull request Dec 22, 2020
@qn895 @kibanamachine
[ML] Fix zoom missing in Anomaly detection URLs (elastic#86182)
f5c039c 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
qn895 added a commit that referenced this pull request Dec 22, 2020
@qn895 @kibanamachine
[7.x] [ML] Fix zoom missing in Anomaly detection URLs (#86182) (#86813)
0737534 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
qn895 added a commit that referenced this pull request Dec 22, 2020
@qn895 @kibanamachine
[7.11] [ML] Fix zoom missing in Anomaly detection URLs (#86182) (#86814)
638c450 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
@qn895 @kibanamachine
[ML] Fix zoom missing in Anomaly detection URLs (elastic#86182)
92e8148 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@darnautov darnautov darnautov approved these changes
@peteharverson peteharverson peteharverson approved these changes

Assignees

@qn895 qn895

Labels

bug Fixes for quality problems that affect the customer experience Feature:Anomaly Detection ML anomaly detection :ml release_note:fix v7.10.2 v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ML] Time range incorrect when opening Single Metric Viewer from Anomaly Explorer

5 participants

@qn895 @elasticmachine @darnautov @kibanamachine @peteharverson