Add cassandra profile command with async-profiler pass-through to Pyroscope

[rustyrazorblade]

Adds a profile cassandra command that runs async-profiler on all Cassandra nodes and sends flamegraph data to Pyroscope.

Changes

• packer/cassandra/bin/flamegraph-to-pyroscope: script wrapping asprof + Pyroscope HTTP ingest
• packer/cassandra/services/flamegraph-cassandra.service: SystemD one-shot unit for standalone use
• packer/cassandra/install/install_flamegraph_service.sh: packer install script
• Event.Profiling.* events added to Event.kt
• ProfilingService and DefaultProfilingService
• profile cassandra [asprof-options] command with full pass-through via @unmatched

Closes #159

Generated with Claude Code

[claude]

test

[github-actions]

Code Coverage

File	Coverage [59.15%]	❌
src/main/kotlin/com/rustyrazorblade/easydblab/commands/cassandra/Cassandra.kt	33.33%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/commands/cassandra/Profile.kt	33.33%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/commands/cassandra/ProfileStart.kt	44.44%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/commands/cassandra/ProfileStop.kt	0.00%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/events/Event.kt	59.11%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/services/ProfilingService.kt	100.00%	✅
src/main/kotlin/com/rustyrazorblade/easydblab/services/ServicesModule.kt	54.67%	❌

Total Project Coverage	67.07%	❌

[claude]

Code Review - Full Review Posted Below

[claude]

Part 1: Shell argument injection - The @unmatched arguments are joined into a raw command string sent over SSH. A user-supplied value like semicolon-rm-rf-slash would execute on the remote host. Args should be validated or risk documented.

[claude]

Part 2: No tests - ProfilingService has real logic: it emits events, builds a command string, calls remote ops, and propagates errors via Result. Per project conventions, non-trivial code requires tests. A DefaultProfilingServiceTest could verify events order, Result.failure propagation, and command string assembly.

[claude]

NOTE: Ignore the partial comments below -- full review follows above (comment was split due to tooling constraints).

[claude]

Part 3: Missing Event.Profiling.Error - runCatching returns Result.failure with no error event emitted. The caller calls .getOrThrow(), so failures surface as unformatted stack traces. Add Event.Profiling.Error(host, message) and emit it in the catch path.

[claude]

Part 4: error() bypasses event bus - Per CLAUDE.md all user-facing output must go through eventBus.emit(). error() throws IllegalStateException with a raw exception message. This should be an Event.Profiling.Error or shared cluster-level event. Part 5: Sequential execution - profiling blocks per node (default 30s), 3+ minutes on a 6-node cluster. Consider parallel execution as done elsewhere in the codebase.

[claude]

Part 6: Shell script unquoted variable in trap - OUTFILE is expanded at trap definition time when using double quotes. Use single quotes so the variable is expanded at trap execution time. Part 7: flamegraph-cassandra.service missing [Install] section - no [Install] stanza means systemctl enable silently does nothing. Add a comment if exclusive CLI trigger is intended. Minor: install_flamegraph_service.sh uses emoji in echo output (inconsistent with other scripts). Profile parent command KDoc lists subcommands inline which will drift; the annotation is the authoritative list.

[claude]

Code Review

Overall this is a clean, well-structured addition. Layering is right (Command to Service to RemoteOps), events are properly typed, Koin registration follows established patterns. A few issues worth addressing before merge:

1. Shell argument injection (ProfilingService.kt lines 39-40)
@unmatched args are joined into a raw SSH command string with no sanitization. A value like ; rm -rf / would execute on the remote host. Args should be validated (allowlist -d, -e, -t, -i and their values) or the risk documented explicitly.

2. No tests
ProfilingService has real logic: emits events, builds a command string, calls remote ops, propagates errors via Result. Per project conventions this requires tests. A DefaultProfilingServiceTest with mocked RemoteOperationsService and EventBus should verify: events emitted in correct order on success; Result.failure returned when executeRemotely throws; correct command string assembled from args.

