Skip to content

Fixed bug where discovered intents were dropped in the mapper when collecting and parsing data from the sniffer #295

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

Merged
merged 3 commits into from
Apr 27, 2025

Conversation

zohar7ch
Copy link
Contributor

@zohar7ch zohar7ch commented Apr 22, 2025

Description

Before this fix, we sometimes didn't resolve the destination properly, which result in dropping discovered intents in the mapper.
The cause of the error is that the sniffer sometimes pass on the destination the hostname of the workload it detects, and not the ip, while the mapper tries to resolve the destination assuming it is an ip address.
So in cases we do have the IP set from the sniffer - we want to use it to resolve the workload that is attached to the address.

Testing

Describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

Please include any manual steps for testing end-to-end or functionality not covered by unit/integration tests.

Also include details of the environment this PR was developed in (language/platform/browser version).

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR and in github.com/otterize/docs

@zohar7ch zohar7ch force-pushed the zohar7ch/tcp-dest-resolve-fix-always-report branch from 739bd65 to 396628a Compare April 22, 2025 14:31
The `model.Destination` type has 2 fields that contains the destination
- the `Destination` and `DestinationIP`. We want to use `DestinationIP`
  if it is defined.
Since this fix may resolve in a lot of new detected discovered intents,
we also send with this intent some metadata (under the intent -> server
-> resolution data), so that could would be able to identify whether
this new intent was created by this fix, and to handle this differently
if it wishes.
@zohar7ch zohar7ch force-pushed the zohar7ch/tcp-dest-resolve-fix-always-report branch from 396628a to 7ae3962 Compare April 23, 2025 06:15
@zohar7ch zohar7ch requested review from omris94 and amitlicht April 24, 2025 12:34
@zohar7ch zohar7ch changed the title Bugfix: resolve TCP traffic destination by DestinationIP if it is defined "Fixed bug where sniffer would not stop gracefully upon context cancellation" Apr 24, 2025
@zohar7ch zohar7ch changed the title "Fixed bug where sniffer would not stop gracefully upon context cancellation" Fixed bug where discovered intents were dropped when collecting data from the sniffer Apr 24, 2025
@zohar7ch zohar7ch changed the title Fixed bug where discovered intents were dropped when collecting data from the sniffer Fixed bug where discovered intents were dropped in the mapper when collecting and parsing data from the sniffer Apr 24, 2025
its only source

We might discover an intent using multiple methods (for example,
socket-scan and tcp-syn-sniffing).
We want to make sure that if we discovered an intent in a way that is
not related to the bugfix - we would keep the intent that way.
This is crucial since Otterize cloud might drop intents that were
discovered only thanks to the bugfix (in order to control the fix
rollout).
@zohar7ch zohar7ch force-pushed the zohar7ch/tcp-dest-resolve-fix-always-report branch from db0c8a0 to b65ef9d Compare April 24, 2025 16:21
@zohar7ch zohar7ch merged commit cd59536 into main Apr 27, 2025
20 checks passed
@zohar7ch zohar7ch deleted the zohar7ch/tcp-dest-resolve-fix-always-report branch April 27, 2025 10:21
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants