-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix setting direction and ignore_outgoing decision #557
Conversation
I didn't test the current fix. We need to add tests with "direction" and "ignore_outgoing". |
debug("Ignore duplicated transaction on %s: %s -> %s", | ||
publisher.name, srcServer, dstServer) | ||
return false | ||
if publisher.IgnoreOutgoing && event["direction"] == "out" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move the check for IgnoreOutgoing into the else branch line 159. Then we don't have to query the value from hashmap and have it more localized (plus much cheaper execution).
Totally agree with logic of 'direction' and 'ignore_outgoing' implemented by this changeset.
Just a note with what we discussed in the team meeting: the current logic for direction "out" is wrong because it's not simply as an else for "in", without checking that the source IP address is actually local. It would make sense I think to fix that as part of this PR, but your call if you prefer to have another PR for that. |
4bb076e
to
9cae7ed
Compare
@@ -133,6 +133,7 @@ func filterEvent(event common.MapStr) error { | |||
func updateEventAddresses(publisher *PublisherType, event common.MapStr) bool { | |||
var srcServer, dstServer string | |||
src, ok := event["src"].(*common.Endpoint) | |||
fmt.Println(ok) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete the Printlns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do. Thanks.
LGTM. |
c1d6d80
to
47c601a
Compare
…ore defining the value of the direction. Adding tests.
1447f35
to
d86ea6c
Compare
Ready |
Use direction to check if it's an outgoing leg
Reported here: https://discuss.elastic.co/t/ignore-outgoing-doesnt-work-for-me/37201/2