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.32%]	❌
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	36.36%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/commands/cassandra/ProfileStop.kt	0.00%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/commands/exec/ExecRun.kt	44.44%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/events/Event.kt	59.27%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/services/ProfilingService.kt	100.00%	✅
src/main/kotlin/com/rustyrazorblade/easydblab/services/ServicesModule.kt	56.58%	❌
src/main/kotlin/com/rustyrazorblade/easydblab/shell/ShellUtils.kt	100.00%	✅

Total Project Coverage	67.94%	❌

[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

[claude]

PR Review: Add cassandra profile command

Overall this is well-structured work — the event design, shell injection protection, and parallel execution are all solid. A few things worth addressing.

---

Design: Ambiguous API from inheriting SystemDServiceManager

ProfilingService extends SystemDServiceManager, which means it exposes both start(host) (inherited) and startProfiling(host, args, interval). These have different behavior:

• start() from AbstractSystemDServiceManager emits Event.Service.Starting and does not write /etc/default/flamegraph-cassandra, so profiling starts with whatever env file is already on disk (or the defaults).
• startProfiling() writes the env file first, then starts the service.

A caller using start() instead of startProfiling() won't get the configured interval or extra args, and will silently get wrong behavior. The KDoc comment says this inheritance is for isRunning()/getStatus() access — consider whether those can be exposed more directly (e.g., via delegation or separate methods) without exposing the misleading start().

---

Test: Mock-echo test

stop should execute systemctl stop command only verifies that a mock was called with a specific command string. Per the project's CLAUDE.md rule:

Tests that only verify a mock was called with the same values that were set up — this proves nothing.

The command string "sudo systemctl stop flamegraph-cassandra" is just a constant — there's no transformation or decision being tested here. This test can be dropped. The stop should emit Stopping then Stopped events on success test already covers the stop path meaningfully.

---

Silently ignored Result from service calls

In both ProfileStart and ProfileStop, the Result<Unit> from startProfiling/stop is returned into withHosts's action: (ClusterHost) -> Unit lambda, which discards it:

hostOperationsService.withHosts(clusterState.hosts, ServerType.Cassandra, parallel = true) { host ->
    profilingService.startProfiling(host.toHost(), args, loopInterval)  // Result<Unit> silently dropped
}

The onFailure handler in the service emits Event.Profiling.Error so failures aren't invisible to the user — but the loop continues even after a host fails, with no way for the command to know. This may be intentional (best-effort on all nodes) but worth a comment to make it explicit.

---

flamegraph-to-pyroscope not executable in git

The script is committed with mode 100644. The install script does chmod +x, so it works in practice, but it is cleaner to set 100755 in git to match the other scripts in packer/cassandra/bin/. Fix with:

git update-index --chmod=+x packer/cassandra/bin/flamegraph-to-pyroscope

---

Minor: env file is fully overwritten each call

If a user calls profile start --loop-interval 30 followed by profile start -e alloc (no interval), the second call overwrites the env file with only FLAMEGRAPH_EXTRA_ARGS=-e alloc, silently losing the previous FLAMEGRAPH_INTERVAL=30. Fine for ephemeral clusters but worth a comment.

---

What looks good

• shellQuote() properly guards against injection when building the remote printf command.
• Event.Profiling.* events are domain-specific — correct compliance with the event bus rules.
• The @Unmatched pass-through for asprof args is a clean approach.
• Tests for the shell-quoting logic (e.g., startProfiling with args should write env file with shell-quoted args) genuinely test the quoting transformation and are meaningful.
• The flamegraph script's cleanup trap correctly uploads any in-progress JFR files before exiting.

[claude]

PR Review: Add cassandra profile command

This is a clean, well-structured feature. The layered design (shell script → systemd-run → ProfilingService → command) is sound, and the refactoring of shellQuote to a shared shell package is a nice cleanup. A few things worth addressing:

Bug: Wrong Content-Type for binary JFR data

In flamegraph-to-pyroscope, the upload uses Content-Type: text/plain for JFR files, which are binary. Pyroscope's JFR ingest endpoint expects application/octet-stream. Using text/plain may cause silent parse failures or malformed flamegraphs even when the upload returns 200.

Potential race: polling may upload a file asprof is still writing

The polling loop wakes every 5 seconds and uploads anything matching profile-*.jfr. If asprof writes the interval file in-place rather than via atomic rename, a poll mid-write sends a partial JFR that Pyroscope will reject or render incorrectly. If asprof --loop guarantees atomic rename before the file appears, document that assumption in the script. If not, consider checking file stability (unchanged size + mtime) before uploading.

isRunning is in the interface but unused

ProfilingService.isRunning is defined, implemented, and tested, but neither ProfileStart nor ProfileStop calls it. Either remove it until a cassandra profile status subcommand is added, or add that subcommand now. Unused interface methods add mock boilerplate for no current value.

No user-friendly message when profiling is already running

systemd-run --unit=flamegraph-cassandra fails if the unit already exists. That failure surfaces as Event.Profiling.Error with a raw systemd message. Consider detecting this case and emitting something like "Profiling is already running on $host — use cassandra profile stop first."

Minor: test casts to DefaultProfilingService to access internal method

The BuildSystemdRunCommand nested tests do getKoin().get<ProfilingService>() as DefaultProfilingService to access buildSystemdRunCommand. Instantiating DefaultProfilingService directly in those tests avoids the cast and the coupling to Koin. Current approach is acceptable — just noting it.

Nits

• shellQuote allows % through unquoted — intentional for asprof format tokens like %t, but worth a brief comment since % is special in some shell contexts.
• flamegraph-to-pyroscope falls back silently to localhost:4040 if PYROSCOPE_SERVER_ADDRESS is not set in /etc/default/cassandra. A warning when the address is missing would help debug misconfigured nodes.

---

The Content-Type issue is the most likely to cause silent data loss; worth fixing before merge. Everything else is solid — good event coverage, clean command assembly logic, and the shellQuote refactor is the right call.

[claude]

PR Review: Add cassandra profile command

This is a solid, well-structured feature. The architecture is clean and follows established patterns. A few things to address before merge:

---

Functional Gap: Missing Packer Install Script

The PR description lists packer/cassandra/install/install_flamegraph_service.sh and packer/cassandra/services/flamegraph-cassandra.service as changes, but neither appears in the diff. The flamegraph-to-pyroscope script needs to land at /usr/local/bin/flamegraph-to-pyroscope on the AMI — without an install step, the command will fail at runtime with a path-not-found error.

The script also depends on cassandra-pid and asprof at /usr/local/async-profiler/bin/asprof. Are these already on the AMI via existing packer scripts? If so, that should be noted. If not, the install script is required for this to work end-to-end.

---

Missing Docs Update

CLAUDE.md requires docs to be updated for user-facing changes. The new cassandra profile start/stop commands should have an entry in docs/.

---

stop Doesn't Handle "Not Running" Gracefully

In ProfilingService.kt, the stop() method calls systemctl stop without first checking whether the unit is running. If profiling was never started, systemctl stop on a non-existent transient unit returns a non-zero exit, surfacing as a raw error. Consider pre-checking with isRunning() and emitting a friendlier NotRunning event, symmetric with how startProfiling handles AlreadyRunning.

---

Minor: output = false in isRunning — Good, But Worth a Comment

The output = false on the systemctl is-active call in isRunning() suppresses console noise from the internal check — that's correct. A brief inline comment would clarify the intent for future readers.

---

What's Working Well

• shellQuote extraction to shell/ShellUtils.kt is a clean improvement — now reusable and tested in its own class rather than buried as an internal extension in ExecRun.
• @Unmatched pass-through for asprof args is the right approach and well-documented in the command docstring.
• isRunning check before systemd-run prevents confusing systemd errors when the unit already exists. Good defensive design.
• Event hierarchy follows the Event.Profiling.* sealed interface pattern correctly, with isError() overrides on the appropriate types.
• Test coverage in ProfilingServiceTest and BuildSystemdRunCommandTest is meaningful — tests actual behavior (command string construction, event sequencing, AlreadyRunning guard) rather than just mock invocations.