3. Missing Event.Profiling.Error event
When executeRemotely fails, runCatching returns Result.failure with no error event emitted. The caller calls .getOrThrow() so failures surface as an unformatted stack trace. Add Event.Profiling.Error(host, message) emitted in the catch path, consistent with how other services handle failures.

4. error() in ProfileCassandra bypasses the event bus
Per CLAUDE.md all user-facing output must go through eventBus.emit(Event.Domain.Type(...)). The error() call in ProfileCassandra throws IllegalStateException producing a raw exception message. This should be Event.Profiling.Error or a shared cluster-level event.

5. Sequential execution
Profiling blocks for -d seconds per node (default 30s). On a 6-node cluster that is 3 minutes minimum. Consider running hosts in parallel -- the pattern is used elsewhere in the codebase for multi-host operations.

6. Shell script: unquoted variable in trap
flamegraph-to-pyroscope uses double-quoted trap which expands OUTFILE at definition time. Safer form: trap with single quotes around the rm command.

7. flamegraph-cassandra.service missing [Install] section
No [Install] stanza means systemctl enable silently does nothing. If this unit is exclusively triggered remotely by the CLI, add a comment making that intent explicit.

Minor: install_flamegraph_service.sh uses an emoji in echo output, inconsistent with other install scripts. The Profile parent command KDoc lists subcommands inline -- this will drift as more are added; the subcommands annotation is the authoritative list.

[rustyrazorblade]

@claude cassandra profile would be a better command. each of the dbs can have their own profile subcommand, as it gets support in the tooling, instead of profile top level command.

[claude]

Claude finished @rustyrazorblade's task in 2m 54s —— View job

---

Restructuring profile cassandra → cassandra profile

• Read current command structure
• Find existing cassandra command group
• Create commands/cassandra/Profile.kt (moved + renamed from commands/profile/ProfileCassandra.kt)
• Add Profile::class to Cassandra.kt subcommands
• Remove top-level Profile from CommandLineParser.kt
• Delete old profile/ package files

Done. The command is now cassandra profile [asprof-options]. Changes in commit d65608a:

• commands/profile/ProfileCassandra.kt → commands/cassandra/Profile.kt (package updated, class renamed to Profile, command name changed to profile, KDoc examples updated)
• commands/profile/Profile.kt deleted (top-level parent command removed)
• Cassandra.kt — added Profile::class to subcommands
• CommandLineParser.kt — removed Profile import and top-level registration

Other database types (ClickHouse, OpenSearch, etc.) can now add their own profile subcommand under their respective parent command groups.
|

[claude]

Code Review: Overall this is a clean well-structured addition. A few issues worth addressing before merging. Bug: curl silently swallows HTTP errors - curl -s does not fail on non-2xx responses so a 400 or 500 still exits 0. Add -f flag. Security: Shell injection in ProfilingService.kt - user args are joined and interpolated into remote command string without sanitization. Missing Pyroscope timestamp parameters - ingest URL needs from/until Unix seconds. APP_NAME does not distinguish nodes - all nodes share the same label. No tests for ProfilingService. Sequential profiling scales poorly for large clusters. Minor: [Install] section missing from systemd unit. Minor: profileCassandra method name should be profileNode or profileDatabase per CLAUDE.md naming guidance.

[rustyrazorblade]

@claude address the review feedback

[rustyrazorblade]

@claude address the review feedback and make sure this runs as part of bin/end-to-end-test

[claude]

Claude finished @rustyrazorblade's task in 6m 50s —— View job

---

Review Feedback Addressed

All items from the code review have been fixed in commit 9c573c7:

Shell script (flamegraph-to-pyroscope)

• Added -f to curl so non-2xx responses fail the script instead of silently succeeding
• Fixed trap quoting: trap 'rm -f "$OUTFILE"' EXIT (single quotes, $OUTFILE now expands at execution time not definition time)
• Added from/until Unix timestamps to the Pyroscope ingest URL
• APP_NAME now includes $(hostname) so each node's profile is stored separately in Pyroscope

flamegraph-cassandra.service: Added comment explaining the intentional absence of [Install] section

