Painless docs overhaul (troubleshooting)#3649
Painless docs overhaul (troubleshooting)#3649kilfoyle wants to merge 11 commits intoelastic:mainfrom
Conversation
Vale Linting ResultsSummary: 4 suggestions found 💡 Suggestions (4)
The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
yetanothertw
left a comment
There was a problem hiding this comment.
Hi David,
The content looks good so far, my comments are mostly focused around the structure (sections) of the troubleshooting guide.
I only managed to review the first troubleshooting page this morning. I'll keep going and will be adding more comments.
troubleshoot/elasticsearch/painless-array-list-manipulation-errors.md
Outdated
Show resolved
Hide resolved
| } | ||
| ``` | ||
|
|
||
| ## Root cause |
There was a problem hiding this comment.
I'm wondering about this structure. The info in this Root cause section is kind of redundant (it repeats the same info as the topic description, so I'm questioning its user value 🫣)
I only skimmed through some of the existing Troubleshoot > Elasticsearch topics, and I don't think they follow a specific pattern. I do like the idea that a template brings consistency across topics, but it stands out against the existing content -- not sure what the best strategy would be (new topics using a new format/structure or follow existing patterns). I know our team is working on developing guidance and templates for troubleshooting topics (check out this PR it also includes a new template for troubleshooting topics).
For consistency, I think the content would be better aligned with the docs team efforts -- this guidance makes more sense to me. Btw it also recommends that we do not explain causes, so you can safely get rid of this section.
|
|
||
| The error occurs because the script tries to access index 2 (the third element) in an array that only has two elements (indices 0, 1). Arrays in Painless are zero-indexed, so accessing an index greater than or equal to the array size causes an exception. | ||
|
|
||
| ## Solution: Check array bounds before accessing |
There was a problem hiding this comment.
Should Solution be substituted with Resolution?
There was a problem hiding this comment.
Maybe it's because I don't know Painless at all, but I would find callouts useful to orient the reader, what should they be paying attention to?
| } | ||
| ``` | ||
|
|
||
| ## Notes |
There was a problem hiding this comment.
This content should be included higher up in the page ("before the fold"), perhaps incorporated into the page description and/or also in the Resolution section.
| - id: elasticsearch | ||
| --- | ||
|
|
||
| # Troubleshoot array manipulation errors in Painless |
There was a problem hiding this comment.
I feel like this title is too generic. It could be a bit more specific and focused on the issue as its experienced from user the point of view, perhaps something along the lines of:
Accessing an index greater than or equal to the array size causes an exception
There was a problem hiding this comment.
I'm thinking about titles such as these, for example:
- Warning: Not enough nodes to allocate all shard replicas
- Total number of shards for an index on a single node exceeded
- Troubleshoot incomplete migration to data tiers
It would also be better aligned with the WIP guidance:
| } | ||
| ``` | ||
|
|
||
| ## Problematic code |
There was a problem hiding this comment.
I'm questioning the usefulness of this section as well, it goes into overly-explaining territory.
If you want to keep the code, maybe hide it in a dropdown so it doesn't make the page look too bloated? Another thing is the title problematic code sounds like attributing blame and I'm sure that's not the intention at all. It's tricky to find the right balance between trying to help the user without making them feel like they did something wrong (even if technically we're addressing errors in this section).
| } | ||
| ``` | ||
|
|
||
| ## Sample document |
There was a problem hiding this comment.
This feels like it's a part of the solution, right? It's an example that illustrates (the Results section) the solution, so maybe it should be included in the Solution/Resolution section?
yetanothertw
left a comment
There was a problem hiding this comment.
Part 2 of comments. Although they're very similar to the first batch. I think the majority of it is a structure mismatch with other existing content, and best practices guidance that's being worked on.
|
|
||
| # Troubleshoot date math errors in Painless | ||
|
|
||
| Follow these guidelines to avoid [date](elasticsearch://reference/scripting-languages/painless/using-datetime-in-painless.md) operation errors in your Painless scripts. |
There was a problem hiding this comment.
A lot of the structure-related comments apply to this topic as well. I think information in the Solution and Notes sections should be brought into the introduction. You can also summarise why (the root cause) this runtime error occurs.
The error message is more important than the problematic code (which is so similar to the solution it feels like a duplication). I think the solution code example with its explanation is enough.
| - id: elasticsearch | ||
| --- | ||
|
|
||
| # Troubleshoot date math errors in Painless |
There was a problem hiding this comment.
The title would be more useful if it was rewritten with the error message in mind, maybe something like illegal_argument_exception when calling date methods without using .value or Fixing illegal_argument_exception in scripts by accessing date fields via .value
| } | ||
| ``` | ||
|
|
||
| ## Root cause |
There was a problem hiding this comment.
Similarly to other pages, I'd integrate this information into the page description, and the same with the Notes section.
|
|
||
| For example, calling `doc['author_score'].value` directly on a document that does not contain that field causes an error. The recommended approach is to use `doc[<field>].size()==0` to check if the field is missing in a document before accessing its value. | ||
|
|
||
| ## Sample documents |
There was a problem hiding this comment.
Are the various solutions based on these examples? If you really need to keep the examples, maybe they can be collapsed?
Or perhaps they're better suited in a contextual type of a doc where the aim is to educate users?
| } | ||
| ``` | ||
|
|
||
| ## Results |
There was a problem hiding this comment.
Does this example refer to all three solutions? (do they all converge into the same result?) If so, you can just make this codeblock part of the solutions section.
| } | ||
| ``` | ||
|
|
||
| ## Notes |
There was a problem hiding this comment.
This information is repetitive at the end of the page. I'd find ways to add into the problem or solution description.
| - id: elasticsearch | ||
| --- | ||
|
|
||
| # Troubleshoot ingest pipeline failures in Painless |
There was a problem hiding this comment.
Maybe something more specific could work, like Troubleshoot ingest pipeline failures in arithmetic operations in Painless
| } | ||
| ``` | ||
|
|
||
| ## Problematic code |
There was a problem hiding this comment.
I would reframe this as instead of using ctx.nanoseconds = ctx.time_field * 1000000; use ctx.timestamp_nanos = millis * 1000000L; or something like that. I'd mention that in the Solution section.
|
|
||
| ## Root cause | ||
|
|
||
| When accessing fields using `ctx.time_field` in ingest pipelines, the values are not automatically parsed to their mapped field types. The script attempts to multiply a string value (time field) directly with an integer. Time strings such as `"00:00:00.022"` remain as strings. They need to be properly parsed as dates and converted to epoch milliseconds before performing arithmetic operations. |
There was a problem hiding this comment.
This should be part of the intro. The same as the Notes section.
…rors.md Co-authored-by: Vlada Chirmicci <vlada.chirmicci@elastic.co>
|
@yetanothertw Thanks for the careful review! I agree with your comments. In fact, they make me think that these pages don't belong in the troubleshooting section at all, but instead perhaps we can add a "debugging" section to the existing Painless docs in the "Explore and Analyze" section. This has the advantages that i) the Painless docs aren't spread across three locales, and ii) the new pages serve more as guidelines when one is writing Painless scripts rather than how to deal with spontaneous errors, as the other troubleshooting docs are geared toward. Heres's a (still very much in draft mode) PR for how I think these pages would better be delivered: #4784 We can chat about it one day soon. No rush or need to reply. :-) |
|
Closing in favour of #4784 which moves all of this into a "debugging" section together with the other Painless docs in the Explore & Analyze section. |
## Summary This replaces the Painless section that was [proposed for the Troubleshooting section of the docs](#3649), to instead be a [Debugging Painless scripts](https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/4784/explore-analyze/scripting/painless-debugging) section in the Explore & Analyze Painless docs. To my mind, this has two advantages: - These Painless pages aren't true "troubleshooting" docs, and I think would not fit well into the Symptoms / Diagnosis / Resolution format that is now standard for that section. Debugging, here, is something that people need to be mindful of as they're composing their scripts, rather than a spontaneous error that needs diagnosis and recovery. - Embedding this content into the existing Explore and Analyze section means that we have our Painless info in two locations (narrative + reference), rather than split across three separate parts of the docs. Much better as a user experience, IMHO. Preview: Please see [Debug Painless scripts in Elasticsearch](https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/4784/explore-analyze/scripting/painless-debugging) and all sub-topics. --- @yetanothertw I made changes as we talked about, except that I didn't move the "root cause" sections. As I read through I realized that these are specific to each example, so they wouldn't make sense where the description is. But, I have moved the "Notes" sections to be part of the descriptions at the top of each page (above the fold). ## Generative AI disclosure 1. Did you use a generative AI (GenAI) tool to assist in creating this contribution? - [ ] Yes - [x] No --------- Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
This adds a new "Painless scripting" section into the Troubleshoot > Elasticsearch docs, based on the Painless doc project with the Kibernum team.
All changes are in this Troubleshoot Painless scripting in Elasticsearch section.
Related: