✨ OPRUN-4240: Add e2e profiling toolchain for heap and CPU analysis #2298
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tmshort
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2298 +/- ##
==========================================
- Coverage 74.32% 74.26% -0.07%
==========================================
Files 90 90
Lines 7008 7014 +6
==========================================
Hits 5209 5209
- Misses 1392 1397 +5
- Partials 407 408 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Given that all other interactions with the code are happening through Makefile targets, it would be nice if we can continue doing that for the profiling as well, something like:
make baseline-profile test-experiment-e2eor maybe
make test-experiment-e2e PROFILING=baselineSimilar goes for other added scripts.
| # Get pod name | ||
| log_info "Finding pod with label: ${POD_LABEL}" | ||
| POD=$(kubectl get pod -n "${NAMESPACE}" -l "${POD_LABEL}" -o name | head -1) | ||
| if [ -z "${POD}" ]; then | ||
| log_error "No pod found with label ${POD_LABEL}" | ||
| exit 1 | ||
| fi | ||
| log_success "Found pod: ${POD}" | ||
| PODS[$component]="${POD}" | ||
|
|
||
| # Create component output directory | ||
| mkdir -p "${OUTPUT_DIR}/${component}" | ||
|
|
||
| # Set up port forwarding | ||
| log_info "Setting up port forwarding to ${POD}:${PPROF_PORT} -> localhost:${LOCAL_PORT}..." | ||
| kubectl port-forward -n "${NAMESPACE}" "${POD}" "${LOCAL_PORT}:${PPROF_PORT}" > "${OUTPUT_DIR}/${component}/port-forward.log" 2>&1 & | ||
| PF_PIDS[$component]=$! | ||
| log_info "Port-forward started (PID: ${PF_PIDS[$component]})" |
There was a problem hiding this comment.
we could set port-forwarding on the deployment directly, and that would simplify the script, because we do not need to search pods - deployment name is fixed.
|
|
||
| # Wait for port forwards to be ready | ||
| log_info "Waiting for port forwards to initialize..." | ||
| sleep 5 |
There was a problem hiding this comment.
instead of wait here, I would try to connect to the endpoint and retry several times if I get the error.
| log_success "Collection complete. Collected ${n} profiles from each component." | ||
|
|
||
| # Clean up empty profiles (created during cluster teardown) | ||
| log_info "Cleaning up empty profile files..." |
There was a problem hiding this comment.
perhaps this should be executed also when user presses ctrl-c?
| EOF | ||
| } | ||
|
|
||
| log_info() { | ||
| echo -e "${BLUE}[INFO]${NC} $*" | ||
| } | ||
|
|
||
| log_error() { | ||
| echo -e "${RED}[ERROR]${NC} $*" | ||
| } | ||
|
|
||
| log_success() { | ||
| echo -e "${GREEN}[SUCCESS]${NC} $*" | ||
| } |
There was a problem hiding this comment.
All added scripts have a share of common code. Perhaps we should extract it into something like common.sh and source it other scripts?
| - --leader-elect | ||
| {{- end }} | ||
| - --metrics-bind-address=:7443 | ||
| {{- if .Values.options.e2e.enabled }} |
There was a problem hiding this comment.
Profiling could we turned on also for other reasons, not just for e2e tests. Hence, I would suggest to name this option something like:
.Values.options.profiling.enabled
| --- | ||
|
|
||
| **Created:** 2025-10-28 | ||
| **Status:** Production Ready |
There was a problem hiding this comment.
Claude thinks it is. :)
|
|
||
| **Created:** 2025-10-28 | ||
| **Status:** Production Ready | ||
| **Version:** 1.0.0 |
There was a problem hiding this comment.
Are we gonna track the version of this separately from the released artifacts?
There was a problem hiding this comment.
There's no need to version this, so we won't bother trying to keep it in sync.
| # This will: | ||
| # 1. Start make test-experimental-e2e | ||
| # 2. Wait for pod to be ready | ||
| # 3. Collect profiles every 15s |
There was a problem hiding this comment.
the code and PR description are actually speaking about 10s intervals.
There was a problem hiding this comment.
Yup, I changed the default, and Claude didn't pick up on this.
|
|
||
| ```bash | ||
| # Set up symlinks to existing data | ||
| ln -s /home/tshort/experimental-e2e-testing/test1 e2e-profiles/baseline |
There was a problem hiding this comment.
perhaps we should make some more abstract paths instead of publishing /home/tshort ?
There was a problem hiding this comment.
Yeah, this is probably a bad example... and Claude didn't clean it up.
I'm going to disagree here. We have a number of other utilities that can be run outside the Makefile. Also, passing arguments to Makefiles is onerous (as given by your example of having to use |
tmshort
force-pushed
the
memory-profiling
branch
from
881c6a5 to
e02313c
Compare
We should not prevent people to call the scripts directly, but having no target on Makefile level makes them harder to discover. I would also say that profiling itself is orthogonal to running e2e tests. We could profile controllers even if do not run tests. If we make target for starting and stopping profiling, then we could do something like: make baseline-profiling e2e stop-profilingIf we want to profile without running tests, we can then do make baseline-profiling
# play with OLM
# ...
# ...
make stop-profiling |
There was a problem hiding this comment.
Consider this separation:
- e2e profiling toolchain for heap and CPU analysis in introduced (independent of any Claude/AI stuff)
- A helpful Claude command is introduced to help with the AI-ness (in a different PR)
Isn't a that more cleaner?
1 is something we'd do traditionally (but now it's being done with assistance from AI).
2 is the new world stuff.
| # E2E Profiling Plugin - Summary | ||
|
|
||
| ## 📊 What Was Created |
There was a problem hiding this comment.
Same comment as the other PR, we probably shouldn't commit this to main. The Readme.md and Usage.md seems to be doing the job.
There was a problem hiding this comment.
This file is useful for Claude to know how to use the tool as a plug-in.
I hear your concerns. Because the scripts are expecting to run a specific e2e test, that would require a bit of refactoring. The functionality works now with the existing e2e tests. I would consider doing that as a follow up. |
That's starting to sound like busy-work. I can do it, but I don't really see the benefit. |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
tmshort
force-pushed
the
memory-profiling
branch
from
e02313c to
a645f32
Compare
Add comprehensive profiling infrastructure to collect, analyze, and compare heap and CPU profiles during e2e test execution. **Two Profiling Workflows:** 1. **Start/Stop Workflow (Recommended)** - Start profiling in background with `make start-profiling` - Run ANY test command (make test-e2e, make test-experimental-e2e, etc.) - Stop and analyze with `make stop-profiling` - Handles cluster teardown gracefully (auto-stops after 3 consecutive failures) - Works with tests that tear down clusters (like test-e2e) 2. **Automated Workflow** - Run integrated test with `./hack/tools/e2e-profiling/e2e-profile.sh run <name>` - Automatically handles profiling lifecycle - Best for scripted/automated profiling runs **Features:** - Automated heap and CPU profile collection from operator-controller and catalogd - Real-time profile capture every 10 seconds during test execution - CPU profiling with 10-second sampling windows running in parallel - Configurable profile modes: both (default), heap-only, or CPU-only - Multi-component profiling with separate analysis for each component - Prometheus alert tracking integrated with profiling reports - Side-by-side comparison of different test runs - Graceful cluster teardown detection and auto-stop **Tooling:** - `start-profiling.sh`: Start background profiling session - `stop-profiling.sh`: Stop profiling, cleanup, and analyze - `common.sh`: Shared library with logging, colors, config, and utilities - `collect-profiles.sh`: Profile collection loop (used by start/run workflows) - `analyze-profiles.sh`: Generate detailed analysis with top allocators, growth patterns, and CPU hotspots - `compare-profiles.sh`: Compare two test runs to identify regressions - `run-profiled-test.sh`: Orchestrate full profiled test runs (automated workflow) - `e2e-profile.sh`: Main entry point with subcommands (run/analyze/compare) **Architecture Improvements:** - **Shared common library**: All scripts source `common.sh` for consistent logging, colors, and utilities - **Deployment-based port-forwarding**: Uses `deployment/` references instead of pod names for automatic failover - **Background execution**: Profiling runs in background using nohup, allowing any test command - **Intelligent retry logic**: 30-second timeout with 2-second intervals, tests components independently - **Robust cleanup (EXIT trap)**: Gracefully terminates processes, force-kills if stuck, removes empty profiles - **Multi-component support**: Profiles operator-controller and catalogd simultaneously in separate directories - **Cluster teardown detection**: Tracks consecutive failures, auto-stops after 3 failures when cluster is torn down **Usage:** Start/Stop Workflow: ```bash # Start profiling make PROFILE_NAME=baseline start-profiling # Run your tests (any command!) make test-e2e # Works! Handles cluster teardown make test-experimental-e2e # Works! go test ./test/e2e/... # Works! # Stop and analyze make stop-profiling ``` Automated Workflow: ```bash # Run with both heap and CPU profiling (default) ./hack/tools/e2e-profiling/e2e-profile.sh run baseline test-experimental-e2e # Run with heap-only profiling (reduced overhead) E2E_PROFILE_MODE=heap ./hack/tools/e2e-profiling/e2e-profile.sh run memory-test # Analyze results ./hack/tools/e2e-profiling/e2e-profile.sh analyze baseline # Compare two runs ./hack/tools/e2e-profiling/e2e-profile.sh compare baseline optimized ``` **Configuration:** Set `E2E_PROFILE_MODE` environment variable: - `both` (default): Collect both heap and CPU profiles - `heap`: Collect only heap profiles (reduces overhead by ~3%) - `cpu`: Collect only CPU profiles **Integration:** - Automatic cleanup of empty profiles from cluster teardown - Prometheus alert extraction from e2e test summaries - Detailed markdown reports with memory growth, CPU usage analysis, and recommendations - Claude Code slash command integration (`/e2e-profile start/stop/run/analyze/compare`) **Key Implementation Details:** - Background profiling: Entire collection runs in nohup with exported environment variables - Fixed interval timing: INTERVAL now includes CPU profiling time, not adds to it - Deployment wait polls until deployments are created before checking availability - Component name sanitization: Hyphens converted to underscores for valid bash variable names - PID tracking for both background process and port-forward cleanup - Consecutive failure tracking: 3 failures triggers graceful auto-stop - Silent error handling: curl errors suppressed when cluster is being torn down - 10-second intervals accurately maintained across all profiling modes - Port-forwards remain stable throughout entire test duration and survive pod restarts - Conditional profile collection based on PROFILE_MODE setting - Cleanup runs on EXIT/INT/TERM with graceful shutdown (2.5s timeout) and force-kill - Code deduplication: Common functions extracted to shared library **Code Quality:** - Reduced duplication: Shared common library for logging and utilities - Improved reliability: Deployment-based port-forwarding survives pod restarts - Better error handling: Clear timeout messages, automatic retry, robust cleanup - Flexible workflows: Start/stop for interactive use, automated for CI/CD - Enhanced documentation: Architecture guide, troubleshooting, workflow examples, and slash commands **Testing:** Verified end-to-end with `make test-e2e`: - Collected 32 heap + 31 CPU profiles per component - Auto-detected cluster teardown and stopped gracefully - Generated comprehensive analysis showing peak memory (24MB operator-controller, 16MB catalogd) - All tests passed with proper cleanup This tooling was essential for identifying memory optimization opportunities and validating that alert thresholds are correctly calibrated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
tmshort
force-pushed
the
memory-profiling
branch
from
a645f32 to
eb1ecbb
Compare
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
3 participants
Goes along with #2309
Add comprehensive profiling infrastructure to collect, analyze, and
compare heap and CPU profiles during e2e test execution.
Two Profiling Workflows:
Start/Stop Workflow (Recommended)
make start-profilingmake stop-profilingAutomated Workflow
./hack/tools/e2e-profiling/e2e-profile.sh run <name>Features:
Tooling:
start-profiling.sh: Start background profiling sessionstop-profiling.sh: Stop profiling, cleanup, and analyzecommon.sh: Shared library with logging, colors, config, and utilitiescollect-profiles.sh: Profile collection loop (used by start/run workflows)analyze-profiles.sh: Generate detailed analysis with top allocators, growth patterns, and CPU hotspotscompare-profiles.sh: Compare two test runs to identify regressionsrun-profiled-test.sh: Orchestrate full profiled test runs (automated workflow)e2e-profile.sh: Main entry point with subcommands (run/analyze/compare)Architecture Improvements:
common.shfor consistent logging, colors, and utilitiesdeployment/references instead of pod names for automatic failoverUsage:
Start/Stop Workflow:
Automated Workflow:
Configuration:
Set
E2E_PROFILE_MODEenvironment variable:both(default): Collect both heap and CPU profilesheap: Collect only heap profiles (reduces overhead by ~3%)cpu: Collect only CPU profilesIntegration:
/e2e-profile start/stop/run/analyze/compare)Key Implementation Details:
Code Quality:
Testing:
Verified end-to-end with
make test-e2e:This tooling was essential for identifying memory optimization opportunities
and validating that alert thresholds are correctly calibrated.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Signed-off-by: Todd Short tshort@redhat.com
Reviewer Checklist