install_flamegraph_service.sh: Removed emoji from echo output to match other install scripts

Event.kt: Added Event.Profiling.Error(host, message) with isError() = true

ProfilingService.kt

• Renamed profileCassandra → profileNode
• Shell-quotes each arg before embedding in the remote command string (prevents injection like ; rm -rf /)
• Emits Event.Profiling.Error on failure via .onFailure { } — errors surface through the event bus instead of as raw stack traces

Profile.kt

• Replaced error() (which bypassed the event bus) with eventBus.emit(Event.Profiling.Error(...))
• Injected HostOperationsService and runs profiling across nodes in parallel with parallel = true

ProfilingServiceTest.kt (new): Tests for event ordering on success, error event + Result.failure on SSH failure, command string assembly, and shell-quoting of args with special characters

bin/end-to-end-test: Added step_cassandra_profile step (runs cassandra profile -d 10) gated on --cassandra flag
| Branch

[claude]

PR Review: cassandra profile command

Overall this is a clean, well-structured addition. The layered design (shell script to service to Kotlin service to command) is solid, events follow project conventions, and the tests cover meaningful behavior. A few things to address:

Security: Shell command construction in ProfilingService

The shellQuote() implementation is correct and handles single-quote escaping and the empty-string case properly. However, if RemoteOperationsService.executeRemotely accepts an argument list rather than a shell string, prefer that -- it eliminates the quoting step entirely and removes the risk of edge-case gaps in the allowlist. Worth checking whether that API surface exists or could be added.

If the string approach stays, the allowlist regex [a-zA-Z0-9_./:=@%+,-]+ is reasonable for asprof args. Minor readability: the hyphen is fine at the end of the character class, but moving it to the front would make intent clearer.

Duration inconsistency between CLI and SystemD unit

flamegraph-cassandra.service hardcodes -d 30, but when the CLI is invoked as cassandra profile with no args the duration falls back to asprof built-in default (10 seconds). A user running cassandra profile with no -d flag gets a 10 s profile; the standalone service gives 30 s. Consider:

• Defaulting the CLI to 30 s via an explicit -d option with a default value
• Or documenting the discrepancy in the command description
• Or updating the service unit to omit -d 30 so both paths use the same default

Result from profileNode is not used in Profile.execute()

The Result is discarded -- errors are handled via event emission in onFailure, which follows project conventions. But if every node fails the command exits 0 with no final summary. Consider emitting a summary event when any nodes failed, so the user knows the overall outcome without scanning per-node output.

Tests

The test suite is solid -- it covers event ordering, command string assembly, shell quoting, and error propagation. Mocking RemoteOperationsService is appropriate given the real SSH side effects.

One potential fragility: additionalTestModules() is called during Koin setup (base-class @beforeeach), which runs before setupMocks() sets mockRemoteOps. The single lambda works correctly only because Koin evaluates single lazily and the first get() call happens after setupMocks(). This is fine in practice but worth a brief comment to make the ordering assumption explicit.

Minor

• flamegraph-cassandra.service has no [Install] section and the comment explains why -- good. Confirm the file ends with a newline (project convention).
• install_flamegraph_service.sh calls systemctl daemon-reload but does not enable the unit, which is correct and intentional.
• The CI coverage report shows ProfilingService.kt at 0 % -- this looks like a timing artifact from an earlier CI run. The test file is present in the current diff and covers the service well.

[rustyrazorblade]

@claude it doesn't look like this will work for continuous profiling. Script uploads after everything is done. I need this to be started then run for hours, and get the data pushed to pyroscope once a minute

[claude]

Claude finished @rustyrazorblade's task in 7m 18s —— View job

---

Redesigned for Continuous Profiling

All tasks complete — commit 74b9c10.

• Read current implementation files
• Rewrite flamegraph-to-pyroscope as a continuous loop
• Update systemd service to Type=simple
• Add Profiling.Started / Stopping / Stopped events to Event.kt
• Rewrite ProfilingService with startProfiling / stopProfiling
• Convert Profile.kt to parent command; add ProfileStart.kt + ProfileStop.kt
• Update ProfilingServiceTest.kt
• Update bin/end-to-end-test step

What changed

flamegraph-to-pyroscope — Infinite loop: profile for FLAMEGRAPH_INTERVAL seconds (default 60), upload collapsed stacks to Pyroscope with from/until timestamps, repeat until SIGTERM. Extra asprof options (-e alloc, -t, etc.) come from FLAMEGRAPH_EXTRA_ARGS in /etc/default/flamegraph-cassandra.

flamegraph-cassandra.service — Changed from Type=oneshot to Type=simple with Restart=on-failure. Loads both /etc/default/cassandra (Pyroscope address) and /etc/default/flamegraph-cassandra (interval + extra args).

Command structure — cassandra profile is now a parent command:

• cassandra profile start [-e alloc] [-t] ... — writes env file with extra asprof args, starts the systemd service on all nodes in parallel
• cassandra profile stop — stops the service on all nodes in parallel

ProfilingService — profileNode replaced with startProfiling (env file write + systemctl start) and stopProfiling (systemctl stop).

Event.kt — Profiling.Complete replaced with Started, Stopping, and Stopped.

bin/end-to-end-test — step_cassandra_profile now runs profile start, waits 65 seconds for one full upload cycle, then profile stop.
| Branch

[claude]

PR Review: Add cassandra profile command with async-profiler pass-through to Pyroscope

Overall this is a well-structured addition. The event hierarchy, PicoCLI patterns, and test organization all follow project conventions. A few issues worth addressing before merge.

---

Issues

1. ProfilingService should extend AbstractSystemDServiceManager (medium)

The codebase has a well-established pattern that all other systemd-backed services use (CassandraService, SidecarService, etc.), documented in services/CLAUDE.md. DefaultProfilingService reimplements start/stop logic from scratch instead of following it.

DefaultProfilingService should extend AbstractSystemDServiceManager("flamegraph-cassandra", ...) and ProfilingService should extend SystemDServiceManager. The only custom behavior needed is writing the env file before start, which can be handled by overriding start(). This also gives you isRunning() / getStatus() for free, which would be useful for diagnosing whether profiling is actually active on a node.

2. Shell robustness: curl -sf combined with set -e (medium)

In flamegraph-to-pyroscope, set -e means a failed curl (Pyroscope unreachable, transient network issue) kills the entire script. systemd's Restart=on-failure will recover it after 5 seconds, but this drops the current profiling cycle entirely. Wrap the upload:

curl -sf ... || { echo "Upload failed, continuing"; }

or remove -f so HTTP errors don't trigger script exit.

3. Shared mutable list passed to parallel operations (low)

In ProfileStart.kt, profilerArgs: MutableList<String> is passed to startProfiling inside withHosts(..., parallel = true). Nothing modifies the list today, but this is a fragile contract. Use profilerArgs.toList() at the call site or type the parameter as List<String> to make immutability explicit.

4. Trivial test adds no value (low)

@Test
fun `Error event is marked as error`() {
    val errorEvent = Event.Profiling.Error("db0", "something went wrong")
    assertThat(errorEvent.isError()).isTrue()
}

Per project conventions, tests that only verify a constant override (override fun isError() = true) prove nothing a compiler check wouldn't also catch. The failure-path tests already confirm Error events are emitted under failure conditions. Remove this or replace with something that exercises real logic.

5. E2E test has a hardcoded 65-second sleep (low)

Consider setting FLAMEGRAPH_INTERVAL=5 in the test environment so the sleep can drop to ~10s and the E2E suite doesn't slow down unnecessarily.

---

Positive Notes

• @Unmatched for asprof pass-through args is exactly the right PicoCLI pattern.
• shellQuote() is a solid implementation for preventing injection in remote commands.
• Event.Profiling is well-structured with correct @Serializable/@SerialName annotations, isError(), and toDisplayString().
• Tests use AssertJ, extend BaseKoinTest, cover both success and failure paths — all per conventions.
• parallel = true on withHosts for start/stop is the right call for multi-node clusters.
• Koin registration as factoryOf (not singleton) is correct for a per-invocation concern.
• The packer install script is clean and consistent with others in the repo.

