Skip to content

Commit 3f18d50

Browse files
pjanottidjaglowski
authored andcommitted
[pkg/stanza] Don't get formatted msg for events without an event provider (open-telemetry#35227)
**Description:** When the event doesn't have a Publisher we should not rely on the behavior of `EvtFormatMessage` API, given, that in some cases it reports error. Instead we should fallback to the non-formatted message to avoid logging error messages on the collector. See issue open-telemetry#35135. **Link to tracking Issue:** Fix open-telemetry#35135 **Testing:** Local validation of the processing of the events reported in the issue. **Documentation:** N/A --------- Co-authored-by: Daniel Jaglowski <[email protected]>
1 parent e030c8a commit 3f18d50

File tree

4 files changed

+53
-3
lines changed

4 files changed

+53
-3
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: pkg/stanza
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Do not get formatted message for Windows events without an event provider.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [35135]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
Attempting to get the formatted message for Windows events without an event provider can result in an error being logged. |
20+
This change ensures that the formatted message is not retrieved for such events.
21+
22+
# If your change doesn't affect end users or the exported elements of any package,
23+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: [user]

pkg/stanza/operator/input/windows/publisher_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
func TestPublisherOpenPreexisting(t *testing.T) {
1515
publisher := Publisher{handle: 5}
16-
err := publisher.Open("")
16+
err := publisher.Open("provider_name_does_not_matter_for_this_test")
1717
require.Error(t, err)
1818
require.Contains(t, err.Error(), "publisher handle is already open")
1919
require.True(t, publisher.Valid())

pkg/stanza/operator/input/windows/publishercache.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,19 @@ func newPublisherCache() publisherCache {
1919
}
2020
}
2121

22-
func (c *publisherCache) get(provider string) (publisher Publisher, openPublisherErr error) {
22+
func (c *publisherCache) get(provider string) (Publisher, error) {
2323
publisher, ok := c.cache[provider]
2424
if ok {
2525
return publisher, nil
2626
}
2727

28+
var err error
2829
publisher = NewPublisher()
29-
err := publisher.Open(provider)
30+
if provider != "" {
31+
// If the provider is empty, there is nothing to be formatted on the event
32+
// keep the invalid publisher in the cache. See issue #35135
33+
err = publisher.Open(provider)
34+
}
3035

3136
// Always store the publisher even if there was an error opening it.
3237
c.cache[provider] = publisher

pkg/stanza/operator/input/windows/publishercache_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,22 @@ func TestGetInvalidPublisher(t *testing.T) {
4545
require.False(t, publisher.Valid())
4646
}
4747

48+
func TestEmptyPublisherNameBehavior(t *testing.T) {
49+
publisherCache := newPublisherCache()
50+
defer func() {
51+
require.NoError(t, publisherCache.evictAll())
52+
}()
53+
54+
publisher, openPublisherErr := publisherCache.get("")
55+
require.NoError(t, openPublisherErr) // There should be no error for an empty provider.
56+
require.False(t, publisher.Valid())
57+
58+
// Checked that the cached version works as expected.
59+
publisher, openPublisherErr = publisherCache.get("")
60+
require.NoError(t, openPublisherErr)
61+
require.False(t, publisher.Valid())
62+
}
63+
4864
func TestValidAndInvalidPublishers(t *testing.T) {
4965
publisherCache := newPublisherCache()
5066
defer func() {

0 commit comments

Comments
 (0)