[pkg/stanza] Fix - Operators with DropOnErrorQuiet should drop log en… #35834
Conversation
|
|
SamerJ
force-pushed
the
fix/parser/error-propagation
branch
3 times, most recently
from
5bb0df7 to
d0e2b3f
Compare
SamerJ
force-pushed
the
fix/parser/error-propagation
branch
from
d0e2b3f to
bbc8a76
Compare
|
@fatsheep9146 |
| if t.OnError == SendOnErrorQuiet || t.OnError == DropOnErrorQuiet { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
The error returned by this method isn't soley used to log a message. Sometimes the error is read and based on that we decide if a log entry should be dropped or not.
A concrete example would be https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.112.0/pkg/stanza/operator/helper/parser.go#L111
I was fearful of any refactoring because I could be going against a possible design tragectory set for this code.
There was a problem hiding this comment.
Won't this change result in a problem for transformers that are not parsers?
There was a problem hiding this comment.
I was not too fearful of removing this block, because it didn't exist before my previous PR #35010
That being said, I see your point.
My rational is as such:
We can't replace an error with nil at this level as that would make it impossible for the caller to know if they should drop the entry or send it.
The caller has visibility on the config like OnError, so I ended moving the responsibility to the caller.
Let me know what you think.
open-telemetry#35834) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Fixed issue in which Operators with DropOnErrorQuiet are not dropping log entries on error. Note: This issue was introduced by a bug fix meant to ensure Silent Operators are not logging errors (open-telemetry#35010). With this fix, this side effect bug has been resolved. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes 35010 <!--Describe what testing was performed and which tests were added.--> #### Testing Added UT
open-telemetry#35834) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Fixed issue in which Operators with DropOnErrorQuiet are not dropping log entries on error. Note: This issue was introduced by a bug fix meant to ensure Silent Operators are not logging errors (open-telemetry#35010). With this fix, this side effect bug has been resolved. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes 35010 <!--Describe what testing was performed and which tests were added.--> #### Testing Added UT
Description
Fixed issue in which Operators with DropOnErrorQuiet are not dropping log entries on error.
Note: This issue was introduced by a bug fix meant to ensure Silent Operators are not logging errors (#35010). With this fix,
this side effect bug has been resolved.
Link to tracking issue
Fixes 35010
Testing
Added UT