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

[receiver/windowseventlog] Add Execution and Security information to parsed event log #27864

Conversation

BinaryFissionGames
Copy link
Contributor

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

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be good after double-checking the Execution optional fields.

pkg/stanza/operator/input/windows/xml.go Outdated Show resolved Hide resolved
.chloggen/feat_windows-evlog_security-and-execution.yaml Outdated Show resolved Hide resolved
@atoulme atoulme added the Run Windows Enable running windows test on a PR label Oct 19, 2023
if len(details) > 0 {
body["details"] = details
}

if e.Security != nil && e.Security.UserID != "" {
body["security"] = map[string]any{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: other locations on this file use map[string]interface{}

@djaglowski what is the general preference on this type of nit issue? Consistency, brevity, latest language patterns? (I have my preferences, but, the biggest one is following the practices of the repo/package).

Anyway, this is not material no need to hold merge because of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd prefer to take steps towards latest language patterns but I don't feel strongly about it here.

@pjanotti
Copy link
Contributor

pjanotti commented Oct 20, 2023

@pjanotti
Copy link
Contributor

@djaglowski I think this is good to be merged, could you PTAL?

@djaglowski djaglowski merged commit a8531c0 into open-telemetry:main Oct 24, 2023
91 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 24, 2023
sigilioso pushed a commit to carlossscastro/opentelemetry-collector-contrib that referenced this pull request 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>
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request 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 pull request 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/stanza Run Windows Enable running windows test on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants