Skip to content

[ML] Fixing categorization token highlighting for multi-line messages#103007

Merged
jgowdyelastic merged 2 commits intoelastic:masterfrom
jgowdyelastic:fixing-categorization-tokens-for-multi-line-messages
Jun 29, 2021
Merged

[ML] Fixing categorization token highlighting for multi-line messages#103007
jgowdyelastic merged 2 commits intoelastic:masterfrom
jgowdyelastic:fixing-categorization-tokens-for-multi-line-messages

Conversation

@jgowdyelastic
Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic commented Jun 22, 2021 •
edited
Loading

Fixes issue introduced in elastic/elasticsearch#73828

Multi-lined messages are now no longer lost after the end of the last matched token.

Issue as described by @droberts195
The problem arises when there’s a token that ends at the end of the first line of the message.
Because the first_non_blank_line char filter deletes everything after it, that token is reported as ending at the very end of the original message, even though it’s short.
Then, in the highlighting, the UI replaces the last token on the first line plus all the other lines with the single short token.
Thus making it look like the second and subsequent lines never existed

Solution is to base our end_offset on the token length, rather than the supplied end_offset from the analyze endpoint

@jgowdyelastic jgowdyelastic added bug Fixes for quality problems that affect the customer experience review :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 v7.14.0 labels Jun 22, 2021
@jgowdyelastic jgowdyelastic self-assigned this Jun 22, 2021
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner June 22, 2021 20:32
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 22, 2021

Pinging @elastic/ml-ui (:ml)

droberts195
droberts195 approved these changes Jun 23, 2021
View reviewed changes
Copy link
Copy Markdown

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 changed the title [ML] Fixing categorization tokens for multi-line messages [ML] Fixing categorization token highlighting for multi-line messages Jun 23, 2021
@droberts195
Copy link
Copy Markdown

droberts195 commented Jun 23, 2021

Fixes issue introduced in elastic/elasticsearch#73828

Just to clarify, elastic/elasticsearch#73828 means people using the default categorization analyzer would have seen the problem from 7.14, but it's always been a bug that if you had a char_filter that deleted the end of the original message and had a token that was right at the end of the truncated message then this problem will have occurred. The difference is that in 7.13 and earlier the default categorization analyzer didn't contain a char_filter, so only advanced users who defined their own categorization analyzer would see this.

I am not suggesting that we backport this to 7.13, but I do think it should be release noted as a bug fix, just in case someone ever observes the effect on an older version.

peteharverson
peteharverson approved these changes Jun 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@jgowdyelastic
Copy link
Copy Markdown
Member Author

jgowdyelastic commented Jun 29, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 29, 2021

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @jgowdyelastic

@jgowdyelastic jgowdyelastic added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 29, 2021
@jgowdyelastic jgowdyelastic merged commit 824463a into elastic:master Jun 29, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 29, 2021
@jgowdyelastic @kibanamachine
[ML] Fixing categorization tokens for multi-line messages (elastic#10…
0568275 
…3007)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine kibanamachine mentioned this pull request Jun 29, 2021
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 29, 2021

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 29, 2021
@kibanamachine @jgowdyelastic
[ML] Fixing categorization tokens for multi-line messages (#103007) (#…
b7eb2ec 
…103627)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: James Gowdy <jgowdy@elastic.co>
@jgowdyelastic jgowdyelastic deleted the fixing-categorization-tokens-for-multi-line-messages branch July 19, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peteharverson peteharverson peteharverson approved these changes

+1 more reviewer

@droberts195 droberts195 droberts195 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jgowdyelastic jgowdyelastic

Labels

auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience Feature:Anomaly Detection ML anomaly detection :ml release_note:fix review v7.14.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jgowdyelastic @elasticmachine @droberts195 @kibanamachine @peteharverson