Skip to content

docs(kv-ir): Explain support for auto-generated KV pairs.#888

Merged
kirkrodrigues merged 17 commits into
y-scope:mainfrom
kirkrodrigues:kv-ir-docs-auto-gen-kv-pairs
May 14, 2025
Merged

docs(kv-ir): Explain support for auto-generated KV pairs.#888
kirkrodrigues merged 17 commits into
y-scope:mainfrom
kirkrodrigues:kv-ir-docs-auto-gen-kv-pairs

Conversation

@kirkrodrigues

@kirkrodrigues kirkrodrigues commented May 8, 2025

Copy link
Copy Markdown
Member

Description

As the title says. Future PRs will flesh out the format itself.

This continues the work started in #707 and #833.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Workflows succeed.
  • New pages render as expected.

Summary by CodeRabbit

  • Documentation
    • Introduced a new guide explaining the distinction between auto-generated and user-generated key-value pairs in KV-IR streams, including query syntax and examples.
    • Updated background documentation for clearer references and direct linking to the new guide.
    • Added a navigation card and table of contents entry for the new section to improve discoverability.

@kirkrodrigues kirkrodrigues requested a review from a team as a code owner May 8, 2025 11:39
@coderabbitai

coderabbitai Bot commented May 8, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

A new documentation file was added to explain the distinction between auto-generated and user-generated key-value pairs in KV-IR streams, introducing a query syntax for these namespaces. Existing documentation was updated to reference this new section, and the documentation index was modified to include and link to the new content.

Changes

File(s) Change Summary
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md Added a new document explaining auto-generated vs. user-generated KV pairs, query syntax, and related concepts.
docs/src/dev-guide/design-kv-ir-streams/background.md Updated text to reference the new auto-generated KV pairs documentation with a specific link for clarity.
docs/src/dev-guide/design-kv-ir-streams/index.md Added a grid card and toctree entry for the new auto-generated KV pairs section in the documentation index.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant LoggingFramework
    participant LogEvent
    participant QueryEngine

    User->>LoggingFramework: Logs event with user-defined KV pairs
    LoggingFramework->>LogEvent: Inserts auto-generated KV pairs (e.g., timestamp, level)
    LogEvent->>QueryEngine: Stores both auto- and user-generated KV pairs
    User->>QueryEngine: Queries using syntax (e.g., @level, userKey)
    QueryEngine->>LogEvent: Filters based on namespaces (auto/user)
    LogEvent-->>User: Returns filtered log events
Loading

Tip

⚡️ Faster reviews with caching
  • CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.

Enjoy the performance boost—your workflow just got faster.


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3424d2a and 158a9c2.

📒 Files selected for processing (4)
  • docs/requirements.txt (1 hunks)
  • docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1 hunks)
  • docs/src/dev-guide/design-kv-ir-streams/background.md (1 hunks)
  • docs/src/dev-guide/design-kv-ir-streams/index.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/design-kv-ir-streams/index.md

[grammar] ~24-~24: Possible subject-verb agreement error.
Context: ...-pairs Auto-generated KV pairs ^^^ What are auto-generated KV pairs and how they're...

(WHAT_ARE_JJ)

docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

[typographical] ~72-~72: Conjunctions like ‘and’ should not follow semicolons. Consider using a comma, or removing the conjunction.
Context: ... set of keys for auto-generated KV pairs; and this makes it easier for users to filter for...

(CONJUNCTION_AFTER_SEMICOLON)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: lint-check (ubuntu-latest)
  • GitHub Check: lint-check (macos-latest)
🔇 Additional comments (10)
docs/requirements.txt (1)

