Skip to content

Dedupe analyzer messages#19419

Merged
istio-testing merged 2 commits intoistio:masterfrom
selmanj:dedupe-messages
Dec 5, 2019
Merged

Dedupe analyzer messages#19419
istio-testing merged 2 commits intoistio:masterfrom
selmanj:dedupe-messages

Conversation

@selmanj
Copy link
Copy Markdown
Contributor

@selmanj selmanj commented Dec 5, 2019

When collecting messages we include a quick check to remove any duplicates.

@selmanj selmanj requested review from a team, sushicw and zerobfd December 5, 2019 18:46
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Dec 5, 2019
@istio-testing istio-testing added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 5, 2019
When collecting messages we include a quick check to remove any duplicates.
Copy link
Copy Markdown
Contributor

@sushicw sushicw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest that if we had Message.Equals(), we could just have the context.messages be map[Message]bool instead of a slice and get deduping for free. Unfortunately, Go doesn't support that. :(

@selmanj
Copy link
Copy Markdown
Contributor Author

selmanj commented Dec 5, 2019

I considered doing that initially but decided not to as it would be a more invasive change. But now that you mention it, the fact that a Message isnt comparable due to the interface{} parameters would have prevented that approach entirely.

@selmanj
Copy link
Copy Markdown
Contributor Author

selmanj commented Dec 5, 2019

/retest

@istio-testing istio-testing merged commit 9ededc7 into istio:master Dec 5, 2019
selmanj added a commit to selmanj/istio that referenced this pull request Dec 6, 2019
Showing the namespace is redundant, as the relevant namespace will either be mentioned in the name of the policy object or the destination rule.

Note that this will cause duplicate messages to appear since we still iterate over all namespaces (which is needed to find all misconfigurations). This isn't a problem now that we dedupe messages (istio#19419).
istio-testing pushed a commit that referenced this pull request Dec 6, 2019
Showing the namespace is redundant, as the relevant namespace will either be mentioned in the name of the policy object or the destination rule.

Note that this will cause duplicate messages to appear since we still iterate over all namespaces (which is needed to find all misconfigurations). This isn't a problem now that we dedupe messages (#19419).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants