[pkg/stanza] Windows input operator should resubscribe when remote host restarts #35175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/label os:windows |
|
/label Run Windows Not sure if this works... |
There was a problem hiding this comment.
We check for a remote subscription here
opentelemetry-collector-contrib/pkg/stanza/operator/input/windows/input.go
Lines 128 to 131 in 9139ce1
I think we should also do same while resubscribing. What do you say?
kuiperda
force-pushed
the
windowseventlog/fix-msrpc-reconnect
branch
from
961aa95
to
40184e9
Compare
atoulme
approved these changes
Sep 17, 2024
2 tasks
|
Apart from my last comment, LGTM |
pjanotti
suggested changes
Sep 17, 2024
kuiperda
force-pushed
the
windowseventlog/fix-msrpc-reconnect
branch
from
1f718d1
to
844a6a4
Compare
kuiperda
force-pushed
the
windowseventlog/fix-msrpc-reconnect
branch
from
cb26e30
to
8595e6f
Compare
kuiperda
force-pushed
the
windowseventlog/fix-msrpc-reconnect
branch
from
8595e6f
to
4ac1c32
Compare
|
@atoulme this can be merged |
djaglowski
pushed a commit
that referenced
this pull request
Oct 3, 2024
… successfully (#35520) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> If `Input::read` returns 0, the operator will attempt to open the subscription again. This happens after a resubscribe *or* whenever there are no event logs to read, which is often. As a result, this error gets logged constantly: ``` "Failed to re-open subscription for remote server"..."error: subscription handle is already open" ``` To fix this, the handle should only attempt to re-open when the resubscribe happens. This is a followup to #35175 which fixed that resubscribe issue for the user that raised it, but they have also been confused by this error being logged. **Link to tracking Issue:** <Issue number if applicable> **N/A** **Testing:** <Describe what testing was performed and which tests were added.> Manually verified log collection is successful before and after remote host reconnects, and that the noisy error is silenced except during resubscribe, where it is relevant. **Documentation:** <Describe the documentation added.> **N/A**
jriguera
pushed a commit
to springernature/opentelemetry-collector-contrib
that referenced
this pull request
Oct 4, 2024
…st restarts (open-telemetry#35175) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> While collecting Windows Event Logs from a remote host, if that host restarts or shuts off, the collector begins logging errors and stops collecting WE logs, even after the remote host is back up. Restarting the collector fixes this. To fix this without restarting the collector, the stanza input operator needs to resubscribe if the remote host restarts. Error log: ``` "Failed to read events from subscription","error":"The handle is invalid." ``` **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> Manually tested with a build of the collector, and confirmed logs continue being collected after the remote host restarts. **Documentation:** <Describe the documentation added.> N/A, this change is expected behavior
jriguera
pushed a commit
to springernature/opentelemetry-collector-contrib
that referenced
this pull request
Oct 4, 2024
… successfully (open-telemetry#35520) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> If `Input::read` returns 0, the operator will attempt to open the subscription again. This happens after a resubscribe *or* whenever there are no event logs to read, which is often. As a result, this error gets logged constantly: ``` "Failed to re-open subscription for remote server"..."error: subscription handle is already open" ``` To fix this, the handle should only attempt to re-open when the resubscribe happens. This is a followup to open-telemetry#35175 which fixed that resubscribe issue for the user that raised it, but they have also been confused by this error being logged. **Link to tracking Issue:** <Issue number if applicable> **N/A** **Testing:** <Describe what testing was performed and which tests were added.> Manually verified log collection is successful before and after remote host reconnects, and that the noisy error is silenced except during resubscribe, where it is relevant. **Documentation:** <Describe the documentation added.> **N/A**
Eromosele-SM
pushed a commit
to sematext/opentelemetry-collector-contrib
that referenced
this pull request
Oct 9, 2024
… successfully (open-telemetry#35520) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> If `Input::read` returns 0, the operator will attempt to open the subscription again. This happens after a resubscribe *or* whenever there are no event logs to read, which is often. As a result, this error gets logged constantly: ``` "Failed to re-open subscription for remote server"..."error: subscription handle is already open" ``` To fix this, the handle should only attempt to re-open when the resubscribe happens. This is a followup to open-telemetry#35175 which fixed that resubscribe issue for the user that raised it, but they have also been confused by this error being logged. **Link to tracking Issue:** <Issue number if applicable> **N/A** **Testing:** <Describe what testing was performed and which tests were added.> Manually verified log collection is successful before and after remote host reconnects, and that the noisy error is silenced except during resubscribe, where it is relevant. **Documentation:** <Describe the documentation added.> **N/A**
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
While collecting Windows Event Logs from a remote host, if that host restarts or shuts off, the collector begins logging errors and stops collecting WE logs, even after the remote host is back up. Restarting the collector fixes this.
To fix this without restarting the collector, the stanza input operator needs to resubscribe if the remote host restarts.
Error log:
Link to tracking Issue:
Testing:
Manually tested with a build of the collector, and confirmed logs continue being collected after the remote host restarts.
Documentation:
N/A, this change is expected behavior