Skip to content
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

[reciever/windowseventlog] Parse additional fields #27810

Closed
BinaryFissionGames opened this issue Oct 17, 2023 · 13 comments
Closed

[reciever/windowseventlog] Parse additional fields #27810

BinaryFissionGames opened this issue Oct 17, 2023 · 13 comments

Comments

@BinaryFissionGames
Copy link
Contributor

Component(s)

receiver/windowseventlog

Is your feature request related to a problem? Please describe.

There are additional fields on the windows event XML schema that are useful to users, but we don't collect into the structured output of the receiver. It would be good to capture some of these fields.

Describe the solution you'd like

Add the following fields to the parsed output of the windows event log receiver:

  1. Security (see: SystemPropertiesType Schema)
    a. This field may contain the UserID of the user that triggered the action, which would be useful for auditing purposes
  2. Execution (see: SystemPropertiesType Schema)
    a. This field gives useful information about the process that triggered the event log when present.

Additionally, it'd be useful to scrape the UserData field. Unfortunately, the schema here is more free-form, which makes it more difficult to map to the key:value structure we have.

One idea could be some recursive structure like:

{
  tag: string
  attributes: map[string]string
  charData: string
  children: []XmlElement 
}: XMLElement

A structure like this could properly represent arbitrary XML, I believe.

Another idea would be to just keep the original XML for this field and store it on the log as unparsed text.


I have also considered adding the xml namespaces (the xmlns attribute on the root tag) as an additional field, but it seems like this tag is always the same and doesn't seem to really add any information to a parsed log.

Describe alternatives you've considered

One alternative idea would be to use the raw flag, in addition to some XML parsing step. Currently, I don't think there's an XML parser operator, or a converter for OTTL, and I think that could lead to an "unwieldy" representation of the log.

Additional context

No response

@BinaryFissionGames BinaryFissionGames added enhancement New feature or request needs triage New item requiring triage labels Oct 17, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@djaglowski
Copy link
Member

@pjanotti, any recommendations on this? Thanks.

@pjanotti
Copy link
Contributor

pjanotti commented Oct 18, 2023

@djaglowski we should add the 2 properties mentioned by @BinaryFissionGames. Since they are structured it should be simple to add them. The event data is typically more useful if the publisher metadata is missing, if the metadata is available its contents are typically rendered in the message text. I will start with the 2 mentioned here since they look more straightforward to implement and will look at the event data later.

@crobert-1 crobert-1 removed the needs triage New item requiring triage label Oct 18, 2023
@BinaryFissionGames
Copy link
Contributor Author

@pjanotti I can take on adding these properties, if it's something you won't be getting to soon.

@pjanotti
Copy link
Contributor

@BinaryFissionGames please, go ahead and add these properties. There is always more stuff to do ;)

@BinaryFissionGames
Copy link
Contributor Author

@pjanotti Threw up a PR for Execution and Security, when you're ready to take a look.

I did try sketching out a solution for UserData here, which is what I outlined above; Not sure how much I like the resultant structure, but also not sure how else to map XML -> JSON without potential key collisions.
https://github.com/observIQ/opentelemetry-collector-contrib/compare/c72e2ed..observIQ:opentelemetry-collector-contrib:feat/windows-evlog/userdata-parsing?expand=1

