-
Notifications
You must be signed in to change notification settings - Fork 181
[ON Week] Fix errors in code blocks in Fleet docs #3982
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
| # If kube_config is not set, KUBECONFIG environment variable will be checked | ||
| # and if not present it will fall back to InCluster | ||
| kube_config: ${fleet} and {agent} Guide/.kube/config | ||
| kube_config: ~/.kube/config |
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.
Wowzer. This must've been a bad regex match? Good find.
| | `event.provider` | `winlog.provider_name` | `Name` attribute | | ||
| | `event.action` | `winlog.task` | | | ||
| | `event.outcome` | `winlog.outcome` | | | ||
| | `host.name` | `winlog.computer_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.
I'm not sure how to validate these changes
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.
@bmorelli25, yeah, I should have added a note about this change as it’s not obvious.. Originally, Cursor only highlighted that event.host.name should be changed to host.name. Here’s what it said:
decode_xml_wineventlog-processor.md- Inconsistent Field Mapping Documentation
File:decode_xml_wineventlog-processor.md(Lines 148-158)
Issue: The ECS field mapping table shows event.host.name but this is not a standard ECS field. The correct field ishost.name.
Current Documentation:
| event.host.name | <Event><System><Computer> |
Recommendation:
| host.name | <Event><System><Computer> |
Based on the source code and the actual output example in the same file (lines 49-51), the correct ECS field ishost.name, notevent.host.name.
So I went ahead and checked the example it refers to on line 39-87 (this is a link to the original doc before the current change).
Because the original table for mapping the ECS fields, where the AI found the error, was inconsistent in how it referred to the winlog fields, I checked the example and figured out I could replace <Event><System><Provider> with the respective winlog field name that matched based on the example - see lines 47 and 61.
I applied the same logic for the other changes in the Field column - checking against the winlog fields that are in the example. Does that make sense?
It made sense to me, but I’m not an expert, so if you think it would be safer to stick with the original values, I can revert these changes and we could only keep event.host.name -> host.name. Or revert that as well?
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.
Just another thought:
So the doc includes two tables at the end:
- the first table shows the mapping of the winlog fields to the original event fields.
- the second table shows the mapping of the ECS fields to the winlog fields.
Because the second column says "Source XML or other field”, I suppose it’s not incorrect to have mixed values in this column, including both winlog fields and the original event information, so my change may be unnecessary. But they’re essentially the same thing as far as I understand (because of the mapping: original event info -> winglog fields -> ECS fields).
Hmm 🤔 If we go with the winlog fields as the values in the "Source XML or other field” column, then maybe the third column “Notes” and the information about the Name attribute should also be deleted.
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.
Thanks for the explanation.
So I agree that event.host.name → host.name is definitely correct as ECS doesn’t have event.host.name.
And I now understand what the other changes are. I guess whether the “Source XML or other field” column says winlog.provider_name or <Event><System><Provider> is just a question of which stage in the pipeline we choose to show. The value itself is the same.
Also the column is literally titled “Source XML or other field”, so having a mix of raw XML and winlog.* is not incorrect. Standardizing on winlog where we already have a clear winlog mapping arguably makes the ECS table easier to read.
Also also, if the source column says winlog.provider_name, I still think it’s still helpful to say that it comes from the provider Name attribute” in the notes (as the Guid attribute matches the same pattern). So I don't think I'd get rid of that.
You could have an extra column and call out XML in one column and field in the other. But idk if that's needed.
tl;dr I think this is an okay change as the information is correct and the table is now easier to read.
bmorelli25
left a comment
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.
These are some really great finds! One question above.
This PR fixes errors in code blocks in the Fleet docs using an AI assistant (Cursor) and semantic code search in Elastic repos for identifying and confirming the errors.
It also fixes a couple of smaller issues:
——
AI-assisted by Cursor using the claude-4.5-sonnet (thinking) model; necessary changes were identified in multiple iterations.