docs: Add user docs for opening log files, level filtering, and formatting structured logs. #132
Conversation
WalkthroughThe pull request introduces substantial updates to the user guide for the YScope Log Viewer, enhancing both its structure and content. It replaces placeholder text with a grid layout featuring three informative cards linked to relevant documentation sections. Additionally, new documentation is added on formatters, log formatting capabilities, syntax for format strings, and log-level filtering methods. The Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (12)
docs/src/user-guide/format-string-formatters.md (5)
1-4: Consider adding a practical example in the introductionThe introduction clearly explains what formatters do, but adding a simple before/after example would help users immediately understand their value.
# Formatters Formatters allow you to transform how field values are displayed. See below for a list of -formatters. +formatters. For example, transforming a timestamp from `1701083400` to `2023-11-27 10:30:00`.
16-16: Fix grammatical issue in the colon escaping instructionThe sentence about escaping colons needs grammatical correction.
-- Date string must escape colons with a backslash, e.g., `HH\:mm\:ss`. ++ Date strings must have colons escaped with a backslash, e.g., `HH\:mm\:ss`.🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ....js.org/docs/en/display/format). - Date string must escape colons with a backslash, e....(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
19-21: Enhance timestamp examples with more diverse scenariosThe current examples could be more comprehensive to demonstrate various use cases.
**Examples:** - `{ts:timestamp}` → `2024-11-27T10:30:00Z` - `{ts:timestamp:YYYY-MM-DD}` → `2024-11-27` +- `{ts:timestamp:HH\:mm\:ss}` → `10:30:00` +- `{ts:timestamp:MMM DD, YYYY}` → `Nov 27, 2024`
32-33: Expand Round Formatter examples and clarify rounding rulesThe current example could be enhanced to cover more scenarios and clarify the rounding behaviour.
**Example:** -- `{value:round}` → `5.7` becomes `6` +**Examples:** +- `{value:round}` → `5.7` becomes `6` (rounds up) +- `{value:round}` → `5.2` becomes `5` (rounds down) +- `{value:round}` → `5.5` becomes `6` (rounds up for .5) + +Note: Values are rounded to the nearest integer using standard rounding rules (>= .5 rounds up).
4-4: Add cross-references to related documentationConsider adding links to related documentation sections for better navigation.
formatters. + +For more information, see: +- [Format String Overview](./format-string-overview.md) +- [Format String Syntax](./format-string-syntax.md)docs/src/user-guide/index.md (3)
Line range hint
1-5: Consider enhancing the introduction to be more welcoming and informativeWhile the current introduction is functional, consider adding:
- A welcome message for new users
- A brief overview of what the YScope Log Viewer does
- What users can expect to learn from this guide
# User guide +Welcome to the YScope Log Viewer documentation! This guide will help you effectively analyze and navigate through your log files. + +The YScope Log Viewer is a powerful tool designed to help you explore, filter, and analyze log files with ease. Whether you're working with local or remote logs, this guide will show you how to make the most of the viewer's features. + This guide documents how to use and operate the YScope Log Viewer. Use the left sidebar (if it's hidden, click the {fas}`bars` icon) to navigate to specific docs.🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...art-overview Quick start ^^^ A guide to open files in the log viewer. ::: :::{grid-...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~20-~20: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...ing-overview Format String ^^^ Docs for format strings. ::: :::{grid-item-card} :link...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
13-13: Enhance card descriptions to be more informativeThe current descriptions could be more detailed to better set user expectations:
-A guide to open files in the log viewer. +Learn how to open and work with both local and remote log files in the viewer. -Docs for format strings. +Learn how to customize log display using format strings and available formatters. -Docs for filtering by log level. +Discover how to filter logs by severity levels to focus on what matters most.Also applies to: 20-21, 27-27
🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...art-overview Quick start ^^^ A guide to open files in the log viewer. ::: :::{grid-...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
31-53: Maintain consistent ordering between navigation elementsThe toctree sections follow a different order than their corresponding grid cards above. Consider maintaining the same order throughout the document for better maintainability:
:::{toctree} :hidden: :caption: Quick start quick-start-overview ::: :::{toctree} :hidden: :caption: Format String :glob: format-string-overview format-string-syntax format-string-formatters ::: :::{toctree} :hidden: :caption: Log Level Filter log-level-filter :::docs/src/user-guide/format-string-overview.md (4)
3-5: Consider adding navigation steps to the settings dialogTo enhance user experience, consider adding specific instructions on how to access the settings dialog (e.g., through a menu, keyboard shortcut, or button).
The log viewer can accept a format string to transform JSON logs into human-readable text. You can select which fields to display and how they should be formatted. The format string -input can be found in the settings dialog. +input can be found in the settings dialog (Settings > Format String or press Ctrl+,).
11-11: Add a comment about the timestamp formatConsider adding a note explaining that the timestamp is in Unix epoch milliseconds format to help users understand the value
1732733160216.{ - "ts": 1732733160216, + "ts": 1732733160216, // Unix epoch in milliseconds "level": "ERROR",
26-28: Enhance timestamp formatter documentationConsider adding more details about the timestamp formatter's capabilities, such as:
- Available date-time formats
- Time zone handling
- Custom format options
You can achieve this with the following format string `{ts:timestamp} {level} {message}`. -`{ts:timestamp}` formats the timestamp field (Unix epoch) into a ISO 8601 date format. +`{ts:timestamp}` formats the timestamp field (Unix epoch) into ISO 8601 date format. +The timestamp formatter supports various formats (e.g., `{ts:timestamp:RFC3339}`), +custom patterns, and time zone conversions. See [Format String Syntax](format-string-syntax#timestamp-formatter) for details. `{level}` and `{message}` display the field as is.
31-33: Add language specifier to the code blockAdd a language specifier to the fenced code block for proper syntax highlighting.
-``` +```text 2024-11-27T18:46:00Z ERROR Failed to process payment transaction.<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 82e677ec00d95fd4c076935903d8ea891a50082d and bb5617ac4c2981aaa3144149f1d2848d89b91187. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `docs/src/attachments/individual-filter.png` is excluded by `!**/*.png` * `docs/src/attachments/severity-filter.png` is excluded by `!**/*.png` </details> <details> <summary>📒 Files selected for processing (6)</summary> * `docs/src/user-guide/format-string-formatters.md` (1 hunks) * `docs/src/user-guide/format-string-overview.md` (1 hunks) * `docs/src/user-guide/format-string-syntax.md` (1 hunks) * `docs/src/user-guide/index.md` (1 hunks) * `docs/src/user-guide/log-level-filter.md` (1 hunks) * `docs/src/user-guide/quick-start-overview.md` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (2)</summary> * docs/src/user-guide/log-level-filter.md * docs/src/user-guide/quick-start-overview.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/src/user-guide/format-string-formatters.md</summary> [uncategorized] ~16-~16: The grammatical number of this noun doesn’t look right. Consider replacing it. Context: ....js.org/docs/en/display/format). - Date string must escape colons with a backslash, e.... (AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER) </details> <details> <summary>docs/src/user-guide/format-string-syntax.md</summary> [uncategorized] ~25-~25: Possible missing comma found. Context: ... - Formatter options can contain any character except each brace (left or right), colo... (AI_HYDRA_LEO_MISSING_COMMA) </details> <details> <summary>docs/src/user-guide/index.md</summary> [uncategorized] ~13-~13: This verb may not be in the correct form. Consider using a different form for this context. Context: ...art-overview Quick start ^^^ A guide to open files in the log viewer. ::: :::{grid-... (AI_EN_LECTOR_REPLACEMENT_VERB_FORM) --- [uncategorized] ~20-~20: This verb may not be in the correct form. Consider using a different form for this context. Context: ...ing-overview Format String ^^^ Docs for format strings. ::: :::{grid-item-card} :link... (AI_EN_LECTOR_REPLACEMENT_VERB_FORM) </details> </details> <details> <summary>🪛 Markdownlint (0.35.0)</summary> <details> <summary>docs/src/user-guide/format-string-overview.md</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/src/user-guide/format-string-syntax.md</summary> 5-5: Expected: 2; Actual: 4 Unordered list indentation (MD007, ul-indent) --- 13-13: Expected: 2; Actual: 4 Unordered list indentation (MD007, ul-indent) --- 15-15: Expected: 4; Actual: 8 Unordered list indentation (MD007, ul-indent) --- 17-17: Expected: 4; Actual: 8 Unordered list indentation (MD007, ul-indent) --- 19-19: Expected: 2; Actual: 4 Unordered list indentation (MD007, ul-indent) --- 21-21: Expected: 4; Actual: 8 Unordered list indentation (MD007, ul-indent) --- 23-23: Expected: 2; Actual: 4 Unordered list indentation (MD007, ul-indent) --- 25-25: Expected: 4; Actual: 8 Unordered list indentation (MD007, ul-indent) --- 9-9: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 32-32: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 45-45: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 59-59: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary> <details> <summary>docs/src/user-guide/index.md (1)</summary> `35-35`: **Verify all referenced documentation files exist** Let's ensure all the linked documentation files are present in the repository to avoid broken links. Also applies to: 43-45, 52-52 <details> <summary>✅ Verification successful</summary> **All referenced documentation files are present** All documentation files mentioned in the user guide are correctly placed in the `docs/src/user-guide/` directory. The verification confirms that there are no broken links in the documentation structure. </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Check if all referenced documentation files exist echo "Checking for referenced documentation files..." for file in quick-start-overview.md format-string-overview.md format-string-syntax.md format-string-formatters.md log-level-filter.md; do if [ -f "docs/src/user-guide/$file" ]; then echo "✓ Found: $file" else echo "✗ Missing: $file" fi doneLength of output: 1448
docs/src/user-guide/format-string-overview.md (1)
35-35: LGTM!
The reference link is well-formatted and follows markdown best practices.
There was a problem hiding this comment.
- Can we move the images into the directory where they're used (i.e.,
user-guide)? - Can we enable LFS for
docs/srclike we do in CLP?
There was a problem hiding this comment.
Can we use the format-struct-logs- syntax instead (to match the sidebar caption)?
There was a problem hiding this comment.
Can we rename to log-level-filtering (to match the sidebar caption)?
|
|
||
| ## Timestamp formatter | ||
|
|
||
| Converts millisecond Unix timestamps to human-readable date-time strings. |
There was a problem hiding this comment.
I added millisecond. Is that accurate?
There was a problem hiding this comment.
yes i wanted to support both per slack, but not there yet...
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (11)
docs/src/user-guide/format-string-formatters.md (4)
11-13: Add language specification to the code block.Add a language specification to the fenced code block for better syntax highlighting.
-``` +```text {field:timestamp[:options]}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 11-11: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `15-24`: **Consider enhancing the timestamp formatter documentation.** The documentation would benefit from: 1. Examples showing different timezone formats (e.g., local time vs UTC) 2. Common use cases like relative time formatting (e.g., "2 hours ago") 3. Handling of invalid timestamp values Would you like me to provide examples for these scenarios? --- `31-33`: **Add language specification to the code block.** Add a language specification to the fenced code block for better syntax highlighting. ```diff -``` +```text {field:round}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `35-38`: **Consider expanding the round formatter documentation.** The documentation would benefit from: 1. Examples of edge cases (e.g., negative numbers, very large numbers) 2. Behaviour with non-numeric values 3. Examples of rounding ties (e.g., what happens with 5.5) Would you like me to provide examples for these scenarios? </blockquote></details> <details> <summary>docs/src/user-guide/format-string-overview.md (4)</summary><blockquote> `3-5`: **Consider adding more specific navigation details.** While the gear icon is helpful, consider adding more specific navigation details, such as the location of the settings dialog in the UI (e.g., top-right corner, menu bar, etc.). ```diff -configure the format string through the settings ({fas}`gear`) dialog. +configure the format string through the settings dialog, accessible via the gear icon ({fas}`gear`) in the top-right corner of the viewer.
11-11: Add a comment to clarify the timestamp format.Consider adding a comment to clarify that "ts" contains a Unix timestamp in milliseconds. This will help users understand the value's format before seeing how it's transformed.
- "ts": 1732733160216, + "ts": 1732733160216, // Unix timestamp in milliseconds (2024-11-27T18:46:00Z)
26-28: Enhance the format string explanation.Consider explaining the significance of the colon syntax in
{ts:timestamp}to help users understand the general pattern for applying formatters.-`{ts:timestamp}` formats the timestamp field (Unix epoch) as an ISO 8601 date format while `{level}` -and `{message}` display the specified fields as is. +`{ts:timestamp}` formats the timestamp field (Unix epoch) as an ISO 8601 date format. The colon syntax +`{field:formatter}` indicates that a formatter should be applied, while `{level}` and `{message}` display +the specified fields as is without any formatting.
31-33: Add language specifier to the code block.Add a language specifier to the fenced code block for consistent formatting.
-``` +```text 2024-11-27T18:46:00Z ERROR Failed to process payment.<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>docs/src/user-guide/format-string-syntax.md (3)</summary><blockquote> `1-7`: **Consider adding a brief introductory paragraph.** Adding a concise overview paragraph after the title would help users quickly understand the purpose and importance of format strings in the context of log viewing. ```diff # Format string syntax + +Format strings allow you to customize how log events are displayed in the YScope Log Viewer. They provide a flexible way to extract and format specific fields from log events while maintaining readability. Each format string is composed of:
18-23: Fix typo and add language specifier to code block.There are two technical issues to address:
- "field-nane" should be "field-name"
- The code block should specify a language for proper syntax highlighting
-``` +```text {<field-name>[:<formatter-name>[:<formatter-options>]]}-### field-nane (required)
+### field-name (required)
Defines the key of the field whose value should replace the placeholder.<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `67-93`: **Add language specifiers to code blocks and format JSON consistently.** The examples are excellent, but the code blocks need technical improvements for better readability. ```diff -``` +```json { "@timestamp": 1427153388942, "level": "INFO", "thread": 0, "latency_secs": 56.4, "an.odd.key{name}": "org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties" }-
+text
{@timestamp:timestamp:YYYY-MM-DD HH:MM:ss.SSS} {level} {{thread}} latency={latency_secs:round} {an.odd.key{name}}-``` +```text 2015-03-23 19:29:48.942 INFO {0} latency=56 org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between bb5617ac4c2981aaa3144149f1d2848d89b91187 and 8d7d10a90fa363c3d96b4f5235c0dcf4ea3ee6c5. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `docs/src/user-guide/format-string-formatters.md` (1 hunks) * `docs/src/user-guide/format-string-overview.md` (1 hunks) * `docs/src/user-guide/format-string-syntax.md` (1 hunks) * `docs/src/user-guide/index.md` (1 hunks) * `docs/src/user-guide/log-level-filter.md` (1 hunks) * `docs/src/user-guide/quick-start-overview.md` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * docs/src/user-guide/index.md * docs/src/user-guide/log-level-filter.md * docs/src/user-guide/quick-start-overview.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> <details> <summary>docs/src/user-guide/format-string-formatters.md</summary> 11-11: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/src/user-guide/format-string-overview.md</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/src/user-guide/format-string-syntax.md</summary> 39-39: null Spaces inside code span elements (MD038, no-space-in-code) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/src/user-guide/format-string-syntax.md</summary> [grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once. Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo... (PHRASE_REPETITION) </details> </details> </details> <details> <summary>🔇 Additional comments (6)</summary> <details> <summary>docs/src/user-guide/format-string-formatters.md (2)</summary> `1-4`: **LGTM! Clear and concise introduction.** The introduction effectively explains the purpose of formatters and sets up the document structure. --- `1-38`: **Well-structured documentation that meets the PR objectives.** The document provides clear, foundational documentation for formatters, aligning well with the PR's goal of establishing a user guide. The structure is logical, and the examples are helpful for understanding the basic usage. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 11-11: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>docs/src/user-guide/format-string-overview.md (1)</summary> `35-37`: **LGTM! Well-structured references.** The references section provides clear links to related documentation using proper Markdown syntax. </details> <details> <summary>docs/src/user-guide/format-string-syntax.md (3)</summary> `8-14`: **LGTM! Clear and concise explanation of static text handling.** The section effectively communicates which characters require escaping and how to escape them. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once. Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo... (PHRASE_REPETITION) </details> </details> --- `60-64`: **LGTM! Clear explanation of the implicit newline behaviour.** The section effectively communicates this important default behaviour to users. --- `95-95`: **Verify the formatter documentation link.** Let's ensure the referenced document exists and contains the formatter information. <details> <summary>✅ Verification successful</summary> **Link to formatter documentation is valid and accurate** The referenced document exists at the correct location and contains comprehensive information about the available formatters, including the timestamp and round formatters with their usage, options, and examples. </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Check if the formatter documentation exists fd -t f "format-string-formatters.md" docs/Length of output: 91
Script:
#!/bin/bash # Let's verify the content of the formatter documentation cat docs/src/user-guide/format-string-formatters.mdLength of output: 818
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
docs/src/user-guide/format-struct-logs-syntax.md (4)
8-14: Consider adding an example of escaped characters.The explanation is clear, but adding a simple example would help users understand the escaping rules better.
Example addition:
* `{` * `}` * `\` + +For example: +``` +Hello \{world\} prints as: Hello {world} +```🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo...(PHRASE_REPETITION)
27-28: Improve readability with proper punctuation.Add a comma to improve the sentence structure.
-Field names can contain any character except the following characters must be escaped with a +Field names can contain any character except the following characters, which must be escaped with a🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...bya.b. * Field names can contain any character except the following characters must be...(AI_HYDRA_LEO_MISSING_COMMA)
67-93: Add language specifications to code blocks.Enhance code block readability by specifying the appropriate language.
-``` +```json { "@timestamp": 1427153388942, ... }-
+text
{@timestamp:timestamp:YYYY-MM-DD HH:MM:ss.SSS} ...-``` +```text 2015-03-23 19:29:48.942 INFO {0} ...<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `95-95`: **Use consistent link formatting.** Make the formatter reference link consistent with other internal links in the document. ```diff -For a list of currently supported formatters, see [Formatters](format-string-formatters). +For a list of currently supported formatters, see [Formatters](#format-string-formatters).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
docs/src/user-guide/individual-filter.pngis excluded by!**/*.pngdocs/src/user-guide/severity-filter.pngis excluded by!**/*.png
📒 Files selected for processing (7)
docs/src/user-guide/.gitattributes(1 hunks)docs/src/user-guide/format-struct-logs-formatters.md(1 hunks)docs/src/user-guide/format-struct-logs-overview.md(1 hunks)docs/src/user-guide/format-struct-logs-syntax.md(1 hunks)docs/src/user-guide/index.md(1 hunks)docs/src/user-guide/log-level-filtering.md(1 hunks)docs/src/user-guide/quick-start.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/src/user-guide/.gitattributes
- docs/src/user-guide/log-level-filtering.md
- docs/src/user-guide/quick-start.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/src/user-guide/index.md
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/src/user-guide/format-struct-logs-formatters.md
11-11: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
31-31: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/src/user-guide/format-struct-logs-overview.md
31-31: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/src/user-guide/format-struct-logs-syntax.md
39-39: null
Spaces inside code span elements
(MD038, no-space-in-code)
18-18: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
79-79: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/src/user-guide/format-struct-logs-syntax.md
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo...
(PHRASE_REPETITION)
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...by a.b. * Field names can contain any character except the following characters must be...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (2)
docs/src/user-guide/format-struct-logs-syntax.md (2)
1-7: LGTM! Well-structured document overview.
The document structure is clear and follows a logical organization with proper heading hierarchy and navigation links.
60-64: LGTM! Clear explanation of trailing newline behaviour.
The section effectively communicates the automatic newline addition.
| {<field-name>[:<formatter-name>[:<formatter-options>]]} | ||
| ``` | ||
|
|
||
| ### field-nane (required) |
There was a problem hiding this comment.
Fix typo in heading.
Correct the spelling of "field-nane" to "field-name".
-### field-nane (required)
+### field-name (required)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### field-nane (required) | |
| ### field-name (required) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
docs/src/user-guide/format-struct-logs-overview.md (4)
3-5: Consider adding keyboard shortcut or menu path to settingsThe introduction effectively explains the feature, but users might benefit from knowing how to access the settings dialog (e.g., keyboard shortcut or menu path).
9-19: Add note about timestamp formatConsider adding a brief note explaining that the
tsfield represents Unix epoch milliseconds. This would help users understand the raw timestamp format before seeing how it's transformed.
26-28: Add comma for better readabilityAdd a comma before "while" to improve sentence flow.
-`{ts:timestamp}` formats the timestamp field (Unix epoch) as an ISO 8601 date format while `{level}` +`{ts:timestamp}` formats the timestamp field (Unix epoch) as an ISO 8601 date format, while `{level}`🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ... field (Unix epoch) as an ISO 8601 date format while{level}and{message}display...(AI_HYDRA_LEO_MISSING_COMMA)
31-33: Specify language for code blockAdd a language identifier to the fenced code block for consistent formatting.
-``` +```text🧰 Tools
🪛 Markdownlint (0.35.0)
31-31: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/src/user-guide/format-struct-logs-overview.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/user-guide/format-struct-logs-overview.md
[uncategorized] ~27-~27: Possible missing comma found.
Context: ... field (Unix epoch) as an ISO 8601 date format while {level} and {message} display...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.35.0)
docs/src/user-guide/format-struct-logs-overview.md
31-31: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| For reference docs, see: | ||
| * [Format string syntax](format-struct-logs-syntax) | ||
| * [Formatters](format-stuct-logs-formatters) |
There was a problem hiding this comment.
Fix typo in link text
There's a typo in the formatters link.
-* [Formatters](format-stuct-logs-formatters)
+* [Formatters](format-struct-logs-formatters)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| For reference docs, see: | |
| * [Format string syntax](format-struct-logs-syntax) | |
| * [Formatters](format-stuct-logs-formatters) | |
| For reference docs, see: | |
| * [Format string syntax](format-struct-logs-syntax) | |
| * [Formatters](format-struct-logs-formatters) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
docs/src/user-guide/format-struct-logs-overview.md (2)
9-19: Add a note about timestamp formatConsider adding a brief comment above the JSON example to explain that
tsis in Unix epoch milliseconds format. This would help users understand the timestamp transformation better.For example, for a JSON log with many fields: + +> Note: The `ts` field is in Unix epoch milliseconds format. ```json
31-33: Add language specification to code blockAdd a language specification to the fenced code block for consistent formatting.
-``` +```text 2024-11-27T18:46:00Z ERROR Failed to process payment.<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md (5)</summary><blockquote> `3-6`: **Consider improving the list formatting.** For better readability and consistency with Markdown best practices, consider using complete sentences in the list: ```diff -Each format string is composed of: -* [Static text](#static-text) -* [Field placeholders](#field-placeholders) -* [An implicit trailing newline](#implicit-trailing-newline) +Each format string is composed of the following elements: +* [Static text](#static-text) - Text that appears verbatim in the output +* [Field placeholders](#field-placeholders) - Dynamic elements replaced with field values +* [An implicit trailing newline](#implicit-trailing-newline) - Automatic line ending
9-10: Improve sentence structure.The current wording is awkward. Consider restructuring for clarity:
-Static text may contain any character except the following characters must be escaped with a -backslash: +Static text may contain any character, but the following characters must be escaped with a +backslash:🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...Static text Static text may contain any character except the following characters must be...(AI_HYDRA_LEO_MISSING_COMMA)
19-19: Add language specifier to code block.For better syntax highlighting and documentation consistency:
-``` +```text
27-28: Improve sentence structure.Similar to the earlier section, the grammar needs improvement:
-* Field names can contain any character except the following characters must be escaped with a - backslash: +* Field names can contain any character, but the following characters must be escaped with a + backslash:🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...bya.b. * Field names can contain any character except the following characters must be...(AI_HYDRA_LEO_MISSING_COMMA)
68-75: Add language specifiers to code blocks.For better syntax highlighting and documentation consistency:
-``` +```json { "@timestamp": 1427153388942, ... }-
+text
{@timestamp:timestamp:YYYY-MM-DD HH:MM:ss.SSS} ...-``` +```text 2015-03-23 19:29:48.942 INFO {0} ...Also applies to: 80-81, 92-93 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between fd19fa29a8cb4f3d2ca1c9aec3da9eab78cba263 and c2e92cf4f094aee1ad756423974dced408ca4868. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `docs/src/user-guide/format-struct-logs-overview.md` (1 hunks) * `docs/src/user-guide/format-struct-logs-syntax.md` (1 hunks) * `docs/src/user-guide/index.md` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * docs/src/user-guide/index.md </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> <details> <summary>docs/src/user-guide/format-struct-logs-overview.md</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md</summary> 39-39: null Spaces inside code span elements (MD038, no-space-in-code) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md</summary> [grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once. Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo... (PHRASE_REPETITION) --- [uncategorized] ~9-~9: Possible missing comma found. Context: ...Static text Static text may contain any character except the following characters must be... (AI_HYDRA_LEO_MISSING_COMMA) --- [uncategorized] ~27-~27: Possible missing comma found. Context: ...by `a.b`. * Field names can contain any character except the following characters must be... (AI_HYDRA_LEO_MISSING_COMMA) </details> </details> </details> <details> <summary>🔇 Additional comments (4)</summary> <details> <summary>docs/src/user-guide/format-struct-logs-overview.md (3)</summary> `21-29`: **LGTM!** The format string explanation is clear and effectively demonstrates the relationship between the JSON fields and their formatted output. --- `35-37`: **LGTM!** The references are properly linked and provide clear navigation to related documentation. --- `3-5`: **Verify FontAwesome icon integration** The usage of `{fas}` for the gear icon assumes FontAwesome is properly configured in the documentation setup. </details> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md (1)</summary> `22-22`: **Fix typo in heading.** Correct the spelling of "field-nane" to "field-name". ```diff -### field-nane (required) +### field-name (required)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
docs/src/user-guide/format-struct-logs-syntax.md (5)
1-7: Consider adding a brief example in the introduction.Adding a simple example right after the introduction would help readers quickly grasp the concept before diving into the details.
* [Field placeholders](#field-placeholders) * [An implicit trailing newline](#implicit-trailing-newline) + +For example: +``` +{timestamp:timestamp:YYYY-MM-DD} {level} {message} +```
8-10: Improve grammar in the static text section.The sentence structure could be clearer.
-Static text Static text may contain any character except the following characters must be escaped with a +Static text may contain any character, except that the following characters must be escaped with a backslash:🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo...(PHRASE_REPETITION)
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...Static text Static text may contain any character except the following characters must be...(AI_HYDRA_LEO_MISSING_COMMA)
19-20: Specify language for the code block.Add a language identifier to the code block for proper syntax highlighting.
-``` +```text {<field-name>[:<formatter-name>[:<formatter-options>]]}--- `27-28`: **Improve grammar in the field names section.** The sentence structure could be clearer. ```diff -* Field names can contain any character except the following characters must be escaped with a +* Field names can contain any character, except that the following characters must be escaped with a backslash:🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...bya.b. * Field names can contain any character except the following characters must be...(AI_HYDRA_LEO_MISSING_COMMA)
68-75: Specify language for the JSON code block.Add a language identifier to the code block for proper syntax highlighting.
-``` +```json { "@timestamp": 1427153388942,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
docs/src/user-guide/individual-filter.pngis excluded by!**/*.pngdocs/src/user-guide/severity-filter.pngis excluded by!**/*.png
📒 Files selected for processing (1)
docs/src/user-guide/format-struct-logs-syntax.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/user-guide/format-struct-logs-syntax.md
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character except the fo...
(PHRASE_REPETITION)
[uncategorized] ~9-~9: Possible missing comma found.
Context: ...Static text Static text may contain any character except the following characters must be...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~27-~27: Possible missing comma found.
Context: ...by a.b. * Field names can contain any character except the following characters must be...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.35.0)
docs/src/user-guide/format-struct-logs-syntax.md
39-39: null
Spaces inside code span elements
(MD038, no-space-in-code)
18-18: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
79-79: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| {@timestamp:timestamp:YYYY-MM-DD HH\:MM\:ss.SSS} {level} \{{thread}\} latency={latency_secs:round} {an\.odd\.key\{name\}} | ||
| ``` | ||
|
|
||
| * In the first placeholder, we have the field name `ts`, a formatter called `timestamp`, and the |
There was a problem hiding this comment.
Fix field name inconsistency in the example explanation.
The explanation refers to field name 'ts' but the example uses '@timestamp'.
-* In the first placeholder, we have the field name `ts`, a formatter called `timestamp`, and the
+* In the first placeholder, we have the field name `@timestamp`, a formatter called `timestamp`, and the📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * In the first placeholder, we have the field name `ts`, a formatter called `timestamp`, and the | |
| * In the first placeholder, we have the field name `@timestamp`, a formatter called `timestamp`, and the |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
docs/src/user-guide/format-struct-logs-syntax.md (5)
8-8: Remove redundant wording.The phrase "Static text" is unnecessarily repeated.
-Static text Static text may contain any character, except the following characters must be escaped with a +Static text may contain any character, except the following characters must be escaped with a🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f...(PHRASE_REPETITION)
18-20: Add language specifier to code block.Specify the language for the code block to enable proper syntax highlighting.
-``` +```text {<field-name>[:<formatter-name>[:<formatter-options>]]}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `67-75`: **Add language specifier to JSON example.** Specify 'json' as the language for proper syntax highlighting of the log event example. ```diff -``` +```json { "@timestamp": 1427153388942, "level": "INFO",🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
79-81: Add language specifier to format string example.Specify 'text' as the language for the YScope format string example.
-``` +```text {@timestamp:timestamp:YYYY-MM-DD HH\:MM\:ss.SSS} {level} \{{thread}\} latency={latency_secs:round} {an\.odd\.key\{name\}}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `91-93`: **Add language specifier to output example.** Specify 'text' as the language for the formatted output example. ```diff -``` +```text 2015-03-23 19:29:48.942 INFO {0} latency=56 org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e18af3cbce887517bd413e95b1254288cefb6779 and 6ca30931e98c88a754ab6eb2405206ae0b068f11. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/src/user-guide/format-struct-logs-syntax.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md</summary> [grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once. Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f... (PHRASE_REPETITION) </details> </details> <details> <summary>🪛 Markdownlint (0.35.0)</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md</summary> 39-39: null Spaces inside code span elements (MD038, no-space-in-code) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md (1)</summary> `1-95`: **Documentation is thorough and well-structured.** The format string syntax documentation is comprehensive, with clear explanations and consistent examples. The escaping rules are well-documented, and the examples effectively demonstrate the concepts. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once. Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f... (PHRASE_REPETITION) </details> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 39-39: null Spaces inside code span elements (MD038, no-space-in-code) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 79-79: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 91-91: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: kirkrodrigues <[email protected]>
Co-authored-by: kirkrodrigues <[email protected]>
|
The suggestions lgtm. Feel free to commit them so that you remain the last pusher. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
docs/src/user-guide/format-struct-logs-overview.md (3)
9-19: Consider using more generic example dataWhile the example is clear, using "Failed to process payment" and production-like trace IDs might cause confusion. Consider using more obviously fictional data.
{ "ts": 1732733160216, "level": "ERROR", - "message": "Failed to process payment.", - "trace_id": "abc123def456", - "span_id": "span_789xyz", - "service": "payment-service", - "environment": "production" + "message": "Example error message", + "trace_id": "EXAMPLE-TRACE-ID", + "span_id": "EXAMPLE-SPAN-ID", + "service": "example-service", + "environment": "demo" }
26-28: Consider adding more timestamp format detailsThe explanation of timestamp formatting could be enhanced by mentioning the specific ISO 8601 format being used (UTC vs local time) and linking to the timestamp formatter documentation.
You can achieve this with the following format string `{ts:timestamp} {level} {message}`. -`{ts:timestamp}` formats the timestamp field (Unix epoch) as an ISO 8601 date format while `{level}` -and `{message}` display the specified fields as is. +`{ts:timestamp}` formats the timestamp field (Unix epoch milliseconds) as an ISO 8601 UTC date format +(e.g., YYYY-MM-DDThh:mm:ssZ) while `{level}` and `{message}` display the specified fields as is. +See [timestamp formatter](format-struct-logs-formatters#timestamp) for more options.
31-33: Add language specifier to code blockAdd 'text' as the language specifier for proper syntax highlighting.
-``` +```text 2024-11-27T18:46:00Z ERROR Failed to process payment.<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 31-31: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md (2)</summary><blockquote> `15-58`: **Add language specifiers to code blocks for better readability.** While the content is technically accurate, the code blocks would benefit from language specifiers for proper syntax highlighting. Add language specifiers to the code blocks. For example: ```diff -``` +```text {<field-name>[:<formatter-name>[:<formatter-options>]]}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 39-39: null Spaces inside code span elements (MD038, no-space-in-code) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `65-95`: **Enhance code block readability with language specifiers and consistent JSON formatting.** While the example is comprehensive, it could be improved with proper language specifiers and consistent JSON formatting. Apply these improvements: 1. Add language specifiers to code blocks: ```diff -``` +```json { "@timestamp": 1427153388942, ... }-
+text
{@timestamp:timestamp:YYYY-MM-DD HH:MM:ss.SSS} ...-``` +```text 2015-03-23 19:29:48.942 INFO {0} ...2. Consider formatting the JSON example with consistent indentation: ```diff { - "@timestamp": 1427153388942, - "level": "INFO", - "thread": 0, - "latency_secs": 56.4, - "an.odd.key{name}": "org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties" + "@timestamp": 1427153388942, + "level": "INFO", + "thread": 0, + "latency_secs": 56.4, + "an.odd.key{name}": "org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties" }🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
79-79: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
91-91: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
docs/src/user-guide/format-struct-logs-overview.md(1 hunks)docs/src/user-guide/format-struct-logs-syntax.md(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/src/user-guide/format-struct-logs-overview.md
31-31: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/src/user-guide/format-struct-logs-syntax.md
39-39: null
Spaces inside code span elements
(MD038, no-space-in-code)
18-18: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
79-79: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/src/user-guide/format-struct-logs-syntax.md
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f...
(PHRASE_REPETITION)
🔇 Additional comments (5)
docs/src/user-guide/format-struct-logs-overview.md (2)
1-5: LGTM! Clear and concise overview.
The overview effectively explains the basic functionality and includes appropriate UI references.
35-37: LGTM! References are clear and links are correct.
The references section provides appropriate links to related documentation.
docs/src/user-guide/format-struct-logs-syntax.md (3)
1-7: LGTM! Clear and well-structured introduction.
The introduction effectively outlines the document's structure with proper section links.
8-14: LGTM! Clear explanation of static text handling.
The section effectively explains which characters require escaping. The repetition of "Static text" is appropriate here as one instance is the heading and the other starts the explanatory sentence.
🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f...
(PHRASE_REPETITION)
60-64: LGTM! Clear explanation of newline handling.
The section effectively explains the implicit trailing newline behaviour.
davemarco
changed the title
There was a problem hiding this comment.
while not strictly necessary in this PR, shall we include an example for picking fields from nest objects?
There was a problem hiding this comment.
It is mentioned in the syntax page, but maybe could include in the longer json example. @kirkrodrigues
Can we also add |
…quotes with straight quotes.
kirkrodrigues
dismissed stale reviews from junhaoliao and themself
via
6c0359b
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
docs/src/user-guide/format-struct-logs-syntax.md (6)
8-14: Improve readability by removing redundant wording.The phrase "Static text" is repeated unnecessarily.
Apply this change:
## Static text -Static text may contain any character, except the following characters must be escaped with a +Text may contain any character, except the following characters must be escaped with a🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once.
Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f...(PHRASE_REPETITION)
18-20: Add language specifier to code block.For better syntax highlighting and consistency.
-``` +```text {<field-name>[:<formatter-name>[:<formatter-options>]]}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `25-26`: **Enhance the nested field example.** The current example could be clearer with proper JSON formatting. ```diff * Nested fields can be specified using periods (`.`) to denote hierarchy. - * E.g., the field `{"a:" {"b": 0}}` may be denoted by `a.b`. + * E.g., the field: + ```json + { + "a": { + "b": 0 + } + } + ``` + may be denoted by `a.b`.
68-78: Add language specifier to JSON code block.For better syntax highlighting and consistency.
-``` +```json { "@timestamp": 1427153388942,
82-84: Add language specifier to format string example.For better syntax highlighting and consistency.
-``` +```text {@timestamp:timestamp:YYYY-MM-DD HH\:MM\:ss.SSS} {level} \{{thread}\} latency={latency.secs:round} {an\.odd\.key\{name\}}<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 82-82: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `94-96`: **Add language specifier to output example.** For better syntax highlighting and consistency. ```diff -``` +```text 2015-03-23 19:29:48.942 INFO {0} latency=56 org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 94-94: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7d56440b6407b9bb9bfcaca0b8d289fa11a8bded and 6c0359bca5b6342a452e697d2c60b14da5c52f40. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/src/user-guide/format-struct-logs-syntax.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md</summary> [grammar] ~8-~8: This phrase is duplicated. You should probably use “Static text” only once. Context: ...ewline](#implicit-trailing-newline) ## Static text Static text may contain any character, except the f... (PHRASE_REPETITION) </details> </details> <details> <summary>🪛 Markdownlint (0.35.0)</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md</summary> 39-39: null Spaces inside code span elements (MD038, no-space-in-code) --- 18-18: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 67-67: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 82-82: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 94-94: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary> <details> <summary>docs/src/user-guide/format-struct-logs-syntax.md (2)</summary> `1-7`: **LGTM! Well-structured introduction.** The introduction clearly outlines the three main components of a format string, with proper linking to each section. --- `60-64`: **LGTM! Clear explanation of the implicit newline behaviour.** The section effectively communicates that every format string automatically ends with a newline. </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
davemarco
changed the title
davemarco
changed the title
|
@junhaoliao technically, this dosent fix 86? it was already fixed by another PR. These docs just refer to nested logs. |
Right, we could have closed issue #86 in PR #123 , but now the docs are added which completes the feature. Anyways, the PR and the issue has now been linked and i think it's fine not mentioning the issue in this PR title. |
Description
This PR is a starting point for the user guide. More can be added later.
I added sections for opening files, format string, and log level filtering.
For the format string. I added a simple example for the overview (I thought including only the full spec might overwhelm users). I also kept the full spec in another page. I also added a list of current field formatters.
Validation performed
Docs still work.
Summary by CodeRabbit
New Features
Documentation