djaglowski pushed a commit that referenced this issue Oct 24, 2023
…parsed event log (#27864)

**Description:**
Adds parsing for Execution and Security sections of the event log, as
defined in the schema here:
https://learn.microsoft.com/en-us/windows/win32/wes/eventschema-systempropertiestype-complextype

**Link to tracking Issue:** #27810

**Testing:**
* Added some unit tests
* Tested on a windows machine to make sure it parsed correctly on a real
system

---------

Co-authored-by: Paulo Janotti <pjanotti@splunk.com>
@pjanotti
Copy link
Contributor

@BinaryFissionGames I didn't look yet at your sketch for UserData. I'm going to open a PR to collect EventData even if the Name attribute is not present. I will take a look at you proposal for UserData after that.

@BinaryFissionGames
Copy link
Contributor Author

I opened #28621 as a draft PR so that the UserData stuff is easier to comment on when you get around to taking a look.

sigilioso pushed a commit to carlossscastro/opentelemetry-collector-contrib that referenced this issue Oct 27, 2023
…parsed event log (open-telemetry#27864)

**Description:**
Adds parsing for Execution and Security sections of the event log, as
defined in the schema here:
https://learn.microsoft.com/en-us/windows/win32/wes/eventschema-systempropertiestype-complextype

**Link to tracking Issue:** open-telemetry#27810

**Testing:**
* Added some unit tests
* Tested on a windows machine to make sure it parsed correctly on a real
system

---------

Co-authored-by: Paulo Janotti <pjanotti@splunk.com>
@BinaryFissionGames
Copy link
Contributor Author

@pjanotti Any chance you'll be able to take a look at #28621 soon? Even just high-level feedback on the structure (if there's a better way to represent it) would be much appreciated!

@pjanotti
Copy link
Contributor

pjanotti commented Nov 2, 2023

Hi @BinaryFissionGames - sorry, I have been pretty busy. I can take a look next Monday. Please, ping me if I don't! ✅ Done.

@pjanotti
Copy link
Contributor

pjanotti commented Nov 6, 2023

A general assumption on my part is that even if we are capturing the rendered message we still want to capture the structured XML data. Here is what I mean: if the provider manifest is properly installed we current capture the message for an event with UserData, e.g: Starting session 1 - ‎2023‎-‎11‎-‎03T15:24:28.469392300Z.. However, we still want to capture the structured XML Data so it becomes easier to query and independent of the message (especially regarding the fact that the message can be localized). XML corresponding to the event above:

<Event xmlns="http://schemas.microsoft.com/win/2004/08/events/event">
 <System>
  <Provider Name="Microsoft-Windows-RestartManager" Guid="{0888e5ef-9b98-4695-979d-e92ce4247224}" /> 
  <EventID>10000</EventID> 
  <Version>0</Version> 
  <Level>4</Level> 
  <Task>0</Task> 
  <Opcode>0</Opcode> 
  <Keywords>0x8000000000000000</Keywords> 
  <TimeCreated SystemTime="2023-11-03T15:24:28.4700739Z" /> 
  <EventRecordID>404448</EventRecordID> 
  <Correlation /> 
  <Execution ProcessID="35952" ThreadID="27360" /> 
  <Channel>Application</Channel> 
  <Computer>MyComputer</Computer> 
  <Security UserID="S-1-5-XXXXXXXXXXXXXXXXXXX" /> 
  </System>
  <UserData>
   <RmSessionEvent xmlns="http://www.microsoft.com/2005/08/Windows/Reliability/RestartManager/">
     <RmSessionId>1</RmSessionId> 
     <UTCStartTime>2023-11-03T15:24:28.4693923Z</UTCStartTime> 
    </RmSessionEvent>
   </UserData>
</Event>

jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this issue Nov 12, 2023
…parsed event log (open-telemetry#27864)

**Description:**
Adds parsing for Execution and Security sections of the event log, as
defined in the schema here:
https://learn.microsoft.com/en-us/windows/win32/wes/eventschema-systempropertiestype-complextype

**Link to tracking Issue:** open-telemetry#27810

**Testing:**
* Added some unit tests
* Tested on a windows machine to make sure it parsed correctly on a real
system

---------

Co-authored-by: Paulo Janotti <pjanotti@splunk.com>
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this issue Nov 24, 2023
…parsed event log (open-telemetry#27864)

**Description:**
Adds parsing for Execution and Security sections of the event log, as
defined in the schema here:
https://learn.microsoft.com/en-us/windows/win32/wes/eventschema-systempropertiestype-complextype

**Link to tracking Issue:** open-telemetry#27810

**Testing:**
* Added some unit tests
* Tested on a windows machine to make sure it parsed correctly on a real
system

---------

Co-authored-by: Paulo Janotti <pjanotti@splunk.com>
Copy link
Contributor

github-actions bot commented Jan 8, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Jan 8, 2024
Copy link
Contributor

github-actions bot commented Mar 8, 2024

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants