-
Notifications
You must be signed in to change notification settings - Fork 16
docs: Explain how to work with auto-generated keys; Restructure to clarify how to specify keys for authoritative fields. #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update reorganizes and expands the structured logs documentation, shifting focus from formatting to broader guidance on working with structured logs. It introduces new documentation sections, updates internal links, revises terminology for clarity, and aligns helper texts and links in the settings UI with the new documentation structure. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPanel
participant Docs
User ->> SettingsPanel: Open settings dialog
SettingsPanel -->> User: Display form fields (format string, log level key, timestamp key)
User ->> SettingsPanel: Click documentation link for a field
SettingsPanel ->> Docs: Open structured logs documentation (formatting, key syntax, etc.)
Docs -->> User: Show relevant structured logs guide
Possibly related PRs
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/user-guide/format-struct-logs-syntax.md (1)
70-136
: New Key-Value Pair IR Example and Format StringThe addition of the "Key-Value Pair IR Example" section is comprehensive and clearly demonstrates how to format log events that combine auto-generated and user-generated fields. The example with the YScope format string and the resulting output is very illustrative and should help users understand the new convention.
Additionally, to adhere to markdown best practices (as noted by the static analysis hints), it is recommended to specify language identifiers for fenced code blocks. For instance, JSON examples could start with "
json" instead of a plain "
". Here’s a suggested diff for improvement:-``` +```jsonThis small enhancement will improve readability and reduce markdown lint warnings.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
74-74: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
89-89: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
101-101: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
122-122: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
132-132: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/src/user-guide/format-struct-logs-syntax.md
(4 hunks)docs/src/user-guide/log-level-filtering.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/format-struct-logs-syntax.md
74-74: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
122-122: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
132-132: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (3)
docs/src/user-guide/format-struct-logs-syntax.md (2)
27-30
: Clarify Auto-Generated Fields UsageThe new bullet point clearly explains that auto-generated fields in a Key-Value Pair IR Stream can be specified with an
@
prefix, and the accompanying hyperlink enhances context. This addition is clear and aligns well with the updated naming conventions.
34-34
: Update on Escaped CharactersIncluding the
@
character in the list of characters that must be escaped is appropriate now that it has a special meaning. This helps avoid ambiguity when field names use the auto-generated prefix.docs/src/user-guide/log-level-filtering.md (1)
5-8
: Enhanced Explanation for JSON Log FilteringThe updated text now clearly informs users that for JSON logs, filtering can be enabled by entering the log level key in the settings dialog—linking directly to the updated format string documentation. This added context sharpens the guidance and enhances overall clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/src/user-guide/format-struct-logs-syntax.md (1)
74-134
: Specify Languages for Fenced Code BlocksMultiple fenced code blocks in this section (covering both the JSON examples and format string examples) do not specify a language. Adding language annotations—for example, using
```json
for JSON examples—will comply with markdownlint MD040 and improve readability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
74-74: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
89-89: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
101-101: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
122-122: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
132-132: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/user-guide/format-struct-logs-syntax.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/format-struct-logs-syntax.md
74-74: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
89-89: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
108-108: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
122-122: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
132-132: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (3)
docs/src/user-guide/format-struct-logs-syntax.md (3)
27-30
: Clear Explanation of Auto-Generated FieldsThe new lines provide a clear explanation of how auto-generated fields can be denoted using the
@
prefix and include a helpful link to the Key-Value Pair IR Stream documentation. Please verify that the link remains valid and consider adding a brief note on when using auto-generated fields is recommended for extra clarity.
34-34
: Accurate Update to Escapable CharactersThe addition of the bullet item for the
@
character ensures that the documentation now correctly reflects all characters that require escaping in field names. This update aligns well with the new auto-generated fields feature.
105-136
: Effective Addition of the Key-Value Pair IR ExampleThe new Key-Value Pair IR Example is a strong addition that clearly demonstrates the formatting of both auto-generated and user-generated fields. The example is detailed and helps clarify the new conventions introduced in this PR. Just ensure that the fenced code blocks in this section include appropriate language identifiers for consistency.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
108-108: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
122-122: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
132-132: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
For JSON logs, you can enable filtering by entering the log level key in the settings ({fas}`gear`) | ||
dialog. The syntax for the key is the same as the field name in the | ||
[format string](format-struct-logs-syntax). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked at the code for #199, but once it's merged, will we need this section of docs any more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this in the context of #199. It's not immediately clear for users what syntax you should use to set the log level key. There is no link like the format string option. However, if you want, we could put a similar comment right in the settings dialog. Like it currently says [JSON] Key to extract the log level from.
We could maybe append Syntax is the same as field name in the format string
there and for timestamp key. We could also maybe just append See here for syntax
with a link to the field name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, gotcha. Good point. How about this:
- Let's update the docs hierarchy to something like this:
- Working with structured logs
- Specifying fields
- Configuring authoritative fields
- Formatting as plain text
- Syntax
- Formatters
- Working with structured logs
- On the "Specifying fields" page, we can move the field-name syntax docs here.
- On the "Configuring authoritative fields", we can have a brief explanation about what the log-level key and timestamp are used for and link to the specifying fields docs for syntax.
- Let's update the help text in the settings dialog:
[JSON]
->[Structured]
(unless that's wrong?)- Format string for formatting a structured log event as plain text. Leave blank to display the entire log event. See here for syntax.
- Field containing each log event's level. See here for syntax.
- Field containing each log event's timestamp. See here for syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working with structured logs
Specifying fields
Configuring authoritative fields
Formatting as plain text
- Syntax
- Formatters
I feel like it might be better to keep all the format string syntax together (vs, separating), which might be more convenient if user is trying to write a format string. How do you feel about adding a new sub header "key syntax" under existing "formatter-name (optional)", then have the log level and timestamp key link directly to that?
If you oppose this, I would prefer to put "specifying fields" last, and just have the others link to it.
- On the "Specifying fields" page, we can move the field-name syntax docs here.
we wouldnt have to do this now. we would move some of it into "key syntax"
- On the "Configuring authoritative fields", we can have a brief explanation about what the log-level key and timestamp are used for and link to the specifying fields docs for syntax.
We wouldn't have to do this now, it would still be in log filtering docs. Note the timestamp key is kind of useless atm.
Let's update the help text in the settings dialog:
[JSON]
->[Structured]
(unless that's wrong?)
I thought of this but left it since I thought we preferred JSON to structured for users? Even the context of IRV2?
- Format string for formatting a structured log event as plain text. Leave blank to display the entire log event. See here for syntax.
- Field containing each log event's level. See here for syntax.
- Field containing each log event's timestamp. See here for syntax.
I'm good with this.
Anyways let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it might be better to keep all the format string syntax together (vs, separating), which might be more convenient if user is trying to write a format string. How do you feel about adding a new sub header "key syntax" under existing "formatter-name (optional)", then have the log level and timestamp key link directly to that?
That's a fair point. How about we do both by just including the content in both places? We should be able to use the include directive to do so.
If you oppose this, I would prefer to put "specifying fields" last, and just have the others link to it.
Sure, we can put it last.
I thought of this but left it since I thought we preferred JSON to structured for users? Even the context of IRV2?
Generally, yes, but since we support reading both JSON and IRv2 into the log viewer, I think we should try to distinguish the two to avoid confusing users in other ways.
Co-authored-by: kirkrodrigues <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/src/user-guide/format-struct-logs-syntax.md (4)
32-38
: Update to Escapable Characters List
Adding@
to the list of characters that must be escaped is appropriate. Consider including a brief note clarifying when the@
should be escaped (for example, when it is part of a literal field name rather than serving as an auto-generation prefix).
89-91
: Specify Language for Format String Code Block
For better readability and to satisfy markdown linting (MD040), please add a language identifier to this fenced code block. If this is a plain text format string, usingtext
would be suitable.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
89-89: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
129-132
: Add Language Identifier for Kv-Pair Format String
The fenced code block showing the format string{@ts:timestamp} {message} {ts:timestamp}
currently lacks a language specifier. Adding one (e.g.,text
) will improve clarity and ensure consistency with markdown best practices.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
129-129: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
139-141
: Specify Language for Output Code Block
For the code block displaying the formatted log output, please consider adding a language identifier (such astext
) to enhance readability and adhere to markdown guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
139-139: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/user-guide/format-struct-logs-syntax.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/format-struct-logs-syntax.md
89-89: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
139-139: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (2)
docs/src/user-guide/format-struct-logs-syntax.md (2)
27-30
: Auto-Generated Fields Explanation is Solid
The new lines clearly explain that auto-generated fields in a Key-Value Pair IR Stream should be prefixed with@
(e.g.,@ts
). This reiterates earlier suggestions and improves clarity.
105-113
: New Kv-Pair IR Log Events Example Enhances Clarity
The added section on formatting kv-pair IR log events effectively distinguishes between auto-generated and user-generated fields. The accompanying note explaining that theauto-generated
key is illustrative (as it does not exist in the actual log event) further aids user understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/src/user-guide/format-struct-logs-syntax.md (4)
33-33
: Clarify the Role of@
in Field Names:
Including@
in the escape list may be confusing since it is also used to denote auto-generated fields. Consider adding a brief note or example that clarifies when the@
should be escaped versus when it is interpreted as an auto-generated field prefix.
90-92
: Add Language Specifier in Code Block:
The fenced code block displaying the YScope format string does not specify a language. For improved readability and to comply with markdownlint recommendations (MD040), please add a language identifier (e.g.,text
).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
90-90: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
128-132
: Add Language Specifier for Format String Block:
The fenced code block containing the YScope format string for kv-pair IR log events is missing a language specifier. Including one (e.g.,text
) will enhance syntax highlighting and maintain consistency with best practices.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
130-130: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
140-142
: Add Language Specifier for Final Output Block:
The code block displaying the formatted string output lacks a language identifier. Adding one (e.g.,text
) would improve clarity and adhere to markdownlint guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
140-140: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/user-guide/format-struct-logs-syntax.md
(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/format-struct-logs-syntax.md
90-90: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
130-130: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (4)
docs/src/user-guide/format-struct-logs-syntax.md (4)
27-29
: Clarify Auto-Generated Fields Usage:
The updated lines now clearly indicate that auto-generated fields in a Key-Value Pair IR Stream can be specified using the@
prefix (e.g.,@ts
). This addition improves the clarity of the document.
49-50
: Reference for Supported Formatters:
The new reference to the list of supported formatters is a useful addition. Please verify that the link to[Formatters](format-struct-logs-formatters)
correctly navigates to the intended section.
106-114
: New Section for kv-pair IR Log Events:
The introduction of the "Formatting kv-pair IR log events" section is well structured. The accompanying note clearly explains that auto-generated kv-pairs are rendered separately from user-generated ones, which should help readers understand the formatting distinction.
144-145
: Ensure Reference Link Accuracy:
The reference for[kvPairIr]
has been appended at the end, which is excellent. Please confirm that the URL points to the correct resource regarding the design of the Key-Value Pair IR Stream.
Co-authored-by: kirkrodrigues <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/src/user-guide/format-struct-logs-syntax.md (3)
90-92
: Specify Language on Fenced Code Block:
The fenced code block displaying the YScope format string does not specify a language. Adding a language identifier (for example,text
ornone
) would improve syntax highlighting and comply with markdownlint guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
90-90: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
130-132
: Specify Language on Fenced Code Block for Format String:
The fenced code block showing the YScope format string for kv-pair IR log events currently lacks a language specifier. Adding one (e.g., ```text) would enhance readability and help satisfy markdownlint regulations.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
130-130: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
140-142
: Specify Language on Fenced Code Block for Output Example:
The fenced code block displaying the formatted output example does not include a language identifier. Adding a language specifier here (for instance, ```text) will improve clarity and adhere to markdown best practices.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
140-140: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/user-guide/format-struct-logs-syntax.md
(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/format-struct-logs-syntax.md
90-90: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
130-130: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (4)
docs/src/user-guide/format-struct-logs-syntax.md (4)
27-29
: Clarify Auto-generated Field Prefix Usage:
The added lines clearly state that auto-generated fields in a Key-Value Pair IR Stream can be specified with an@
prefix. Please ensure that the documentation further clarifies how this convention interacts with the escaping rules for field names—specifically, when the@
character in a field name must be escaped versus when it is used as a prefix for auto-generated fields.
49-50
: Link to Supported Formatters:
The addition of a hyperlink to the list of currently supported formatters is a valuable improvement. Please verify that the linked page remains up-to-date as new formatters are added in the future.
71-73
: New Examples Section for JSON Log Events:
The introduction of the “Formatting JSON logs events” section with a practical example is clear and useful. This addition should help users better understand the format string syntax in real-world scenarios.
106-107
: Introduction of KV-Pair IR Log Events Section:
The new section on formatting kv-pair IR log events is well-integrated and provides clear distinctions between auto-generated and user-generated keys. Consider adding a brief clarification about how the format string handles these two namespaces if further detail is needed.
* Field names can contain any character, except the following characters must be escaped with a | ||
backslash: | ||
* `.` | ||
* `$` | ||
* `@` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Review Escape Rules for the @
Character:
The listing for characters that must be escaped now includes @
. It would be helpful to clarify in the documentation that the @
used as a prefix for auto-generated fields is exempt from this escaping requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/src/user-guide/struct-logs/specifying-keys.md (1)
7-7
: Refine Preposition UsageThe sentence “Both options are found the settings dialog ({fas}
gear
).” appears to be missing a preposition. Consider updating it to “Both options are found in the settings dialog ({fas}gear
).”🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: Possible missing preposition found.
Context: ...l and timestamp Both options are found the settings dialog ({fas}gear
). See belo...(AI_HYDRA_LEO_MISSING_IN)
docs/src/user-guide/struct-logs/format/syntax.md (2)
25-27
: Validate Include DirectiveThe include directive for
../key-syntax.md
is appropriately used to detail nested and auto-generated keys. Please ensure that the relative path remains correct within the current documentation structure.
84-90
: Clarify Example ExplanationThe descriptive text for the formatting example is detailed and assists in understanding how placeholders work. Consider verifying that the terminology is consistent—e.g. using “field key” instead of “field name” if that reflects the updated conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/src/user-guide/log-level-filtering.md
(2 hunks)docs/src/user-guide/log-level-filtering.md
(1 hunks)docs/src/user-guide/log-level-filtering.md
(1 hunks)docs/src/user-guide/index.md
(2 hunks)docs/src/user-guide/log-level-filtering.md
(0 hunks)docs/src/user-guide/struct-logs/format/index.md
(2 hunks)docs/src/user-guide/struct-logs/format/syntax.md
(4 hunks)docs/src/user-guide/struct-logs/index.md
(1 hunks)docs/src/user-guide/struct-logs/key-syntax.md
(1 hunks)docs/src/user-guide/struct-logs/specifying-keys.md
(1 hunks)src/components/modals/SettingsModal/SettingsDialog.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/src/user-guide/struct-logs/index.md
- docs/src/user-guide/struct-logs/key-syntax.md
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/src/user-guide/log-level-filtering.md
- docs/src/user-guide/log-level-filtering.md
- docs/src/user-guide/log-level-filtering.md
- docs/src/user-guide/log-level-filtering.md
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
: - Preferfalse == <expression>
rather than!<expression>
.
src/components/modals/SettingsModal/SettingsDialog.tsx
🪛 LanguageTool
docs/src/user-guide/struct-logs/specifying-keys.md
[uncategorized] ~7-~7: Possible missing preposition found.
Context: ...l and timestamp Both options are found the settings dialog ({fas}gear
). See belo...
(AI_HYDRA_LEO_MISSING_IN)
🔇 Additional comments (13)
src/components/modals/SettingsModal/SettingsDialog.tsx (3)
46-58
: Documentation update looks good!Good update changing "[JSON]" to "[Structured]" for more inclusive terminology. The updated documentation link is more specific and helps users find the right information more easily.
67-82
: Documentation update is consistent and helpfulThe change from "[JSON]" to "[Structured]" maintains consistency with other help text changes. The updated link to the specific documentation about specifying keys will be more helpful for users than the previous general documentation link.
89-104
: Documentation update is appropriateThis change continues the consistent terminology update from "[JSON]" to "[Structured]" and provides the same specific documentation link as the log level key help text. This consistency in documentation references will help users better understand how to work with structured logs.
docs/src/user-guide/struct-logs/specifying-keys.md (2)
1-6
: Overall Section ClarityThe new "Specifying keys" section is clearly written and effectively introduces the concept of specifying keys when viewing structured logs.
9-11
: Include Directive for Key SyntaxThe inclusion of the
key-syntax.md
file is correctly implemented to offer additional details on key syntax.docs/src/user-guide/struct-logs/format/index.md (3)
1-6
: Revised Title and Introductory TextThe updated section title "Formatting as plain text" and the accompanying introductory content clearly describe how the log viewer uses a format string for structured logs.
36-38
: Updated Reference LinksThe reference links have been updated to
[Format string syntax](syntax)
and[Formatters](formatters)
, which aligns well with the new documentation structure. Please verify that these links resolve correctly within the new framework.
40-46
: New Table of Contents SectionThe added toctree for "Cluster setup" efficiently organises related components. The hidden configuration and caption help users navigate the documentation seamlessly.
docs/src/user-guide/struct-logs/format/syntax.md (3)
19-19
: Clarify Placeholder SyntaxThe newly documented placeholder syntax
{<field-key>[:<formatter-name>[:<formatter-options>]]}
is clearly specified. This update enhances clarity regarding how field placeholders should be structured.
39-40
: Updated Formatter LinkThe link
[Formatters](formatters)
is concise and should direct users effectively. Confirm that the target document exists and is accessible.
124-128
: Explanation for Auto-Generated FieldsThe explanation regarding the use of the
@
prefix for auto-generated fields is clear and informative.docs/src/user-guide/index.md (2)
17-21
: Updated Structured Logs CardThe grid item card for structured logs now accurately reflects the new focus on "Working with structured logs" with a clear and appropriate description.
40-44
: Updated Toctree CaptionChanging the toctree caption to "Working with structured logs" improves consistency throughout the user guide. The updated glob pattern also correctly references the new location.
Made changes discussed in here / slack. Not exactly sure how the docs actually update on yscope.com. As a result, the links in this PR may be temporarily broken until the site is actually updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a slightly flatter layout (mainly for the benefit of the left-panel index)?
Working with structured logs
- Overview
- Formatting as plain text
- Format string syntax
- Formatters
- Specifying keys
The extra hierarchy through "Structured log guides" seems a bit extra. Can we also rename "Structured log guides" to "Overview"?
In a prior comment, I made some suggestions about help text modifications for the Settings dialog, but I guess you may have missed them; I made them again so if you disagree with them, you can let me know.
Co-authored-by: kirkrodrigues <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/src/user-guide/struct-logs/format/syntax.md (3)
79-81
: Fenced Code Block Language Specification:
Consider specifying a language identifier for this fenced code block (for example, using```text
) to enhance readability and adhere to markdownlint guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
79-79: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
119-121
: Fenced Code Block Language Specification:
Similarly, for consistency and improved clarity, add a language identifier to this fenced code block.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
119-119: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
129-131
: Consistent Code Block Annotation:
Adding a language specifier for this final formatted output code block will help maintain consistency with best practices and improve readability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
129-129: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/src/user-guide/index.md
(2 hunks)docs/src/user-guide/struct-logs/format/index.md
(2 hunks)docs/src/user-guide/struct-logs/format/syntax.md
(4 hunks)docs/src/user-guide/struct-logs/key-syntax.md
(1 hunks)docs/src/user-guide/struct-logs/specifying-keys.md
(1 hunks)src/components/modals/SettingsModal/SettingsDialog.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/modals/SettingsModal/SettingsDialog.tsx
- docs/src/user-guide/struct-logs/specifying-keys.md
- docs/src/user-guide/struct-logs/key-syntax.md
- docs/src/user-guide/index.md
- docs/src/user-guide/struct-logs/format/index.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/struct-logs/format/syntax.md
79-79: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
119-119: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (6)
docs/src/user-guide/struct-logs/format/syntax.md (6)
19-21
: Field Placeholder Syntax Update:
The documentation now employs<field-key>
instead of<field-name>
to reflect the updated terminology. This change enhances clarity and consistency across the documentation.
22-24
: Updated Section Header:
The section header now reads "### field-key (required)", which aligns perfectly with the new naming conventions.
25-26
: Modular Documentation via Include Directive:
The inclusion of../key-syntax.md
helps reduce duplication by reusing existing content on key syntax. Please verify that the relative path is correct relative to this file.
38-38
: Formatter Link Update:
The reference now directs users to Formatters for a list of supported formatters. Ensure that this link resolves correctly in the final rendered documentation.
61-64
: Clear JSON Log Formatting Example:
The new example for formatting JSON log events is well presented—it demonstrates the use of date formatting and field substitution neatly.
95-102
: Enhanced KV-Pair Log Example:
The new section on formatting kv-pair IR log events effectively illustrates the handling of auto-generated keys (with the@
prefix) alongside user-generated keys. The accompanying note offers useful context regarding key namespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/src/user-guide/struct-logs/index.md (1)
1-4
: Overview Section ClarityThe “Overview” header and the short introductory text clearly state the purpose of the guides for working with structured logs. Consider briefly mentioning that these guides also cover aspects of auto-generated and hierarchical keys (per the PR objectives) to help users immediately understand the extended scope.
docs/src/user-guide/struct-logs/format/syntax.md (2)
19-20
: Specify Language for Fenced Code BlockThe code snippet showing the format string placeholder syntax is helpful, but the fenced code block does not have a language identifier. Adding one (e.g., using ```text) would improve readability and comply with markdown linting rules.
79-81
: Fenced Code Block Language IdentifiersSeveral fenced code blocks (the format string examples starting at lines 79, 119, and the output block starting at line 129) lack a language specifier. Adding a language identifier (for example, using
text
for plain text examples) would enhance readability and address markdown lint issues (MD040).Also applies to: 119-122, 129-132
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
79-79: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/src/user-guide/index.md
(2 hunks)docs/src/user-guide/struct-logs/format/syntax.md
(4 hunks)docs/src/user-guide/struct-logs/index.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/src/user-guide/index.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/struct-logs/format/syntax.md
79-79: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
119-119: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
129-129: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (3)
docs/src/user-guide/struct-logs/index.md (1)
5-21
: Grid Layout Structure and Link VerificationThe grid layout with two cards is visually clear and well structured. Please verify that the link targets (
format/index
andspecifying-keys
) are correct relative paths and that the corresponding pages exist in the documentation. Also, ensure that the grid container syntax renders as expected on all supported devices.docs/src/user-guide/struct-logs/format/syntax.md (2)
25-26
: Include Directive for Key SyntaxThe inclusion of the external document via the directive (
:::{include} ../key-syntax.md
) is a good way to keep the documentation DRY. Please verify that the included file exists at the specified relative path and that its content is up to date.
38-39
: Link to Supported FormattersThe line “For a list of currently supported formatters, see Formatters.” is a useful reference. Confirm that the target of the link (
formatters
) correctly routes to the updated information and that it reflects the current formatter support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/modals/SettingsModal/SettingsDialog.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ...
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
: - Preferfalse == <expression>
rather than!<expression>
.
src/components/modals/SettingsModal/SettingsDialog.tsx
🪛 GitHub Check: lint-check
src/components/modals/SettingsModal/SettingsDialog.tsx
[failure] 91-91:
'
can be escaped with '
, ‘
, '
, ’
[failure] 69-69:
'
can be escaped with '
, ‘
, '
, ’
🪛 ESLint
src/components/modals/SettingsModal/SettingsDialog.tsx
[error] 69-69: '
can be escaped with '
, ‘
, '
, ’
.
(react/no-unescaped-entities)
[error] 91-91: '
can be escaped with '
, ‘
, '
, ’
.
(react/no-unescaped-entities)
🪛 GitHub Actions: lint
src/components/modals/SettingsModal/SettingsDialog.tsx
[error] 69-69: 'can be escaped with
',
‘,
',
’` react/no-unescaped-entities
🔇 Additional comments (2)
src/components/modals/SettingsModal/SettingsDialog.tsx (2)
46-58
: Helper text updated appropriatelyThe helper text has been updated to use "structured log event" terminology and includes a link to the relevant documentation. The URL has been properly updated to point to the specific documentation for structured logs formatting.
46-104
:✅ Verification successful
Ensure documentation links work
The PR objectives mention uncertainty about how documentation updates are reflected on the yscope.com website, with possible temporarily broken links. Please verify that these documentation links will resolve correctly when the PR is merged.
🌐 Web query:
Are the URLs https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html and https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html accessible?
💡 Result:
Based on the provided search results, the two URLs related to the YScope Log Viewer documentation appear to be part of its structured content hierarchy. While the search results don't explicitly list these exact URLs, they confirm:
Documentation Structure: The YScope Log Viewer documentation uses paths like
/yscope-log-viewer/main/user-guide/
for organization [8]. This matches the base path in both queried URLs.Content Validation:
- The
/struct-logs/format/
section would logically correspond to CLP's Intermediate Representation (IR) format documentation mentioned in multiple sources [4][10][13]- The
/struct-logs/specifying-keys/
path aligns with CLP's key-value pair logging capabilities described in the Python logging library documentation [10][13]Accessibility Confirmation:
- The documentation hub explicitly states: "Use the left sidebar... to navigate to specific docs" [8]
- Search results show working documentation patterns like
/user-guide/index
[8] and/main/
paths [4], confirming the URL structure is validWithout explicit restrictions in the search results and given YScope's open documentation approach [4], these URLs should be accessible as part of their standard documentation. For direct verification, you would need to visit the links, but the evidence suggests they follow YScope's established documentation patterns.
Citations:
- 1: https://log-viewer.opcodes.io/docs/3.x/configuration/access-to-log-viewer
- 2: https://www.youtube.com/watch?v=23iHoDeO7M4
- 3: https://experienceleague.adobe.com/en/docs/experience-manager-65/content/managing/managing-further-reference/seo-and-url-management
- 4: https://docs.yscope.com
- 5: https://www.usenix.org/conference/osdi24/presentation/wang-rui
- 6: https://www.cisco.com/c/en/us/td/docs/security/wsa/wsa-14-5/user-guide/wsa-userguide-14-5/b_WSA_UserGuide_11_7_chapter_010101.html
- 7: https://cdn.logic-control.com/media/LogViewer.pdf
- 8: https://docs.yscope.com/yscope-log-viewer/main/user-guide/index
- 9: https://cloud.google.com/release-notes
- 10: https://github.com/y-scope/clp-loglib-py/blob/main/README.md
- 11: https://scholar.google.ca/citations?user=fe9Sy-cAAAAJ&hl=en
- 12: https://docs.olcf.ornl.gov/systems/frontier_user_guide.html
- 13: https://github.com/y-scope/clp-loglib-py
- 14: https://www.w3.org/TR/WCAG21/
- 15: https://www.usenix.org/system/files/osdi21-rodrigues.pdf
- 16: https://www.era.nih.gov/docs/commons_userguide.pdf
- 17: https://docs.yscope.com/yscope-log-viewer/main/
- 18: https://www.servicenow.com/docs/bundle/yokohama-api-reference/page/integrate/inbound-rest/concept/c_AttachmentAPI.html
- 19: https://nvlpubs.nist.gov/nistpubs/legacy/sp/nistspecialpublication800-122.pdf
Documentation links successfully verified
After reviewing the documentation URLs, both links appear to follow YScope's established structured documentation pattern and should be accessible once merged. Please ensure they are tested in the production/staging environment to confirm no temporary accessibility issues arise post-deployment.
- Verify that:
•https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html
•https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html
return the expected content.🧰 Tools
🪛 GitHub Check: lint-check
[failure] 91-91:
'
can be escaped with'
,‘
,'
,’
[failure] 69-69:
'
can be escaped with'
,‘
,'
,’
🪛 ESLint
[error] 69-69:
'
can be escaped with'
,‘
,'
,’
.(react/no-unescaped-entities)
[error] 91-91:
'
can be escaped with'
,‘
,'
,’
.(react/no-unescaped-entities)
🪛 GitHub Actions: lint
[error] 69-69: '
can be escaped with
',
‘,
',
’` react/no-unescaped-entities
helperText: ( | ||
<span> | ||
[Structured] Key that maps to each log event's log level. See | ||
{" "} | ||
<Link | ||
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html"} | ||
level={"body-sm"} | ||
rel={"noopener"} | ||
target={"_blank"} | ||
> | ||
here | ||
</Link> | ||
{" "} | ||
for syntax. | ||
</span> | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix unescaped apostrophe
The helper text content looks good, but there's an unescaped apostrophe in "event's" that's causing the build to fail. This needs to be escaped according to React guidelines.
- [Structured] Key that maps to each log event's log level. See
+ [Structured] Key that maps to each log event's log level. See
📝 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.
helperText: ( | |
<span> | |
[Structured] Key that maps to each log event's log level. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
here | |
</Link> | |
{" "} | |
for syntax. | |
</span> | |
), | |
helperText: ( | |
<span> | |
[Structured] Key that maps to each log event's log level. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
here | |
</Link> | |
{" "} | |
for syntax. | |
</span> | |
), |
🧰 Tools
🪛 GitHub Check: lint-check
[failure] 69-69:
'
can be escaped with '
, ‘
, '
, ’
🪛 ESLint
[error] 69-69: '
can be escaped with '
, ‘
, '
, ’
.
(react/no-unescaped-entities)
🪛 GitHub Actions: lint
[error] 69-69: 'can be escaped with
',
‘,
',
’` react/no-unescaped-entities
helperText: ( | ||
<span> | ||
[Structured] Key that maps to each log event's timestamp. See | ||
{" "} | ||
<Link | ||
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html"} | ||
level={"body-sm"} | ||
rel={"noopener"} | ||
target={"_blank"} | ||
> | ||
here | ||
</Link> | ||
{" "} | ||
for syntax. | ||
</span> | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix unescaped apostrophe
Similar to the previous comment, there's an unescaped apostrophe in "event's" that needs to be escaped for the component to render properly.
- [Structured] Key that maps to each log event's timestamp. See
+ [Structured] Key that maps to each log event's timestamp. See
📝 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.
helperText: ( | |
<span> | |
[Structured] Key that maps to each log event's timestamp. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
here | |
</Link> | |
{" "} | |
for syntax. | |
</span> | |
), | |
helperText: ( | |
<span> | |
[Structured] Key that maps to each log event's timestamp. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
here | |
</Link> | |
{" "} | |
for syntax. | |
</span> | |
), |
🧰 Tools
🪛 GitHub Check: lint-check
[failure] 91-91:
'
can be escaped with '
, ‘
, '
, ’
🪛 ESLint
[error] 91-91: '
can be escaped with '
, ‘
, '
, ’
.
(react/no-unescaped-entities)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more suggestions.
For the PR title, how about:
docs: Explain how to work with auto-generated keys; Restructure to clarify how to specify keys outside the format string.
Co-authored-by: kirkrodrigues <[email protected]>
re: title |
PR looks good, but let's hold on merging until we open-source the kv-ir docs in the clp repo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also resolve the conflicts?
Co-authored-by: kirkrodrigues <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/src/user-guide/struct-logs/key-syntax.md (1)
14-14
: Duplicate: Updated[kv-pair-ir]
link anchor
This matches an earlier suggestion from kirkrodrigues to point the anchor to the auto-generated KV pairs documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/user-guide/struct-logs/key-syntax.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/src/user-guide/struct-logs/key-syntax.md
1-1: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
@@ -0,0 +1,14 @@ | |||
* Nested keys can be specified using periods (`.`) to denote hierarchy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Add a top-level heading for consistency with Markdown style
As per markdownlint MD041, the first line should be a top-level heading. Consider adding:
# Key Syntax
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
* Auto-generated keys in a [Key-Value Pair IR Stream][kv-pair-ir] can be specified by using `@` as | ||
a prefix. | ||
* E.g., the auto-generated key `ts` would be specified as `@ts`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Simplify the auto-generated keys bullet for clarity
Combine the wrapped lines into one concise sentence:
-* Auto-generated keys in a [Key-Value Pair IR Stream][kv-pair-ir] can be specified by using `@` as
- a prefix.
+* Auto-generated keys in a [Key-Value Pair IR Stream][kv-pair-ir] can be specified by prefixing them with `@`.
📝 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.
* Auto-generated keys in a [Key-Value Pair IR Stream][kv-pair-ir] can be specified by using `@` as | |
a prefix. | |
* E.g., the auto-generated key `ts` would be specified as `@ts`. | |
* Auto-generated keys in a [Key-Value Pair IR Stream][kv-pair-ir] can be specified by prefixing them with `@`. | |
* E.g., the auto-generated key `ts` would be specified as `@ts`. |
* Nested keys can be specified using periods (`.`) to denote hierarchy. | ||
* E.g., the field `{"a:" {"b": 0}}` may be denoted by `a.b`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the JSON example syntax
The inline JSON in the example is missing a closing quote after the key. Update it to:
- * E.g., the field `{"a:" {"b": 0}}` may be denoted by `a.b`.
+ * E.g., the field `{"a": {"b": 0}}` may be denoted by `a.b`.
📝 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.
* Nested keys can be specified using periods (`.`) to denote hierarchy. | |
* E.g., the field `{"a:" {"b": 0}}` may be denoted by `a.b`. | |
* Nested keys can be specified using periods (`.`) to denote hierarchy. | |
* E.g., the field `{"a": {"b": 0}}` may be denoted by `a.b`. |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
* Keys can contain any character, except the following characters must be escaped with a backslash: | ||
* `.` | ||
* `@` | ||
* `{` | ||
* `}` | ||
* `:` | ||
* `\` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Improve wording and escape sequence formatting
- Rephrase the sentence for better readability:
-* Keys can contain any character, except the following characters must be escaped with a backslash: +* Keys can contain any character; however, the following characters must be escaped with a backslash:
- Ensure the backslash itself is displayed correctly in Markdown by escaping it:
-* `\` +* `\\`
📝 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.
* Keys can contain any character, except the following characters must be escaped with a backslash: | |
* `.` | |
* `@` | |
* `{` | |
* `}` | |
* `:` | |
* `\` | |
* Keys can contain any character; however, the following characters must be escaped with a backslash: | |
* `.` | |
* `@` | |
* `{` | |
* `}` | |
* `:` | |
* `\\` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SettingsTabPanel/index.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
: - Preferfalse == <expression>
rather than!<expression>
.
src/components/CentralContainer/Sidebar/SidebarTabs/SettingsTabPanel/index.tsx
🧬 Code Graph Analysis (1)
src/components/CentralContainer/Sidebar/SidebarTabs/SettingsTabPanel/index.tsx (2)
src/utils/config.ts (1)
getConfig
(182-182)src/typings/config.ts (2)
CONFIG_KEY
(44-44)LOCAL_STORAGE_KEY
(45-45)
helperText: ( | ||
<span> | ||
[Structured] Key that maps to each log event's log level. See | ||
{" "} | ||
<Link | ||
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"} | ||
level={"body-sm"} | ||
rel={"noopener"} | ||
target={"_blank"} | ||
> | ||
here | ||
</Link> | ||
{" "} | ||
for syntax. | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Enhance link clarity for log level key syntax.
Similarly, the link text “here” in the log level key helper text is not self-descriptive. Update it to a phrase that reflects its target, for example:
- <Link
- href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"}
- level={"body-sm"}
- rel={"noopener"}
- target={"_blank"}
- >
- here
- </Link>
+ <Link
+ href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"}
+ level={"body-sm"}
+ rel={"noopener"}
+ target={"_blank"}
+ >
+ structured logs key syntax guide
+ </Link>
[Structured] Format string for formatting a structured log event as plain text. | ||
Leave blank to display the entire log event. See | ||
{" "} | ||
<Link | ||
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/format-struct-logs-overview.html"} | ||
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html"} | ||
level={"body-sm"} | ||
rel={"noopener"} | ||
target={"_blank"} | ||
> | ||
format string syntax docs | ||
here | ||
</Link> | ||
{" "} | ||
or leave this blank to display the entire log event. | ||
for syntax. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Descriptive link text for accessibility.
The link text “here” is too generic and may not be clear for assistive technologies. Consider using more descriptive text, for example:
- <Link
- href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html"}
- level={"body-sm"}
- rel={"noopener"}
- target={"_blank"}
- >
- here
- </Link>
+ <Link
+ href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html"}
+ level={"body-sm"}
+ rel={"noopener"}
+ target={"_blank"}
+ >
+ structured logs formatting guide
+ </Link>
📝 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.
[Structured] Format string for formatting a structured log event as plain text. | |
Leave blank to display the entire log event. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/format-struct-logs-overview.html"} | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
format string syntax docs | |
here | |
</Link> | |
{" "} | |
or leave this blank to display the entire log event. | |
for syntax. | |
[Structured] Format string for formatting a structured log event as plain text. | |
Leave blank to display the entire log event. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/format/index.html"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
structured logs formatting guide | |
</Link> | |
{" "} | |
for syntax. |
helperText: ( | ||
<span> | ||
[Structured] Key that maps to each log event's timestamp. See | ||
{" "} | ||
<Link | ||
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"} | ||
level={"body-sm"} | ||
rel={"noopener"} | ||
target={"_blank"} | ||
> | ||
here | ||
</Link> | ||
{" "} | ||
for syntax. | ||
</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Use meaningful link text for timestamp key syntax.
The helper text for the timestamp key also uses “here”, which offers no context. Consider a descriptive label:
- <Link
- href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"}
- level={"body-sm"}
- rel={"noopener"}
- target={"_blank"}
- >
- here
- </Link>
+ <Link
+ href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"}
+ level={"body-sm"}
+ rel={"noopener"}
+ target={"_blank"}
+ >
+ structured logs key syntax guide
+ </Link>
📝 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.
helperText: ( | |
<span> | |
[Structured] Key that maps to each log event's timestamp. See | |
{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
here | |
</Link> | |
{" "} | |
for syntax. | |
</span> | |
helperText: ( | |
<span> | |
[Structured] Key that maps to each log event's timestamp. See{" "} | |
<Link | |
href={"https://docs.yscope.com/yscope-log-viewer/main/user-guide/struct-logs/specifying-keys.html#syntax"} | |
level={"body-sm"} | |
rel={"noopener"} | |
target={"_blank"} | |
> | |
structured logs key syntax guide | |
</Link>{" "} | |
for syntax. | |
</span> | |
) |
Description
Updates the docs to explain how to use auto-generated and hierarchal keys.
I decided to add a new example instead of beefing up old example with autogenerated keys, as the I thought the current example is already quite complex.
Checklist
breaking change.
Validation performed
Ran docs. Tested new example format string
Summary by CodeRabbit
Documentation
Style