[Logs] Update Logpush dataset field definitions (2026-06-24)#31674
[Logs] Update Logpush dataset field definitions (2026-06-24)#31674soheiokamoto wants to merge 1 commit into
Conversation
|
/bonk please review this automated sync of Logpush dataset field definitions. Focus on the following:
|
Review⏸️ Automatic reviews for this PR are paused. This PR has already received 2 automatic reviews. To run another review, a codeowner can comment
✅ No issues found in commit Code ReviewThis code review is in beta and may not always be helpful — use your judgment. ✅ No code review issues found. Style Guide Review✅ No style-guide issues found. CommandsOnly codeowners can run commands. Post a comment with the command to trigger it.
|
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR adds two new Logpush datasets (Firewall events and WebSocket Analytics, both account- and zone-scoped), updates the Email Security Alerts dataset with five new fields, and adds a changelog entry. The changes follow the established code-generator pattern.
Issues that need attention
-
Changelog duplication — the "New datasets" section lists WebSocket Analytics twice (lines 12 and 13). This should be deduplicated; one entry is enough even though the dataset is available in both scopes.
-
Grammar issues in field descriptions — The new
account/firewall_events.mdcontains several awkward or grammatically incorrect descriptions. Because these files are code-generated, the fixes need to be applied in the internal source YAML so they are not overwritten by future syncs. The same issues also exist in the zone-scoped file, so fixing upstream will improve both. -
Changelog title length — The title is quite long and repeats "in Cloudflare Logs" at the end. Consider shortening it (for example, "New Logpush datasets and updated fields").
Please address the duplication in the changelog before merging.
|
I've completed the review of PR #31674. Labels appliedAdded Summary of changesThe PR is an automated sync that introduces:
Issues flaggedI submitted a REQUEST CHANGES review with inline comments pointing out:
These issues are minor but the duplication in the changelog is worth fixing before merge. |
|
Preview URL: https://2d9ec43d.preview.developers.cloudflare.com Files with changes (up to 15) |
…dictory Firewall events update - Remove duplicate WebSocket Analytics new-dataset bullet (appeared twice, one per scope). - Remove redundant `Firewall events (added): ZoneName` updated-fields entry — `ZoneName` is already listed in the new account-scope Firewall events dataset. Fixes issues flagged in #31674 (comment)
soheiokamoto
force-pushed
the
sync/log-fields-2026-06-24
branch
2 times, most recently
from
5825370 to
a96952c
Compare
|
/review |
Review of PR #31674Labels
Summary of changesThis PR is an automated sync of Logpush dataset field definitions that:
The diff matches the stated intent, and the changelog entry is clear and accurate. Issues that need attention1. PR description lists WebSocket Analytics twiceThe pull request body under Suggested fix: Remove the duplicate line in the PR body. 2. PR description calls Firewall events a "New dataset"The PR body frames Firewall events as a completely new dataset, but it is actually a new account-scope availability for an existing zone-scoped dataset. The committed changelog correctly clarifies this, so this is only a minor accuracy issue in the PR description itself. 3. Remaining grammar issues in generated field descriptionsA few descriptions were inherited from the upstream source and were not fixed in this sync. Because these files are code-generated, the corrections must be applied in the internal source YAML or they will be overwritten by future syncs.
VerdictApprove with minor notes. The committed code is accurate and the changelog is well-structured. The remaining issues are cosmetic grammar gaps that should be fixed upstream to survive future automated syncs. |
soheiokamoto
force-pushed
the
sync/log-fields-2026-06-24
branch
from
a96952c to
2d9ec43
Compare
Review of PR #31674LabelsThe existing labels are appropriate for this change:
Summary of changesThis PR is an automated sync of Logpush dataset field definitions that:
The diff is clean, follows the established code-generator format, and the committed files no longer contain the grammar issues flagged in earlier reviews. Issues that need attention1. Redundant phrasing in field descriptions (fix upstream)Both
Because "ASN" stands for "Autonomous System Number", the word "number" is redundant. These files are code-generated, so the fix must be applied in the internal source YAML or it will be overwritten by future syncs. Suggested upstream fix: - The ASN number of the visitor.
+ The ASN of the visitor.
2. Changelog title length (optional)The changelog title is functional but verbose:
Since this post lives on the Logs product changelog, the trailing phrase "in Cloudflare Logs" is redundant. Consider shortening it upstream (e.g., "New WebSocket Analytics Logpush dataset and updated fields").
Inline review comments
Fix upstream: "ASN number" is redundant. Consider "The ASN of the visitor."
Fix upstream: Same redundancy as the account-scoped file. VerdictApprove with minor notes. All previously reported grammar issues ("as a string", "returned to the browser", "requested by the visitor") have been resolved in the committed files. The changelog is clear and accurate, the new fields are correctly alphabetized, and the frontmatter is valid. The remaining "ASN number" redundancy is the only item that should be corrected upstream to survive future automated syncs. |
soheiokamoto
force-pushed
the
sync/log-fields-2026-06-24
branch
from
2d9ec43 to
10074fc
Compare
|
I've completed the review of PR #31674 by reading all changed files and verifying the current state against prior review feedback and the style guide. LabelsThe existing labels are appropriate:
Summary of changesThis PR is an automated sync of Logpush dataset field definitions that:
All previously reported grammar issues ("as a string," "returned to the browser," "requested by the visitor") have been resolved in both the account and zone Issues that need attention1. Enum formatting inconsistency in WebSocket Analytics (fix upstream)Every other Logpush dataset page uses
Fix in upstream source YAML so it survives future automated syncs. 2. Ambiguous enum value
|
2 participants
New datasets
Updated fields in existing datasets
ZoneName. The Firewall events dataset is now also available for account-scope Logpush, in addition to the existing zone scope.BCC,DKIMResult,DMARCPolicy,DMARCResult, andSPFResult.Files changed
src/content/docs/logs/logpush/logpush-job/datasets/account/— dataset pagessrc/content/docs/logs/logpush/logpush-job/datasets/zone/— dataset pagessrc/content/changelog/logs/2026-06-24-log-fields-updated.mdx— changelogDocumentation checklist