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

Promtail: Fix collecting userdata field from Windows Event Log #7461

Merged
merged 6 commits into from
Nov 7, 2022

Conversation

latere-a-latere
Copy link
Contributor

@latere-a-latere latere-a-latere commented Oct 18, 2022

What this PR does / why we need it:
The windows event log target for Promtail had a typo that led to EventData being assigned to UserData. This PR fixes this typo.

Which issue(s) this PR fixes:
Fixes #6167

Special notes for your reviewer:
First time contributing real code! Please let me know if I missed anything :)

Checklist

  • Reviewed the CONTRIBUTING.md guide
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@latere-a-latere latere-a-latere requested a review from a team as a code owner October 18, 2022 22:10
Copy link
Contributor

@chaudum chaudum left a comment

Choose a reason for hiding this comment

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

Thanks for spotting and fixing the bug!

LGTM

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@MichelHollands MichelHollands merged commit 17c36d6 into grafana:main Nov 7, 2022
lxwzy pushed a commit to lxwzy/loki that referenced this pull request Nov 7, 2022
…na#7461)

**What this PR does / why we need it**:
The windows event log target for Promtail had a typo that led to
`EventData` being assigned to `UserData`. This PR fixes this typo.

**Which issue(s) this PR fixes**:
Fixes grafana#6167
changhyuni pushed a commit to changhyuni/loki that referenced this pull request Nov 8, 2022
…na#7461)

**What this PR does / why we need it**:
The windows event log target for Promtail had a typo that led to
`EventData` being assigned to `UserData`. This PR fixes this typo.

**Which issue(s) this PR fixes**:
Fixes grafana#6167
MichelHollands pushed a commit that referenced this pull request Nov 10, 2022
**What this PR does / why we need it**:
Windows Event Logs have an Event Message field that is intended for
human eyes, and often contains data that already present in the event
data XML. Omitting this field the same way we can already omit
`user_data` and `event_data` can easily save a lot of bytes of data per
event - Event ID 4264 alone has ~2KB of just text that is already
present in `event_data`.

**Which issue(s) this PR fixes**:
Fixes #7395 

**Special notes for your reviewer**:
I also took the liberty to improve upon the existing test
'`Test_renderEntries` by using unique values for each field rather than
10's everywhere. I expect this to conflict with my other PR, #7461.
latere-a-latere added a commit to latere-a-latere/loki that referenced this pull request Nov 14, 2022
- PR grafana#7462 and grafana#7461 were **not** included in the 2.7.x release (they were submitted right after the cutoff and did not get backported) and accidentally ended up in the 2.7.x changelog. Moved them up to Main/Unreleased.
- Moved grafana#7602 to enhancements section of Promtail
vlad-diachenko pushed a commit that referenced this pull request Nov 29, 2022
… 2.7.x (#7690)

Fixes the changelog.
- PR #7462 and #7461 were **not** included in the 2.7.x release (they
were submitted right after the cutoff and did not get backported) and
accidentally ended up in the 2.7.x changelog. Moved them up to
Main/Unreleased.
- Moved #7602 to enhancements section of Promtail
vlad-diachenko added a commit that referenced this pull request Nov 29, 2022
…t log fix" (#7801)

Reverts #7675
as long as #7461 was not merged to
release-2.7.x branch, we do not need the upgrade guide
Abuelodelanada pushed a commit to canonical/loki that referenced this pull request Dec 1, 2022
…na#7461)

**What this PR does / why we need it**:
The windows event log target for Promtail had a typo that led to
`EventData` being assigned to `UserData`. This PR fixes this typo.

**Which issue(s) this PR fixes**:
Fixes grafana#6167
Abuelodelanada pushed a commit to canonical/loki that referenced this pull request Dec 1, 2022
**What this PR does / why we need it**:
Windows Event Logs have an Event Message field that is intended for
human eyes, and often contains data that already present in the event
data XML. Omitting this field the same way we can already omit
`user_data` and `event_data` can easily save a lot of bytes of data per
event - Event ID 4264 alone has ~2KB of just text that is already
present in `event_data`.

**Which issue(s) this PR fixes**:
Fixes grafana#7395 

**Special notes for your reviewer**:
I also took the liberty to improve upon the existing test
'`Test_renderEntries` by using unique values for each field rather than
10's everywhere. I expect this to conflict with my other PR, grafana#7461.
Abuelodelanada pushed a commit to canonical/loki that referenced this pull request Dec 1, 2022
… 2.7.x (grafana#7690)

Fixes the changelog.
- PR grafana#7462 and grafana#7461 were **not** included in the 2.7.x release (they
were submitted right after the cutoff and did not get backported) and
accidentally ended up in the 2.7.x changelog. Moved them up to
Main/Unreleased.
- Moved grafana#7602 to enhancements section of Promtail
@latere-a-latere latere-a-latere deleted the Userdata-fix branch December 15, 2022 13:42
@DylanGuedes DylanGuedes added kind/bug backport release-2.7.x add to a PR to backport it into release 2.7.x labels Feb 23, 2023
@grafanabot
Copy link
Collaborator

Hello @DylanGuedes!
Backport pull requests need to be either:

  • Pull requests which address bugs,
  • Urgent fixes which need product approval, in order to get merged,
  • Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

@DylanGuedes DylanGuedes added type/bug Somehing is not working as expected backport release-2.7.x add to a PR to backport it into release 2.7.x and removed backport release-2.7.x add to a PR to backport it into release 2.7.x labels Feb 23, 2023
@DylanGuedes DylanGuedes added backport release-2.7.x add to a PR to backport it into release 2.7.x and removed backport release-2.7.x add to a PR to backport it into release 2.7.x labels Feb 23, 2023
grafanabot pushed a commit that referenced this pull request Feb 23, 2023
**What this PR does / why we need it**:
The windows event log target for Promtail had a typo that led to
`EventData` being assigned to `UserData`. This PR fixes this typo.

**Which issue(s) this PR fixes**:
Fixes #6167

(cherry picked from commit 17c36d6)
DylanGuedes added a commit that referenced this pull request Feb 23, 2023
…Event Log (#8605)

Backport 17c36d6 from #7461

---------

Co-authored-by: MarNicGit <47538428+MarNicGit@users.noreply.github.com>
Co-authored-by: DylanGuedes <djmgguedes@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.7.x add to a PR to backport it into release 2.7.x size/XS type/bug Somehing is not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Promtail Windows Event Log missing user_data
5 participants