Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new log parser configuration for 'DUMMY_LOGTYPE', structured to support both community and partner integrations. It includes the required configuration files, metadata, and comprehensive test data to ensure correct parsing behavior. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new log parser for 'DUMMY_LOGTYPE' in both the community and partner directories, along with associated metadata and test data. The review feedback highlights a functional discrepancy where the parser output for the 'additional' field does not match the expected test results. Additionally, the reviewer noted the use of placeholder values in metadata files, which violates production-ready requirements, and suggested optimizations for the grok pattern to improve performance and remove redundant configurations.
| if ![msg_empty] { | ||
| mutate { | ||
| replace => { | ||
| "msg_label.key" => "msg" | ||
| } | ||
| } | ||
| mutate { | ||
| merge => { | ||
| "event.idm.read_only_udm.additional.fields" => "msg_label" | ||
| } | ||
| on_error => "msg_label_empty" | ||
| } | ||
| } |
There was a problem hiding this comment.
The parser logic for the additional field does not align with the expected output structure defined in test_events.json. The current code merges a msg_label object into additional.fields, while the expected JSON shows the message directly under additional.msg.
if ![msg_empty] {
mutate {
replace => {
"event.idm.read_only_udm.additional.msg" => "%{msg}"
}
}
}
| "product": "DUMMY Product", | ||
| "vendor": "Test Vendor", | ||
| "description": "Some sort of product from this vendor.", | ||
| "log_type": "DUMMY_LOGTYPE" |
There was a problem hiding this comment.
This file contains placeholder "DUMMY" and "Test Vendor" values. Per the Repository Style Guide, contributions must be production-ready and not just functionally correct. Please update the metadata with the actual product, vendor, and log type information. Additionally, note that identical placeholder files have been added to both the community and partner directories; ensure that only the intended final versions are committed to the correct paths.
References
- Contributions must be production-ready, which includes using actual product and vendor information rather than placeholders. (link)
| grok { | ||
| match => { | ||
| "message" => [ | ||
| "%{GREEDYDATA:event_data} \\| %{GREEDYDATA:kv_msg}" |
There was a problem hiding this comment.
Using multiple GREEDYDATA patterns separated by a literal can be inefficient due to potential backtracking. Since the separator is a pipe, consider using DATA (non-greedy) for the first part to improve performance and ensure correct splitting if multiple pipes are present in the log.
"%{DATA:event_data} \\| %{GREEDYDATA:kv_msg}"
| "%{GREEDYDATA:event_data} \\| %{GREEDYDATA:kv_msg}" | ||
| ] | ||
| } | ||
| overwrite => ["event_data" ,"msg" ,"kv_msg"] |
There was a problem hiding this comment.
The field msg is included in the overwrite list, but it is not defined as a named capture in the grok pattern on line 16. This makes its inclusion in the overwrite list redundant.
overwrite => ["event_data", "kv_msg"]
prasoonbirla-google
force-pushed
the
partner-parsers
branch
from
9995cd8 to
964270d
Compare
1 participant
Title (Please follow the convention below)
Please use a clear and concise title that summarizes your changes.
If this PR is related to an internal Buganizer ticket, please include its ID at the beginning.
Convention:
[Optional Buganizer ID: 123456789] Short, descriptive title of changesExamples:
Fix: Resolve issue with API endpoint returning 500 error[Buganizer ID: 987654321] Feature: Add support for custom data typesDocs: Update README with installation instructionsDescription
Please provide a detailed description of your changes. This helps reviewers understand your work and its context.
What problem does this PR solve?
(e.g., "Fixes a bug where X was happening," "Implements feature Y to allow Z," "Improves performance of function A.")
How does this PR solve the problem?
(e.g., "Modified algorithm in
src/foo.js," "Added new componentBar.vue," "Updated dependencybazto version 1.2.3.")Any other relevant information (e.g., design choices, tradeoffs, known issues):
(e.g., "Chose approach A over B due to performance considerations," "This change might affect X in certain edge cases," "Requires manual migration steps for existing users.")
Checklist:
Please ensure you have completed the following items before submitting your PR.
This helps us review your contribution faster and more efficiently.
General Checks:
Open-Source Specific Checks:
For Google Team Members and Reviewers Only:
Screenshots (If Applicable)
If your changes involve UI or visual elements, please include screenshots or GIFs here.
Ensure any sensitive data is redacted or generalized.
Further Comments / Questions
Any additional comments, questions, or areas where you'd like specific feedback.