[receiver/windowseventlog] Add suppress_rendering_info parameter and simplify internal logic. #34720
Conversation
djaglowski
force-pushed
the
wel-supress-rendering-info
branch
from
6c24134
to
c22a815
Compare
| i.Logger().Error("Failed to create entry", zap.Error(err)) | ||
| return | ||
| if i.remote.Server != "" { | ||
| e.Attributes["remote_server"] = i.remote.Server |
There was a problem hiding this comment.
I think the server.address attribute may be appropriate here. Any opinions on this?
There was a problem hiding this comment.
It seems reasonable to me.
djaglowski
force-pushed
the
wel-supress-rendering-info
branch
2 times, most recently
from
1372b67
to
1560b54
Compare
djaglowski
force-pushed
the
wel-supress-rendering-info
branch
from
edc9933
to
28b81d0
Compare
|
@djaglowski ping me when you think this one is ready (or if you want another pass while still a draft) |
|
Thanks @pjanotti, I'm still just trying to get it to pass CI. There have been a bunch of unrelated tests failing but now it's just https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/10706954934/job/29685874116?pr=34720#step:6:556, which may or may not be related. I don't know if you have any insight on that one but if you do it would be very helpful. |
|
@djaglowski there is something really going on with these tests, when I submitted the PR with the overall changes to the tests I wasn't getting any failure on my machine, but, now it seems almost constant. Nothing conclusive yet but will be looking more deeply into it. |
I noticed on #34720 and #35026 that execution of the test continued beyond a failure of `require.EventuallyWithT`. Based on the description alone, I would expect that using `assert` within `require.EventuallyWithT` should cause execution to stop if the assertion fails, but it appears this may not be the case. However, this change apparently works as intended.
This is a step towards #34720 which should have no user facing impact. It consolidates the event model by removing the notion of a "raw" event. The behavior of `raw` flag should remain the same because we still use different functions to build the output from the unified event.
…simplify internal logic.
djaglowski
force-pushed
the
wel-supress-rendering-info
branch
from
3092d54
to
a167f60
Compare
Description:
This PR contains several changes described in #34131. It does not go as far as breaking out a separate parsing component, but I think it is enough to satisfy the known use cases.
I will likely break out some smaller PRs, but currently this includes:
suppress_rendering_infoparameter, which acts orthogonally torawflag.EventXMLstruct for all configurations. Remove a lot of related duplication.EventXMLso they can be more easily reused.RemoteServerfield fromEventXML. Instead, setattributes["remote_server"]if remote collection is used.EventXML.Computermay satisfy the use case. If the attribute is needed, find a better name for it.Link to tracking Issue:
Resolves #34131