Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add data aggregation level #556

Merged
merged 19 commits into from
Aug 6, 2024
Merged

feat: add data aggregation level #556

merged 19 commits into from
Aug 6, 2024

Conversation

nddq
Copy link
Contributor

@nddq nddq commented Jul 22, 2024 •
edited
Loading

Description

This PR introduced the config map option for data aggregation. More details can be found in the docs.

Related Issue

#138

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

  • Added unit tests for parsing data aggregation level from config file
  • Deploy on cluster with different data aggregation level, verified that on high, packetparser doesn't attach bpf program to eth0

Additional Notes

Add any additional notes or context about the pull request here.

Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@nddq nddq self-assigned this Jul 22, 2024
@nddq nddq requested a review from a team as a code owner July 22, 2024 20:59
@nddq nddq changed the title feat: add toggle for attach bpf program to default interface feat: add data aggregation level Jul 22, 2024
timraymond
timraymond reviewed Jul 23, 2024
View reviewed changes
Copy link
Member

@timraymond timraymond left a comment

Choose a reason for hiding this comment

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

LGTM on resolving the discussion about the config option UX.

controller/Dockerfile.windows-native Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
anubhabMajumdar
anubhabMajumdar reviewed Jul 24, 2024
View reviewed changes
pkg/config/config_test.go Show resolved Hide resolved
pkg/config/config.go Show resolved Hide resolved
pkg/plugin/packetparser/packetparser_linux.go Outdated Show resolved Hide resolved
docs/metrics/dataAggregation.md Outdated Show resolved Hide resolved
docs/metrics/dataAggregation.md Outdated Show resolved Hide resolved
anubhabMajumdar
anubhabMajumdar reviewed Jul 26, 2024
View reviewed changes
docs/metrics/dataAggregation.md Outdated Show resolved Hide resolved
pkg/config/config.go Show resolved Hide resolved
pkg/plugin/packetparser/packetparser_linux_test.go Outdated Show resolved Hide resolved
nddq added 3 commits July 29, 2024 14:34
@nddq
Update dynamic.h
1a72d9d 
Signed-off-by: Quang Nguyen <[email protected]>
@nddq
Update config.yaml
7df72c2 
Signed-off-by: Quang Nguyen <[email protected]>
@nddq
linters
3aac0b5
vakalapa
vakalapa requested changes Jul 29, 2024
View reviewed changes
pkg/config/config.go Show resolved Hide resolved
pkg/plugin/packetparser/packetparser_linux.go Outdated Show resolved Hide resolved
pkg/config/hubble_config_linux.go Outdated Show resolved Hide resolved
anubhabMajumdar
anubhabMajumdar approved these changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@anubhabMajumdar anubhabMajumdar left a comment

Choose a reason for hiding this comment

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

Please add Testing Done before merging.

@nddq
Copy link
Contributor Author

nddq commented Aug 1, 2024

Please add Testing Done before merging.

Updated

@nddq nddq enabled auto-merge August 1, 2024 00:52
@nddq nddq added this pull request to the merge queue Aug 6, 2024
Merged via the queue into main with commit 37d8401 Aug 6, 2024
22 checks passed
@nddq nddq deleted the feat/eth0bpfToggle branch August 6, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snguyen64 snguyen64 snguyen64 approved these changes

@vakalapa vakalapa vakalapa approved these changes

@MikeZappa87 MikeZappa87 Awaiting requested review from MikeZappa87 MikeZappa87 is a code owner automatically assigned from microsoft/retina

@timraymond timraymond Awaiting requested review from timraymond

@anubhabMajumdar anubhabMajumdar Awaiting requested review from anubhabMajumdar

Assignees

@nddq nddq

Labels
area/plugins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nddq @timraymond @vakalapa @anubhabMajumdar @snguyen64