[rustyrazorblade]

@claude address the review feedback

[claude]

Claude finished @rustyrazorblade's task in 6m 21s —— View job

---

Review Feedback Addressed

All items from the latest code review have been fixed in commit 94c6b79.

ProfilingService → AbstractSystemDServiceManager (medium)

• ProfilingService now extends SystemDServiceManager; DefaultProfilingService extends AbstractSystemDServiceManager("flamegraph-cassandra", ...), gaining isRunning(), getStatus(), and restart() for free
• stopProfiling() removed — callers use the standard stop(), which is overridden to emit Event.Profiling.* events
• ProfileStop.kt updated to call profilingService.stop()

curl -sf + set -e robustness (medium)

• Wrapped the curl upload with || echo "Upload failed..." so a transient Pyroscope outage logs a message and continues the loop instead of killing the service

Mutable list in parallel ops (low)

• profilerArgs.toList() captured before withHosts(..., parallel = true) so the immutable copy is shared across goroutines

Trivial test removed (low)

• Error event is marked as error test deleted

E2E hardcoded sleep (low)

• Added --interval option to ProfileStart (written to /etc/default/flamegraph-cassandra as FLAMEGRAPH_INTERVAL=N)
• E2E step now uses cassandra profile start --interval 5 + sleep 15 instead of the hardcoded 65-second wait
• Two new tests cover the interval env-file write (interval alone, and interval + args combined)
  | Branch

[rustyrazorblade]

This is not using continuous mode correctly. It should call async profiler with --loop as per the doc:

https://github.com/async-profiler/async-profiler/blob/master/docs/ProfilerOptions.md

[rustyrazorblade]

@claude see my comment ^^

[claude]

Claude finished @rustyrazorblade's task in 4m 23s —— View job

---

Fixed in commit c33db2d. Here's what changed in flamegraph-to-pyroscope:

Before: The script called asprof -d $INTERVAL in a shell while true loop. This blocked for the full interval each time and didn't use async-profiler's continuous profiling support.

After: asprof is started once with --loop ${INTERVAL}s, which is async-profiler's native continuous profiling mode — it profiles for the interval, writes the output file, resets, and immediately starts the next interval without stopping. The script runs a polling loop to pick up completed files and upload them to Pyroscope.

Key design points:

• --loop ${INTERVAL}s -o collapsed -f ${OUTDIR}/profile-%t.collapsed — async-profiler writes one file per interval, each with a unique %t timestamp in the name
• File mtime is used as the until timestamp for Pyroscope; from = mtime - INTERVAL
• cleanup() sends SIGTERM to asprof and uploads any remaining files before the service exits
• EXTRA_ARGS array now safely handles the case where FLAMEGRAPH_EXTRA_ARGS is unset (avoids empty element in previous EXTRA_ARGS=($VAR) form)
  | Branch

[rustyrazorblade]

@claude like other services, this should have a start / stop and since it uses the loop, run in the background.

The end to end test should do a 5 second profile only, using -e alloc

[claude]

Claude finished @rustyrazorblade's task in 2m 44s —— View job

---

Changes applied

• Read current ProfileStart.kt, ProfileStop.kt, ProfilingService.kt
• Understand how other services handle background start
• Confirm cassandra profile start is already non-blocking (systemd Type=simple — systemctl start returns as soon as the process is forked)
• Update E2E test to use --interval 5 -e alloc
• Commit and push changes

Commit a4e0e0f — bin/end-to-end-test:

• Added -e alloc to cassandra profile start --interval 5
• Reduced sleep from 15s → 10s (one 5-second cycle + 5s margin is sufficient)

The start/stop via systemd is already the right pattern. Since the service is Type=simple, systemctl start returns as soon as the process is forked — the profiler loop runs in the background until profile stop is called. No structural changes needed.
| Branch