8-9: Pinning snowballstemmer version to workaround Sphinx issue
Locking snowballstemmer to ~2.2 with a clarifying comment effectively addresses the known Sphinx bug [#13533].

docs/src/dev-guide/design-kv-ir-streams/index.md (2)

19-25: New grid-item-card for Auto-generated KV pairs
Adding a card linking to the auto-gen-kv-pairs section improves discoverability in the KV-IR streams guide.

🧰 Tools
🪛 LanguageTool

[grammar] ~24-~24: Possible subject-verb agreement error.
Context: ...-pairs Auto-generated KV pairs ^^^ What are auto-generated KV pairs and how they're...

(WHAT_ARE_JJ)


35-37: Include new doc in hidden toctree
The hidden toctree entry for auto-gen-kv-pairs correctly integrates the new section into navigation.

docs/src/dev-guide/design-kv-ir-streams/background.md (1)

199-201: Explicit link to auto-generated KV pairs doc
Updating the future-docs placeholder to directly reference [auto-generated KV pairs](auto-gen-kv-pairs.md) clarifies the namespace discussion.

docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (6)

1-2: Add introductory section for auto-generated KV pairs
The new top-level header and blank line correctly establish the section.


3-12: Explain auto- vs user-generated KV pairs clearly
This concise explanation and context comparing KV-IR with JSON lines is well articulated.


14-26: Add example Go LPS and JSON output
Including Figures 1 and 2 with code cards aids comprehension. The Go and JSON snippets are clear and correctly formatted.


45-49: Define KV pair categories
The definitions of auto-generated and user-generated KV pairs are precise and easy to understand.


54-68: Query syntax for auto-generated KV pairs
Detailing the @ prefix for namespace-specific filtering and escape requirements is essential and well explained.


75-83: Outline additional benefits
The sections on cross-language consistency and advanced querying possibilities are well described and motivate the feature effectively.

Comment thread docs/src/dev-guide/design-kv-ir-streams/index.md Outdated
Comment thread docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 158a9c2 and b123477.

📒 Files selected for processing (2)
  • docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1 hunks)
  • docs/src/dev-guide/design-kv-ir-streams/index.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

[uncategorized] ~14-~14: A punctuation mark might be missing here.
Context: ...onflict with a user's keys. (figure-1)= :::{card} ```go logger.Info("Task compl...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~61-~61: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the key `timesta...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: lint-check (ubuntu-latest)
  • GitHub Check: lint-check (macos-latest)
🔇 Additional comments (9)
docs/src/dev-guide/design-kv-ir-streams/index.md (2)

19-25: Great: Added navigation card for auto-generated KV pairs
This new grid-item-card aligns with the existing background card in style and clearly guides readers to the “Auto-generated KV pairs” section.


35-37: Hidden toctree updated
The auto-gen-kv-pairs entry has been correctly added to the hidden toctree, ensuring the new section is included in the documentation navigation.

docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (7)

1-2: Clear section title
The heading # Auto-generated KV pairs accurately describes the content that follows.


3-12: Introductory context is well-written
The opening paragraph succinctly contrasts KV-IR streams with JSON lines, setting up the motivation for separate namespaces.


14-27: Figure 1 card is well-formatted
The code snippet and caption clearly illustrate the structured LPS example using Zap.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~14-~14: A punctuation mark might be missing here.
Context: ...onflict with a user's keys. (figure-1)= :::{card} ```go logger.Info("Task compl...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


28-43: Figure 2 card is clear
The JSON output example complements Figure 1 and is formatted correctly with caption.


45-53: Definitions are precise
The distinction between auto-generated and user-generated KV pairs is clearly defined.


85-93: Note and references are properly formatted
The note directive and external link definitions are correctly specified.


59-67: 🧹 Nitpick (assertive)

Add missing article for clarity
Consider adjusting line 61 to include the article “a” before “user-generated KV pair” for grammatical correctness:

- Since KV-IR streams store auto-generated KV pairs and user-generated KV pairs in different
+ Since KV-IR streams store auto-generated KV pairs and a user-generated KV pair in different

Likely an incorrect or invalid review comment.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~61-~61: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the key `timesta...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

Comment thread docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b123477 and f11f740.

📒 Files selected for processing (1)
  • docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

[uncategorized] ~61-~61: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the key `timesta...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


7-7: Link fragments should be valid
null

(MD051, link-fragments)


7-7: Link fragments should be valid
null

(MD051, link-fragments)


42-42: Link fragments should be valid
null

(MD051, link-fragments)


50-50: Link fragments should be valid
null

(MD051, link-fragments)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: lint-check (macos-latest)
🔇 Additional comments (1)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1)

