WIP: NETOBSERV-2660 Add server performance benchmark improvements#1444
Open
Amoghrd wants to merge 6 commits intonetobserv:mainfrom
Open
WIP: NETOBSERV-2660 Add server performance benchmark improvements#1444Amoghrd wants to merge 6 commits intonetobserv:mainfrom
Amoghrd wants to merge 6 commits intonetobserv:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Amoghrd
added
no-qe
Contributor
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Amoghrd
force-pushed
the
server-perf-followup
branch
2 times, most recently
from
af91892 to
8152d50
Compare
Implement future improvements listed in BENCHMARKS.md with full coverage across all NetObserv UI views (Traffic, Topology, Overview): **Export Flows:** - Add BenchmarkExport with 7 scenarios testing CSV export from all tabs - Test filters, limits, column selection, and complex query combinations **Filter-Heavy Queries:** - BenchmarkFilterHeavyTableView: 1, 2, 4, 8 filters on flow records - BenchmarkFilterHeavyTopology: 1, 2, 4, 8 filters on topology metrics - BenchmarkFilterHeavyOverview: 1, 2, 4, 8 filters on overview metrics **Large Result Sets:** - BenchmarkLargeResultSets: Test with 100, 1K, 5K, 10K flow records **Aggregation Levels:** - BenchmarkTopologyAggregations: Test by namespace, app, resource, owner - BenchmarkOverviewAggregations: Test namespace, app, and mixed aggregations **Concurrent User Scenarios:** - BenchmarkConcurrentTableView: Parallel load on table view - BenchmarkConcurrentTopology: Parallel load on topology view - BenchmarkConcurrentOverview: Parallel load on overview page **Makefile Targets:** - Add convenient make targets for specific benchmark groups - Users can now run `make benchmark-export`, `make benchmark-filters`, etc. - Simplifies running benchmarks without remembering go test flags All benchmarks run successfully individually or in small groups. The concurrent benchmarks are separated to avoid macOS port exhaustion when running the full suite. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Amoghrd
force-pushed
the
server-perf-followup
branch
from
8152d50 to
324e90a
Compare
Split the 816-line server_perf_test.go into 4 focused files for better organization and maintainability: **server_perf_test.go** (175 lines): - Shared helper functions (setupBenchmarkServers, setupBenchmarkServersWithSize) - BenchmarkExport (used across all views) **server_perf_table_test.go** (169 lines): - BenchmarkTable - BenchmarkLargeResultSets - BenchmarkFilterHeavyTableView - BenchmarkConcurrentTableView **server_perf_topology_test.go** (202 lines): - BenchmarkTopologyLoki - BenchmarkTopologyAuto - BenchmarkFilterHeavyTopology - BenchmarkConcurrentTopology - BenchmarkTopologyAggregations - runTopologyQuery helper **server_perf_overview_test.go** (288 lines): - BenchmarkOverviewLoki - BenchmarkOverviewAuto - BenchmarkFilterHeavyOverview - BenchmarkConcurrentOverview - BenchmarkOverviewAggregations - Overview query builder helpers All benchmarks verified and working correctly. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove duplication of filter test data across table, topology, and overview benchmark files by extracting to a shared helper function. **Changes:** - Add getCommonFilterTests() to server_perf_test.go - Returns standardized filter strings (1, 2, 4, 8 filter combinations) - Update BenchmarkFilterHeavyTableView to use shared helper - Update BenchmarkFilterHeavyTopology to use shared helper - Update BenchmarkFilterHeavyOverview to use shared helper **Impact:** - Removed 58 lines of duplicated filter data - Added 29 lines for shared helper - Net reduction: 29 lines (834 → 805) - Filter test data now defined in ONE place **Benefits:** - Single source of truth for filter test combinations - Easier to add/modify filter test scenarios - Consistent filter testing across all views All benchmarks verified and working correctly. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace view-specific query helpers with generic shared helpers that work for both topology and overview benchmarks. **Changes:** - Add runMetricsQuery() to server_perf_test.go (shared) - Generic helper to run a single /api/flow/metrics query - Used by topology benchmarks - Add runMetricsQueries() to server_perf_test.go (shared) - Generic helper to run multiple /api/flow/metrics queries - Uses runMetricsQuery() internally - Used by overview benchmarks - Remove runTopologyQuery() from topology_test.go - Replaced with runMetricsQuery() - Remove runOverviewQueries() from overview_test.go - Replaced with runMetricsQueries() **Impact:** - Overview: 269 → 254 lines (-15) - Topology: 183 → 170 lines (-13) - Test: 204 → 224 lines (+20 for shared helpers) - Total: 805 → 797 lines (-8) **Benefits:** - Single implementation of metrics query logic - View-agnostic helpers work for both topology and overview - Easier to maintain and enhance - More consistent testing approach All benchmarks verified and working correctly. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace duplicated HTTP request/response patterns with runFlowRecordsQuery() - Use runMetricsQuery() for histogram metrics query - Remove unused net/http import - Reduce file from 150 to 100 lines (33% reduction) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Export flows: 7 → 4 scenarios (removed CSVWithColumns, WithMultipleFilters, WithLimit1000) Large result sets: 4 → 3 sizes (removed 5000records - middle ground between 1K and 10K) Data source tests: Removed Loki-only benchmarks, keeping Auto (more realistic - chooses best source automatically) Filter combinations: 4 → 3 levels (removed 8 filters scenario, keeping 1, 2, 4) - BenchmarkTopologyLoki: Removed (11 sub-benchmarks) - BenchmarkOverviewLoki: Removed (5 sub-benchmarks) - Total reduction: ~20 sub-benchmarks removed while maintaining comprehensive coverage Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implement future improvements listed in BENCHMARKS.md:
All benchmarks run successfully individually or in small groups. The concurrent benchmarks are separated to avoid macOS port exhaustion when running the full suite.
Dependencies
n/a
Checklist