-
Notifications
You must be signed in to change notification settings - Fork 9
docs: Add guide for using ClpKeyValuePairStreamHandler
.
#56
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
WalkthroughThe pull request introduces a new section in the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Logger as Logger
participant Handler as ClpKeyValuePairStreamHandler
participant Stream as CLP IR Stream
App->>Logger: Log message with key-value dictionary
Logger->>Handler: Forward log record (with key-value data)
Handler->>Handler: Serialize key-value pairs into CLP format
Handler->>Stream: Write the serialized data
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
README.md
Outdated
Introduced in version 0.0.14, `ClpKeyValuePairStreamHandler` enables applications to log and | ||
serialize key-value pair objects directly into the CLP key-value pair IR stream format using | ||
Python's standard logging APIs. It operates similarly to Python's standard `logging.StreamHandler`, | ||
with the following differences: | ||
- Log events (`logging.LogRecord`) should contain the key-value pairs that a user wants to log | ||
as a Python dictionary. | ||
- These key-value pairs are written directly, without being formatted as a string. | ||
- The key-value pairs will be serialized into the CLP key-value pair IR format before writing to | ||
the stream. | ||
|
||
> [!NOTE] | ||
> The current `ClpKeyValuePairStreamHandler` does not support `CLPLogLevelTimeout`. This feature | ||
> will be added in a future release. | ||
### Key-value pairs as Python dictionary | ||
|
||
Key-value pairs in the log event, presented as Python dictionaries, must abide by the following | ||
rules: | ||
|
||
- Keys must be of type `str`. | ||
- Values must be one of the following types: | ||
- Primitives: `int`, `float`, `str`, `bool`, or `None`. | ||
- Arrays (`list`), where each array: | ||
- may contain primitive values, dictionaries, or nested arrays. | ||
- can be empty. | ||
- Dictionaries (`dict`), where each dictionary: | ||
- must adhere to the aforementioned rules for keys and values. | ||
- can be empty. |
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.
Some parts are directly copied from ClpKeyValuePairStreamHandler
's docstring.
README.md
Outdated
- [clp-ffi-py][clp-ffi-py-pypi]: This library provides [Deserializer][clp-ffi-py-deserializer-doc] | ||
to access a kv-pair IR stream in Python. See [this example][clp-ffi-py-deserializer-example] for | ||
usage details. | ||
- clp-ffi-js: TODO |
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.
Do we want to mention log-viewer here?
README.md
Outdated
to access a kv-pair IR stream in Python. See [this example][clp-ffi-py-deserializer-example] for | ||
usage details. | ||
- clp-ffi-js: TODO | ||
|
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.
We could also add a sentence to say the generated IR stream can be directly ingested into clp-s.
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)
README.md (3)
50-64
: Enhance Readability in Bullet Points
Some bullet points (e.g. on lines 60 and 63) use phrases like "can be empty" without a subject, which affects the clarity of the instructions. Consider rephrasing these lines to form complete sentences (e.g. "Arrays can be empty." and "Dictionaries can be empty.") to improve readability and compliance with style guidelines.🧰 Tools
🪛 LanguageTool
[style] ~60-~60: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...(MISSING_IT_THERE)
[style] ~63-~63: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. ### Auto-generated kv-pairs ...(MISSING_IT_THERE)
65-85
: Clarify Auto-generated vs. User-generated kv-pairs Description
While the section successfully delineates auto-generated from user-generated key-value pairs, the description for properties like "unix_millisecs" (line 74) could be refined. For example, consider rephrasing "Unix timestamp in milliseconds since epoch" to "The Unix timestamp (in milliseconds since the epoch)" to align with grammatical expectations.🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: The singular proper name ‘Unix’ must be used with a third-person or a past tense verb.
Context: ...): - "unix_millisecs" (int
): Unix timestamp in milliseconds since epoch. - "utc...(HE_VERB_AGR)
107-115
: Overview of IR Stream Reading Options
This section concisely summarises the available options for reading and deserialising kv-pair IR streams. For further clarity, consider adding an example snippet in future updates to illustrate the deserialization process in practice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~60-~60: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...
(MISSING_IT_THERE)
[style] ~63-~63: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. ### Auto-generated kv-pairs ...
(MISSING_IT_THERE)
[grammar] ~74-~74: The singular proper name ‘Unix’ must be used with a third-person or a past tense verb.
Context: ...): - "unix_millisecs" (int
): Unix timestamp in milliseconds since epoch. - "utc...
(HE_VERB_AGR)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (ubuntu-latest, 3.11)
- GitHub Check: test (macos-latest, 3.11)
- GitHub Check: test (macos-latest, 3.8)
- GitHub Check: test-python37 (ubuntu-22.04, 3.7)
- GitHub Check: test-python37 (macos-13, 3.7)
🔇 Additional comments (3)
README.md (3)
34-49
: Clear Introduction toClpKeyValuePairStreamHandler
Documentation
The new section clearly outlines the purpose and operational differences of theClpKeyValuePairStreamHandler
relative to Python’s standard logging handlers. This addition provides a solid overview for users on how key-value pairs are logged and serialized into the CLP IR stream format.
87-105
: Well-Formatted and Clear Example Code
The provided example code snippet succinctly demonstrates how to use theClpKeyValuePairStreamHandler
. It is well-structured and adheres to best practices, making it easy for users to follow.
353-356
: Accuracy and Maintenance of Link References
The newly added link references for [clp-ffi-py-deserializer-doc], [clp-ffi-py-deserializer-example], and [clp-ffi-py-kv-pair-ir-stream] are correctly formatted and enhance the documentation. Ensure that these links remain up-to-date with external documentation to maintain accuracy over time.
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)
README.md (4)
46-52
: Effective Use of Blockquotes for Notes and WarningsThe blockquotes clearly differentiate the note about the lack of support for setting a Formatter and the warning regarding the current absence of the CLPLogLevelTimeout feature. To better adhere to markdownlint standards, check that there are no unintended blank lines inside these blockquotes.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
55-68
: Enhance Bullet Point Clarity in RequirementsThe "Key-value pair requirements" list is very informative. However, consider rephrasing the bullet items at lines 64 and 67 (e.g. "Arrays can be empty" and "Dictionaries can be empty") so that each point forms a complete sentence, thereby improving readability and style.
🧰 Tools
🪛 LanguageTool
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...(MISSING_IT_THERE)
[style] ~67-~67: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. #### Automatically generated...(MISSING_IT_THERE)
93-112
: Example Code Quality and File Handling ConsiderationThe provided example code effectively demonstrates the usage of
ClpKeyValuePairStreamHandler
. As a suggestion, consider using a context manager (e.g. usingwith open(Path("example.clp.zst"), "wb") as f:
) to ensure that the file handle is properly closed after logging operations.
358-363
: Unused Link Reference DefinitionThe link reference definition for
[clp-ffi-py-kv-pair-ir-stream]
does not appear to be used anywhere in the document. Consider removing it to keep the documentation concise and avoid potential confusion.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
360-360: Link and image reference definitions should be needed
Unused link or image reference definition: "clp-ffi-py-kv-pair-ir-stream"(MD053, link-image-reference-definitions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...
(MISSING_IT_THERE)
[style] ~67-~67: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. #### Automatically generated...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.17.2)
README.md
49-49: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
360-360: Link and image reference definitions should be needed
Unused link or image reference definition: "clp-ffi-py-kv-pair-ir-stream"
(MD053, link-image-reference-definitions)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test (ubuntu-latest, 3.11)
- GitHub Check: test (ubuntu-latest, 3.8)
- GitHub Check: test-python37 (ubuntu-22.04, 3.7)
- GitHub Check: test (macos-latest, 3.8)
- GitHub Check: test-python37 (macos-13, 3.7)
🔇 Additional comments (5)
README.md (5)
33-39
: NewLogging handlers
Section AdditionThe newly added section uses clear headings (e.g. "## Logging handlers" and "### ClpKeyValuePairStreamHandler") and appropriately highlights the version update with a release note. This clearly signals to users the introduction of the new handler.
40-44
: Comprehensive Handler Functionality DescriptionThe paragraph explains that the handler writes structured log events to the CLP IR stream and specifies how keys and metadata are handled. This information is concise and informative.
69-78
: Well-Documented Auto-generated KV-Pairs SectionThe explanation of automatically generated kv-pairs is concise and clear, effectively guiding the reader on how metadata is appended with each log event.
79-92
: Clear Presentation of Auto-generated KV-Pair DetailsThe table format neatly summarises the auto-generated kv-pair information, making it easy for users to understand the structure of each log event’s metadata.
113-122
: Clear Guidance on Reading KV-Pair IR StreamsThis section offers a succinct list of the available options for reading and deserializing kv-pair IR streams, complete with useful links and examples.
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)
README.md (4)
46-53
: Blockquote Formatting and ContentThe note and warning blocks are informative. However, a minor nitpick: a blank line inside the blockquote (as flagged by markdownlint) might affect rendering consistency. Consider removing or consolidating the blank line within the blockquote to adhere to [MD028] guidelines.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
55-67
: Key-Value Pair Requirements ClarityThe bullet list under "Key-value pair requirements" is comprehensive. As a minor suggestion, consider rephrasing the items in lines 64 and 67 to include a subject (e.g., "Each array can be empty." / "Each dictionary can be empty.") which may improve clarity for readers who prefer complete sentence constructions.
🧰 Tools
🪛 LanguageTool
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...(MISSING_IT_THERE)
[style] ~67-~67: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. #### Automatically generated...(MISSING_IT_THERE)
93-111
: Example Code for ClpKeyValuePairStreamHandlerThe example code snippet is straightforward and demonstrates the intended usage of the handler. One small suggestion: although it is an example, consider using a context manager (the
with
statement) when opening files to promote best practices for resource management in real-world applications.
40-50
: Additional Content Consideration Based on Past FeedbackPast review comments mentioned possibly referencing the log viewer and clarifying that the generated IR stream can be directly ingested into clp-s. While the log viewer is already mentioned, you might consider adding a brief sentence in this new section (or a subsequent update) to mention the potential for direct ingestion into clp-s. This could further assist users in understanding integrations available with the handler.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~64-~64: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...
(MISSING_IT_THERE)
[style] ~67-~67: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. #### Automatically generated...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.17.2)
README.md
49-49: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-python37 (ubuntu-22.04, 3.7)
- GitHub Check: test (macos-latest, 3.8)
🔇 Additional comments (7)
README.md (7)
33-34
: New Section Introduction ClarityThe new "## Logging handlers" section is well placed after "Quick Start" and clearly introduces the documentation update. This sets the stage nicely for the ensuing details on the
ClpKeyValuePairStreamHandler
.
36-39
: Handler Title and Version AnnotationThe "### ClpKeyValuePairStreamHandler" title coupled with the "⭐ New in v0.0.14" note effectively communicates the new feature’s introduction and version. This is clear and impactful.
40-44
: Handler Description DetailThe descriptive paragraph explains the handler’s purpose and behaviour clearly. It concisely states how structured log events (in the form of Python dictionaries) are processed into the CLP key-value pair IR stream. This aligns well with the documented requirements.
69-77
: Auto-generated KV-Pairs SectionThe "Automatically generated kv-pairs" section clearly distinguishes between user-generated and system-generated keys. This detailed explanation and the included note effectively clarify potential key collisions.
81-91
: Auto-generated KV-Pairs TableThe table detailing auto-generated kv-pairs is well-formatted and utilises markdown's table syntax effectively. This structured format enhances readability and quick reference for users.
113-122
: Reading KV-Pair IR Streams SectionThe section on "Reading kv-pair IR streams" is informative and clearly outlines the available options, including links to additional documentation. It effectively complements the new handler feature.
357-362
: Link References and Documentation ConsistencyThe new link references for
[clp-ffi-py-deserializer-doc]
,[clp-ffi-py-deserializer-example]
,[clp-ffi-py-pypi]
, and[py-logging-formatter]
are clear and correctly positioned at the end of the document. This approach helps maintain a clean main text and provides easy access to detailed resources.
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.
Some trivial suggestions. The overall modification lgtm
Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com>
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)
README.md (3)
33-33
: Blank Line Consistency:
A new blank line was inserted on line 33. Please verify that this spacing is consistent with other section breaks in the document.
56-68
: Key-Value Pair Requirements Clarity:
The bullet-list of key-value requirements is comprehensive. To enhance readability, consider rephrasing some bullet items as complete sentences (e.g. "Arrays may contain primitive values, dictionaries, or nested arrays and can be empty."). This would ensure clarity for all readers.🧰 Tools
🪛 LanguageTool
[style] ~65-~65: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...(MISSING_IT_THERE)
[style] ~68-~68: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. #### Automatically generated...(MISSING_IT_THERE)
94-112
: Example Code Quality:
The example demonstrating the usage ofClpKeyValuePairStreamHandler
is succinct and clear. For improved clarity, consider adding inline comments within the code snippet to briefly explain each step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~65-~65: To form a complete sentence, be sure to include a subject.
Context: ..., dictionaries, or nested arrays. - can be empty. - Dictionaries (dict
), wh...
(MISSING_IT_THERE)
[style] ~68-~68: To form a complete sentence, be sure to include a subject.
Context: ...tioned rules for keys and values. - can be empty. #### Automatically generated...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.17.2)
README.md
50-50: Blank line inside blockquote
null
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (macos-latest, 3.8)
- GitHub Check: test-python37 (macos-13, 3.7)
🔇 Additional comments (9)
README.md (9)
34-34
: New Section: "Logging handlers" Added
This new section provides a clear separation for logging handler documentation. The title is concise and appropriately placed.
36-36
: Subsection Title Clarity:
"### ClpKeyValuePairStreamHandler" clearly identifies the handler being documented.
38-38
: Version Indicator Notation:
The inclusion of "⭐ New in v0.0.14" effectively highlights the introduction of this handler in the specified version.
40-44
: Handler Description Clarity:
The paragraph explaining what the handler does is clear and informative. It outlines the purpose and functionality well. Please ensure that key terminology (e.g. “kv-pair” and “IR stream format”) consistently matches internal documentation and implementation.
46-50
: Informational Note on Formatter:
The note properly explains that the handler does not support setting a formatter because it deals with structured log events. Additionally, consider checking for any unnecessary blank lines within the blockquote that might trigger markdownlint (MD028) warnings.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
50-50: Blank line inside blockquote
null(MD028, no-blanks-blockquote)
51-55
: Warning About CLPLogLevelTimeout:
The warning block clearly informs the user thatClpKeyValuePairStreamHandler
currently does not supportCLPLogLevelTimeout
and that this feature will be added in a future release.
70-79
: Auto-Generated KV-Pairs Section:
The section distinguishing user-generated from auto-generated kv-pairs is well written and informative. It clearly lays out the additional metadata included with each log event.
80-92
: KV-Pairs Table Formatting:
The table detailing the auto-generated kv-pairs is easy to read and provides valuable quick-reference information.
359-362
: Updated Reference Links:
The newly added link definitions for[clp-ffi-py-deserializer-doc]
,[clp-ffi-py-deserializer-example]
,[clp-ffi-py-pypi]
, and[py-logging-formatter]
appear to be correct. Please double-check that all URLs are current and that the anchors used in the document match these references.
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 don't have any more comments.
Description
As title.
Checklist
breaking change.
Validation performed
Summary by CodeRabbit
ClpKeyValuePairStreamHandler
class and its usage for logging structured key-value pairs.