6-7: Suppress False-Positive Link Fragment Warnings:
The markdownlint MD051 errors for references like [Figure 1](#figure-1) and [Figure 2](#figure-2) stem from our custom MDX anchors ((figure-1)=, :::{card}) rather than standard Markdown. These links resolve correctly in the generated site—feel free to ignore or disable the MD051 rule here.

Also applies to: 42-42, 50-50

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


7-7: Link fragments should be valid
null

(MD051, link-fragments)


7-7: Link fragments should be valid
null

(MD051, link-fragments)


Since KV-IR streams store auto-generated KV pairs and user-generated KV pairs in different
namespaces, we differentiate which namespace to query using special query syntax. For instance, if a
log event has both an auto-generated KV pair and user-generated KV pair with the key `timestamp`, we

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Missing Article:
Add the article “a” before “user-generated KV pair” so the sentence reads:

“…both an auto-generated KV pair and a user-generated KV pair with the key timestamp…”

🧰 Tools
🪛 LanguageTool

[uncategorized] ~61-~61: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the key `timesta...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

@@ -0,0 +1,92 @@
# Auto-generated KV pairs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Title Capitalization:
Ensure consistency with other guide sections by using Title Case in the heading. For example, change:

-# Auto-generated KV pairs
+# Auto-generated KV Pairs
📝 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.

Suggested change
# Auto-generated KV pairs
# Auto-generated KV Pairs

@LinZhihao-723 LinZhihao-723 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall lgtm.
Left some comments for two paragraphs.

Comment on lines +8 to +12
logging library added the timestamp and level KV pairs automatically. (The key for the message,
timestamp, and log level are Zap configuration options.) Storing these two categories of KV pairs in
a format like JSON has the disadvantage that both categories share the same key namespace; so a user
needs to pick keys that don't conflict with the logging library's keys, and the logging library
needs to pick keys that are unlikely to conflict with a user's keys.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
logging library added the timestamp and level KV pairs automatically. (The key for the message,
timestamp, and log level are Zap configuration options.) Storing these two categories of KV pairs in
a format like JSON has the disadvantage that both categories share the same key namespace; so a user
needs to pick keys that don't conflict with the logging library's keys, and the logging library
needs to pick keys that are unlikely to conflict with a user's keys.
logging library added the timestamp and level KV pairs automatically.
Although the keys for the message, timestamp, and log level can be configured via Zap options,
storing these two categories of KV pairs in a format like JSON requires additional effort to prevent
keys conflicts, since both categories share the same key namespace:
* Users must avoid selecting keys that might conflict with those used by the logging framework—-this
may even require knowledge of all reserved keys.
* The logging library must choose keys that are unlikely to collide with user-defined keys, and
still implement a policy to resolve any conflicts that do occur.
  • Add a little more details to explain why this is really a headache and inevitable.
  • How about using users instead a user? Feel like it's easier to read.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you mean "A user"?

Comment thread docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md Outdated
One benefit of supporting auto-generated KV pairs is that our CLP logging libraries or plugins can
use a consistent set of keys for auto-generated KV pairs, making it easier for users to filter for
those keys. For instance, if a user is logging in both Java and Python, and they use CLP logging
plugins to generate KV-IR streams, the streams generated should both use the same key for log

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
plugins to generate KV-IR streams, the streams generated should both use the same key for log
plugins to generate KV-IR streams, the generated streams should both use the same key for log

Should sound more natural?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (2)

1-1: 🧹 Nitpick (assertive)

Title Case in Heading
Ensure consistency with other guide sections by capitalizing “Pairs” in the main heading:

-# Auto-generated KV pairs
+# Auto-generated KV Pairs

59-61: 🧹 Nitpick (assertive)

Missing Article Before “user-generated KV pair”
Add the article “a” before “user-generated KV pair” for grammatical consistency:

-If a log event has both an auto-generated KV pair and user-generated KV pair with the same key,
+If a log event has both an auto-generated KV pair and a user-generated KV pair with the same key,
🧰 Tools
🪛 LanguageTool

[uncategorized] ~59-~59: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the same key, e....

(AI_EN_LECTOR_MISSING_DETERMINER_A)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f11f740 and 1f7ff57.

📒 Files selected for processing (1)
  • docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

[uncategorized] ~14-~14: A punctuation mark might be missing here.
Context: ...onflict with a user's keys. (figure-1)= :::{card} ```go logger.Info("Task compl...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~59-~59: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the same key, e....

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


7-7: Link fragments should be valid
null

(MD051, link-fragments)


7-7: Link fragments should be valid
null

(MD051, link-fragments)


42-42: Link fragments should be valid
null

(MD051, link-fragments)


50-50: Link fragments should be valid
null

(MD051, link-fragments)

🔇 Additional comments (3)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (3)

17-22: Figure 1 Code Block
The Go example for the structured LPS is clear, correctly formatted, and uses the appropriate syntax highlighting.


31-39: Figure 2 Code Block
The JSON output block is well-formed, labelled with the json language tag, and accurately illustrates the example.


90-93: Custom Link Fragments
The [json-lines], [log-viewer], and [Zap] references rely on our MkDocs/AST configuration rather than standard Markdown anchors. Despite markdownlint warnings, these fragments are supported and render correctly—no change needed.

Comment on lines +85 to +88
:::{note}
The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a
set of key-less auto-generated and user-generated values, formatted into a single string.
:::

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Hyphenation in “key-less”
For consistency with common usage, consider using “keyless” without a hyphen:

-The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a set of key-less auto-generated and user-generated values...
+The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a set of keyless auto-generated and user-generated values...
📝 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.

Suggested change
:::{note}
The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a
set of key-less auto-generated and user-generated values, formatted into a single string.
:::
:::{note}
The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a
set of keyless auto-generated and user-generated values, formatted into a single string.
:::

Comment thread docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md Outdated
Comment on lines +71 to +73
One benefit of supporting auto-generated KV pairs is that our CLP logging libraries or plugins can
use a consistent set of keys for auto-generated KV pairs, making it easier for users to filter for
those keys. For instance, if a user is logging in both Java and Python, and they use CLP logging

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Reduce Redundancy in Phrase
The phrase “auto-generated KV pairs” appears twice in the same sentence. Consider tightening it for readability:

-One benefit of supporting auto-generated KV pairs is that our CLP logging libraries or plugins can use a consistent set of keys for auto-generated KV pairs, making it easier for users to filter for those keys.
+One benefit is that our CLP logging libraries or plugins can standardize keys for auto-generated pairs, making it easier for users to filter them.
📝 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.

Suggested change
One benefit of supporting auto-generated KV pairs is that our CLP logging libraries or plugins can
use a consistent set of keys for auto-generated KV pairs, making it easier for users to filter for
those keys. For instance, if a user is logging in both Java and Python, and they use CLP logging
One benefit is that our CLP logging libraries or plugins can standardize keys for auto-generated pairs, making it easier for users to filter them.
For instance, if a user is logging in both Java and Python, and they use CLP logging

@kirkrodrigues

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 12, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (3)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (3)

1-1: 🧹 Nitpick (assertive)

Title Capitalization Consistency
Ensure the main heading follows Title Case to match other guide sections. For example:

- # Auto-generated KV pairs
+ # Auto-generated KV Pairs

78-80: 🧹 Nitpick (assertive)

Reduce redundancy in phrasing
Revisit this sentence per earlier feedback to tighten it and remove repeated mentions of “KV pairs”. For example:

- One benefit of supporting auto-generated KV pairs is that our CLP logging libraries or plugins can use a consistent set of keys for these KV pairs, making it easier for users to filter for the pairs.
+ One benefit is that our CLP logging libraries or plugins can standardize keys for auto-generated pairs, making them easier to filter.

93-95: 🧹 Nitpick (assertive)

Consistent hyphenation of ‘keyless’
Replace “key-less” with “keyless” to align with common usage:

- The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a set of key-less auto-generated and user-generated values...
+ The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a set of keyless auto-generated and user-generated values...
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f7ff57 and 9a24dd9.

📒 Files selected for processing (1)
  • docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

[uncategorized] ~17-~17: A punctuation mark might be missing here.
Context: ...sert it into the log event. (figure-1)= :::{card} ```go logger.Info("Task compl...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~28-~28: Possible missing preposition found.
Context: ...2), ) +++ **Figure 1**: An example structured LPS. ::: (figure-2)= :::{card} jso...

(AI_HYDRA_LEO_MISSING_OF)


[uncategorized] ~67-~67: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the same key, e....

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

5-5: Link fragments should be valid
null

(MD051, link-fragments)


5-5: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


13-13: Link fragments should be valid
null

(MD051, link-fragments)


45-45: Link fragments should be valid
null

(MD051, link-fragments)

🔇 Additional comments (2)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (2)

17-29: Verify custom directive rendering
Confirm that the (figure-1)= anchor and the ::: {card} block render as intended in your documentation engine. The syntax for figure cards can differ between markdown processors.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~17-~17: A punctuation mark might be missing here.
Context: ...sert it into the log event. (figure-1)= :::{card} ```go logger.Info("Task compl...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~28-~28: Possible missing preposition found.
Context: ...2), ) +++ **Figure 1**: An example structured LPS. ::: (figure-2)= :::{card} jso...

(AI_HYDRA_LEO_MISSING_OF)


5-6: Ignore markdownlint MD051 false positives
The fragment links ([Figure 1](#figure-1), [Figure 2](#figure-2)) and reference-style links ([json-lines], [log-viewer], [Zap]) are correctly defined in this document. The MD051 warnings can be safely disregarded.

Also applies to: 97-99

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

5-5: Link fragments should be valid
null

(MD051, link-fragments)


5-5: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)

Comment on lines +48 to +51
One of the key differences between KV-IR streams and a format like [JSON lines][json-lines] (JSONL)
is that KV-IR streams support storing auto-generated KV pairs separately from user-generated KV
pairs. Since the JSONL format has no such support, the two categories are stored in the same key
namespace, which introduces the following constraints:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Clarify comparison to JSONL
The phrase “a format like [JSON lines][json-lines] (JSONL)” can be streamlined. For example:

- One of the key differences between KV-IR streams and a format like [JSON lines][json-lines] (JSONL) is...
+ One of the key differences between KV-IR streams and JSON Lines (JSONL) is...
📝 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.

Suggested change
One of the key differences between KV-IR streams and a format like [JSON lines][json-lines] (JSONL)
is that KV-IR streams support storing auto-generated KV pairs separately from user-generated KV
pairs. Since the JSONL format has no such support, the two categories are stored in the same key
namespace, which introduces the following constraints:
One of the key differences between KV-IR streams and JSON Lines (JSONL) is that KV-IR streams support storing auto-generated KV pairs separately from user-generated KV pairs. Since the JSONL format has no such support, the two categories are stored in the same key namespace, which introduces the following constraints:

Comment thread docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md
Comment on lines +3 to +8
Structured log events typically contain two categories of KV pairs---those generated by the user
(a developer) and those generated by the logging library. To illustrate, consider the structured
[Zap] log printing statement (LPS) in [Figure 1](#figure-1) and its output in [Figure 2](#figure-2).
Comparing the user-defined KV pairs in [Figure 1](#figure-1) with KV pairs in [Figure 2](#figure-2),
we can see that the logging library adds the `timestamp` and `level` KV pairs automatically. We
define the two categories of KV pairs as follows:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Improve readability with em dashes and sentence splitting
Replace triple hyphens --- with an em dash and consider breaking up the long first sentence. For example:

- Structured log events typically contain two categories of KV pairs---those generated by the user (a developer) and those generated by the logging library.
+ Structured log events typically contain two categories of KV pairs—those generated by the user (a developer) and those generated by the logging library.
📝 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.

Suggested change
Structured log events typically contain two categories of KV pairs---those generated by the user
(a developer) and those generated by the logging library. To illustrate, consider the structured
[Zap] log printing statement (LPS) in [Figure 1](#figure-1) and its output in [Figure 2](#figure-2).
Comparing the user-defined KV pairs in [Figure 1](#figure-1) with KV pairs in [Figure 2](#figure-2),
we can see that the logging library adds the `timestamp` and `level` KV pairs automatically. We
define the two categories of KV pairs as follows:
Structured log events typically contain two categories of KV pairsthose generated by the user
(a developer) and those generated by the logging library. To illustrate, consider the structured
[Zap] log printing statement (LPS) in [Figure 1](#figure-1) and its output in [Figure 2](#figure-2).
Comparing the user-defined KV pairs in [Figure 1](#figure-1) with KV pairs in [Figure 2](#figure-2),
we can see that the logging library adds the `timestamp` and `level` KV pairs automatically. We
define the two categories of KV pairs as follows:
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

5-5: Link fragments should be valid
null

(MD051, link-fragments)


5-5: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (7)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (7)

3-4: Use an Em Dash for Clarity
Replace the inline triple hyphens (---) with an em dash (—) for readability:

-Structured log events typically contain two categories of KV pairs---those generated by the user
+Structured log events typically contain two categories of KV pairs—those generated by the user

1-1: 🧹 Nitpick (assertive)

Title Case Consistency
For consistency with other guide sections, convert headings to Title Case. For example:

-# Auto-generated KV pairs
+# Auto-Generated KV Pairs

67-67: 🧹 Nitpick (assertive)

Add Missing Article
Include the article “a” before “user-generated KV pair”:

-If a log event has both an auto-generated KV pair and user-generated KV pair with the same key,
+If a log event has both an auto-generated KV pair and a user-generated KV pair with the same key,
🧰 Tools
🪛 LanguageTool

[uncategorized] ~67-~67: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the same key, e....

(AI_EN_LECTOR_MISSING_DETERMINER_A)


65-65: 🧹 Nitpick (assertive)

Heading Case Consistency
Apply Title Case to the second-level heading:

-## Querying auto-generated KV pairs
+## Querying Auto-Generated KV Pairs

93-94: 🧹 Nitpick (assertive)

Consistent Hyphenation
Use “keyless” instead of “key-less”:

-The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a set of key-less auto-generated and user-generated values...
+The reason we categorize unstructured text KV pairs as auto-generated is because we see them as a set of keyless auto-generated and user-generated values...

48-48: 🧹 Nitpick (assertive)

Streamline ‘JSON lines’ Reference
Simplify the markdown reference to match common usage:

-One of the key differences between KV-IR streams and [JSON lines][json-lines] (JSONL) is...
+One of the key differences between KV-IR streams and JSON Lines (JSONL) is...

78-79: 🧹 Nitpick (assertive)

Improve Phrasing and Remove Slash
Replace the slash with “or” and reduce redundancy for clarity:

- One benefit of supporting auto-generated KV pairs is that our CLP logging libraries/plugins can use a consistent set of keys for these KV pairs, making it easier for users to filter for them.
+ One benefit is that our CLP logging libraries or plugins can standardize keys for auto-generated pairs, making it easier for users to filter them.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a24dd9 and 3b839e6.

📒 Files selected for processing (1)
  • docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

[uncategorized] ~17-~17: A punctuation mark might be missing here.
Context: ...sert it into the log event. (figure-1)= :::{card} ```go logger.Info("Task compl...

(AI_EN_LECTOR_MISSING_PUNCTUATION)


[uncategorized] ~67-~67: You might be missing the article “a” here.
Context: ... has both an auto-generated KV pair and user-generated KV pair with the same key, e....

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🪛 markdownlint-cli2 (0.17.2)
docs/src/dev-guide/design-kv-ir-streams/auto-gen-kv-pairs.md

5-5: Link fragments should be valid
null

(MD051, link-fragments)


5-5: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


6-6: Link fragments should be valid
null

(MD051, link-fragments)


13-13: Link fragments should be valid
null

(MD051, link-fragments)


45-45: Link fragments should be valid
null

(MD051, link-fragments)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: lint-check (ubuntu-latest)
  • GitHub Check: lint-check (macos-latest)

filter for an auto-generated KV pair with the key `timestamp` and value `0`. Accordingly, filters on
user-generated KV pairs whose keys start with the character `@` will need to escape the `@` symbol.

## Additional benefits

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Heading Case Consistency
Apply Title Case to this section header:

-## Additional benefits
+## Additional Benefits
📝 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.

Suggested change
## Additional benefits
## Additional Benefits

@kirkrodrigues kirkrodrigues requested review from LinZhihao-723 and removed request for LinZhihao-723 May 14, 2025 00:02

@LinZhihao-723 LinZhihao-723 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For the PR title, how about:

docs(kv-ir): Add introduction for auto-generated KV pairs.

@kirkrodrigues

Copy link
Copy Markdown
Member Author

For the PR title, how about:

docs(kv-ir): Add introduction for auto-generated KV pairs.

Mm, that might be misinterpreted as we only added the intro for the section. How about:

docs(kv-ir): Explain support for auto-generated KV pairs.

@LinZhihao-723

Copy link
Copy Markdown
Member

For the PR title, how about:

docs(kv-ir): Add introduction for auto-generated KV pairs.

Mm, that might be misinterpreted as we only added the intro for the section. How about:

docs(kv-ir): Explain support for auto-generated KV pairs.

Sure. lgtm

@kirkrodrigues kirkrodrigues changed the title docs(kv-ir): Add section about auto-generated KV pairs. docs(kv-ir): Explain support for auto-generated KV pairs. May 14, 2025
@kirkrodrigues kirkrodrigues merged commit a704442 into y-scope:main May 14, 2025
6 checks passed
@kirkrodrigues kirkrodrigues deleted the kv-ir-docs-auto-gen-kv-pairs branch May 14, 2025 03:46
junhaoliao pushed a commit to junhaoliao/clp that referenced this pull request May 17, 2026
Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants