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: Exclude event message #7462

Merged
merged 7 commits into from
Nov 10, 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:
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.

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

@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Oct 18, 2022
@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.1%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@latere-a-latere latere-a-latere marked this pull request as ready for review October 18, 2022 23:04
@latere-a-latere latere-a-latere requested a review from a team as a code owner October 18, 2022 23:04
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.

Hey @marnicgit thanks for the contribution!

LGTM

@salvacorts salvacorts added size/L and removed size/M labels Oct 27, 2022
@pull-request-size pull-request-size bot added size/M and removed size/L labels Nov 8, 2022
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-target-branch/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 1f1dd81 into grafana:main Nov 10, 2022
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
@latere-a-latere latere-a-latere deleted the exclude-event-message branch November 30, 2022 19:23
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Promtail] Exclude event message from Windows Events job
5 participants