Add generated ABI contract for schema and eBPF#3
Merged
Conversation
📝 WalkthroughWalkthroughThe change centralizes tracepoint, SQLite schema, and kernel event definitions in ChangesABI centralization and generated artifacts
Estimated code review effort: 5 (Critical) | ~90 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/abi/abi.yaml (1)
360-386: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPersist
bytesinmetadata.convertKernelEventalready serializessyscall_nr,fd, andaux, butbytesis dropped before storage. Add it toMetadata(orRaw) so the I/O byte count isn’t lost.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/abi/abi.yaml` around lines 360 - 386, Update the metadata/raw representation used by convertKernelEvent to include the kernel_event bytes field, and serialize its value alongside syscall_nr, fd, and aux before storage. Preserve the existing byte count without changing handling of the other event fields.internal/daemon/service.go (1)
108-127: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDrain path missing
SchemaVersionassignment.The normal ingest path (line 127) sets
ev.SchemaVersion = abi.SchemaVersion, but the shutdown drain path (lines 108–118) appends events to the batch without this assignment. Events arriving during shutdown will haveSchemaVersion = 0instead of1, which could break downstream consumers that check schema compatibility.🐛 Proposed fix: set SchemaVersion in the drain path
case ev, ok := <-ingest: if !ok { drained = true } else { + ev.SchemaVersion = abi.SchemaVersion batch = append(batch, ev) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/service.go` around lines 108 - 127, Update the shutdown drain loop in the ingest handling flow to assign abi.SchemaVersion to each event before appending it to batch, matching the assignment in the normal case ev path. Ensure events drained during shutdown carry the same schema version as regular ingested events.
🧹 Nitpick comments (1)
internal/storage/schema_gen_test.go (1)
20-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGood schema consistency test.
The
schemaDocColumnsparser correctly extracts column names fromabi.SchemaDocby filtering lines prefixed with two spaces and taking the first field. TheIndexes:line and blank lines are naturally excluded since they don't start with" ".The errcheck lint findings on
store.Close()(line 25) androws.Close()(line 31) are common in Go test code. If the project enforces errcheck in CI, consider wrapping witht.Cleanup(func() { _ = store.Close() })as done ininternal/diff/engine_test.go:19.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/schema_gen_test.go` around lines 20 - 74, Update TestSchemaMatchesGenerated cleanup to handle Close errors through t.Cleanup callbacks that explicitly discard the results of store.Close and rows.Close, matching the project’s established test pattern and satisfying errcheck.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/abi/capture.md`:
- Around line 29-36: Update the “Source of truth” section in capture.md to
identify only internal/abi/abi.yaml as the ABI source input. Move
collector_bpfel.o, kernel_event.gen.h, and tracepoints_gen.go into a
generated-artifacts list, while retaining collector.bpf.c and real_linux.go
under the appropriate source inputs category.
---
Outside diff comments:
In `@internal/abi/abi.yaml`:
- Around line 360-386: Update the metadata/raw representation used by
convertKernelEvent to include the kernel_event bytes field, and serialize its
value alongside syscall_nr, fd, and aux before storage. Preserve the existing
byte count without changing handling of the other event fields.
In `@internal/daemon/service.go`:
- Around line 108-127: Update the shutdown drain loop in the ingest handling
flow to assign abi.SchemaVersion to each event before appending it to batch,
matching the assignment in the normal case ev path. Ensure events drained during
shutdown carry the same schema version as regular ingested events.
---
Nitpick comments:
In `@internal/storage/schema_gen_test.go`:
- Around line 20-74: Update TestSchemaMatchesGenerated cleanup to handle Close
errors through t.Cleanup callbacks that explicitly discard the results of
store.Close and rows.Close, matching the project’s established test pattern and
satisfying errcheck.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bdfe172-29d1-4d30-b367-8c40a94cd939
⛔ Files ignored due to path filters (3)
go.sumis excluded by!**/*.suminternal/abi/gen/main.gois excluded by!**/gen/**internal/ebpf/collector_bpfel.ois excluded by!**/*.o
📒 Files selected for processing (31)
.github/workflows/ci.ymlAGENTS.mdMakefileREADME.mddocs/README.mddocs/abi/README.mddocs/abi/capture.mddocs/abi/schema.mddocs/architecture.mddocs/querying.mddocs/recording.mdgo.modinternal/abi/abi.yamlinternal/abi/gen.gointernal/abi/kernel_event.gen.hinternal/abi/schema_gen.gointernal/abi/tracepoints_gen.gointernal/agent/agent.gointernal/cli/cli.gointernal/daemon/service.gointernal/diff/engine_test.gointernal/ebpf/collector.bpf.cinternal/ebpf/collector_bpfel.gointernal/ebpf/collector_contract_test.gointernal/ebpf/gen.gointernal/ebpf/real_linux.gointernal/query/engine_test.gointernal/storage/schema_gen.gointernal/storage/schema_gen_test.gointernal/storage/store.gointernal/storage/types.go
💤 Files with no reviewable changes (1)
- internal/storage/types.go
parseGlobalFlags now accepts single-dash flags and stops scanning at the first unrecognized token instead of scanning the whole arg list, so flag-shaped words inside free-text query/SQL input are no longer mistaken for global flags. Global flags are now stripped from os.Args before cobra ever runs, since pflag doesn't understand "-longname" the way the old stdlib parser did. Also: stop advertising --version as an inherited subcommand flag (it never worked there), reject non-positive --interval in `watch` instead of panicking, drop the dead printRootHelp, point AGENTS.md/capture.md at the real eBPF loader files instead of the deleted real_linux.go/ real_stub.go, and cross-compile for darwin in CI now that macOS runners are gone.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
internal/ebpf/collector_linux.go (1)
93-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilently dropped decode errors reduce observability.
If the eBPF struct layout drifts from the Go decoder, every event fails to decode with no visible indication. Consider adding a counter or periodic stderr log for decode failures to aid production debugging.
♻️ Suggested improvement: add a decode-error counter
var dropped uint64 + var decodeErrs uint64 go func() { <-ctx.Done() _ = reader.Close() }() for { record, err := reader.Read() if err != nil { if errors.Is(err, perf.ErrClosed) { + if decodeErrs > 0 { + fmt.Fprintf(os.Stderr, "ltm: ebpf collector failed to decode %d events\n", decodeErrs) + } return nil } return err } if record.LostSamples != 0 { dropped += record.LostSamples continue } ke, err := decodeKernelEvent(record.RawSample) if err != nil { + decodeErrs++ continue }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/ebpf/collector_linux.go` around lines 93 - 96, Instrument decode failures in the event-processing loop around decodeKernelEvent by incrementing a dedicated decode-error counter, and expose or periodically report that counter through the collector’s existing observability mechanism. Preserve the current continue behavior so malformed events remain skipped while failures become visible in production.internal/storage/write.go (1)
29-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSatisfy
errcheckon the deferredRollback/Close.golangci-lint reports these as errors.
defer tx.Rollback()anddefer stmt.Close()are idiomatic, but to keep the lint gate green, ignore the returns explicitly.♻️ Explicitly discard the deferred error returns
- defer tx.Rollback() + defer func() { _ = tx.Rollback() }()- defer stmt.Close() + defer func() { _ = stmt.Close() }()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/write.go` around lines 29 - 35, Update the deferred cleanup calls in the insert flow around tx and stmt to explicitly discard their error returns, satisfying errcheck while preserving the existing rollback and close behavior.Source: Linters/SAST tools
internal/cli/commands_runtime.go (1)
38-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
noctxhere is a deliberate design choice — suppress it to keep CI green.The recorder is intentionally detached (
Setsid) so it outlives the launching process; switching toexec.CommandContextwould tie its lifetime to a cancelable context and kill it prematurely. Add a scoped//nolintwith rationale so the lint error doesn't block the pipeline.♻️ Document the intentional detachment
- cmd := exec.Command(exe, daemonArgs(cfg)...) + // The daemon is detached (Setsid) to outlive this process, so it must not be + // bound to a cancelable context. + cmd := exec.Command(exe, daemonArgs(cfg)...) //nolint:noctx // detached long-lived daemon🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/commands_runtime.go` around lines 38 - 44, Add a scoped noctx lint suppression at the exec.Command call in the runtime command launch flow, with a brief rationale that Setsid intentionally detaches the recorder and it must outlive the launching process; preserve exec.Command rather than switching to a context-bound variant.Source: Linters/SAST tools
internal/collector/filter.go (1)
17-19: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAvoid appending onto the shared
defaultIgnorePathsslice.
append(defaultIgnorePaths, extra...)reuses the package-level backing array whenever it has spare capacity, which can silently corrupt the defaults across callers. It's safe today only because the literal'slen == cap; copying into a fresh slice removes that latent aliasing hazard.♻️ Build a fresh slice
func newIgnoreRules(extra []string) []string { - return normalizeIgnoreRules(append(defaultIgnorePaths, extra...)) + combined := make([]string, 0, len(defaultIgnorePaths)+len(extra)) + combined = append(combined, defaultIgnorePaths...) + combined = append(combined, extra...) + return normalizeIgnoreRules(combined) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/collector/filter.go` around lines 17 - 19, Update newIgnoreRules to build a fresh slice containing defaultIgnorePaths before appending extra, rather than appending directly to the shared package-level slice. Preserve the existing normalizeIgnoreRules behavior and returned rule ordering while preventing backing-array aliasing across callers.internal/daemon/flush.go (1)
56-56: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the redundant
SchemaVersionwrite
InsertEventsnever storesSchemaVersion, andscanEventrestores it fromabi.SchemaVersionon read, so this assignment only touches the in-memory batch and can be dropped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/flush.go` at line 56, Remove the redundant ev.SchemaVersion assignment from InsertEvents, leaving scanEvent’s restoration from abi.SchemaVersion unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/storage/maintenance.go`:
- Around line 23-28: Update the prune flow around the VACUUM execution in the
maintenance method to avoid running a full VACUUM unconditionally after every
prune. Prefer the supported incremental_vacuum strategy when the database is
configured with auto_vacuum=INCREMENTAL, or gate full VACUUM behind an explicit
opt-in or periodic maintenance condition while preserving the zero-row early
return and prune result handling.
In `@internal/storage/open.go`:
- Around line 38-43: Update the schema initialization loop in Open to call
db.ExecContext with context.Background() instead of db.Exec, preserving the
existing error cleanup and wrapping behavior so initialization remains
cancellable-compatible and satisfies the noctx linter.
In `@internal/storage/query.go`:
- Around line 65-92: Remove the one-off EventsByPID, EventsByPath, and
EventsBetween helpers from the Store API, and extend Filter to support their
query semantics. Reuse Query with Filter{PIDs: []int{pid}} and Filter{From:
from, To: to}; add an exact-path filter field for EventsByPath that preserves
path = ? OR old_path = ? behavior, distinct from PathLike. Keep EventsAfterID
and LatestEventID unchanged.
- Around line 239-268: Update execReadOnlySQL to wrap its context with the same
two-minute timeout used by the agent path before calling Store.RawSQL, ensuring
the CLI cannot run arbitrary read-only SQL indefinitely while preserving
RawSQL’s caller-provided context behavior.
---
Nitpick comments:
In `@internal/cli/commands_runtime.go`:
- Around line 38-44: Add a scoped noctx lint suppression at the exec.Command
call in the runtime command launch flow, with a brief rationale that Setsid
intentionally detaches the recorder and it must outlive the launching process;
preserve exec.Command rather than switching to a context-bound variant.
In `@internal/collector/filter.go`:
- Around line 17-19: Update newIgnoreRules to build a fresh slice containing
defaultIgnorePaths before appending extra, rather than appending directly to the
shared package-level slice. Preserve the existing normalizeIgnoreRules behavior
and returned rule ordering while preventing backing-array aliasing across
callers.
In `@internal/daemon/flush.go`:
- Line 56: Remove the redundant ev.SchemaVersion assignment from InsertEvents,
leaving scanEvent’s restoration from abi.SchemaVersion unchanged.
In `@internal/ebpf/collector_linux.go`:
- Around line 93-96: Instrument decode failures in the event-processing loop
around decodeKernelEvent by incrementing a dedicated decode-error counter, and
expose or periodically report that counter through the collector’s existing
observability mechanism. Preserve the current continue behavior so malformed
events remain skipped while failures become visible in production.
In `@internal/storage/write.go`:
- Around line 29-35: Update the deferred cleanup calls in the insert flow around
tx and stmt to explicitly discard their error returns, satisfying errcheck while
preserving the existing rollback and close behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4f9a5db-634b-44e2-ab3f-9122f840f92e
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (35)
.github/workflows/ci.ymlAGENTS.mdREADME.mddocs/README.mddocs/abi/README.mddocs/abi/capture.mddocs/abi/schema.mddocs/architecture.mddocs/querying.mddocs/recording.mdgo.modinternal/cli/cli.gointernal/cli/cli_test.gointernal/cli/commands_query.gointernal/cli/commands_runtime.gointernal/cli/flags.gointernal/cli/output.gointernal/collector/collector.gointernal/collector/filter.gointernal/daemon/flush.gointernal/daemon/service.gointernal/ebpf/attach_linux.gointernal/ebpf/collector_linux.gointernal/ebpf/collector_stub.gointernal/ebpf/decode_linux.gointernal/ebpf/proc_linux.gointernal/ebpf/real_linux.gointernal/ebpf/real_stub.gointernal/ebpf/source.gointernal/storage/demo.gointernal/storage/maintenance.gointernal/storage/open.gointernal/storage/query.gointernal/storage/store.gointernal/storage/write.go
💤 Files with no reviewable changes (3)
- internal/ebpf/real_stub.go
- internal/ebpf/real_linux.go
- internal/storage/store.go
✅ Files skipped from review due to trivial changes (8)
- internal/storage/demo.go
- docs/README.md
- internal/ebpf/source.go
- docs/abi/README.md
- docs/recording.md
- docs/architecture.md
- README.md
- docs/querying.md
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/ci.yml
- docs/abi/schema.md
- AGENTS.md
- internal/daemon/service.go
- docs/abi/capture.md
Comment on lines
+239
to
+268
| // RawSQL executes an arbitrary read-only query and returns column names plus | ||
| // rows as generic values, for `ltm sql`. Callers must use a read-only Store | ||
| // (query_only=ON) so writes fail at the SQLite layer. | ||
| func (s *Store) RawSQL(ctx context.Context, query string) ([]string, [][]any, error) { | ||
| if !s.readOnly { | ||
| return nil, nil, errors.New("raw SQL requires read-only store") | ||
| } | ||
| rows, err := s.db.QueryContext(ctx, query) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| defer rows.Close() | ||
| cols, err := rows.Columns() | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| var out [][]any | ||
| for rows.Next() { | ||
| vals := make([]any, len(cols)) | ||
| ptrs := make([]any, len(cols)) | ||
| for i := range vals { | ||
| ptrs[i] = &vals[i] | ||
| } | ||
| if err := rows.Scan(ptrs...); err != nil { | ||
| return nil, nil, err | ||
| } | ||
| out = append(out, vals) | ||
| } | ||
| return cols, out, rows.Err() | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
RawSQL has no execution timeout — a slow query can hang the CLI indefinitely.
RawSQL accepts arbitrary user SQL but passes through ctx without any deadline. In commands_query.go line 151, execReadOnlySQL calls it with context.Background(), so there's no upper bound. The agent path (line 103) correctly uses a 2-minute timeout.
Consider adding a default timeout in execReadOnlySQL or documenting that callers must supply a bounded context.
🔧 Proposed fix: add timeout in execReadOnlySQL
func execReadOnlySQL(cfg Config, query string, jsonOut bool) error {
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
store, err := storage.OpenReadOnly(cfg.DBPath)
if err != nil {
return err
}
defer store.Close()
- cols, rows, err := store.RawSQL(context.Background(), query)
+ cols, rows, err := store.RawSQL(ctx, query)
if err != nil {
return err
}🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 250-250: Error return value of rows.Close is not checked
(errcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/storage/query.go` around lines 239 - 268, Update execReadOnlySQL to
wrap its context with the same two-minute timeout used by the agent path before
calling Store.RawSQL, ensuring the CLI cannot run arbitrary read-only SQL
indefinitely while preserving RawSQL’s caller-provided context behavior.
…ACUUM, consolidate query helpers RawSQL executed arbitrary multi-statement input, so a query like "SELECT 1; PRAGMA query_only=OFF; DELETE FROM events" could flip off the read-only guard mid-script and delete rows through a "read-only" Store; RawSQL now rejects any input with more than one statement and caps results at 10k rows. Prune no longer runs a full VACUUM after every call (a blocking, whole-file rewrite) unless the caller opts in. EventsByPID/EventsByPath/EventsBetween are folded into Query via a Filter with ExactPath/PathLike (now also matching old_path) and OrderAsc, with basic range/limit validation. Open/OpenReadOnly resolve to an absolute path before building the file: DSN (a relative path was silently misparsed as a URI authority) and ping the connection so a bad DSN or permission problem surfaces at open time.
…add --vacuum query/engine.go and diff/engine.go move off the removed EventsByPID/ EventsByPath/EventsBetween onto storage.Query with Filter. execReadOnlySQL gets the same 2-minute timeout the agent path already had, closing a gap where a user-supplied `ltm sql`/`ltm query sql` could run unbounded. `ltm prune` gains an explicit --vacuum flag instead of always compacting. runWatch rejects a non-positive --interval instead of panicking in time.NewTicker.
…decode errors newIgnoreRules appended directly to the shared package-level defaultIgnorePaths slice instead of a copy, risking backing-array aliasing across calls. flushLoop set ev.SchemaVersion before insert, but the column isn't persisted and scanEvent recomputes it from abi.SchemaVersion on read anyway, so the write was a no-op. The eBPF collector loop now counts and logs malformed kernel events instead of silently continuing past them.
…erating Each file the ABI generator produces now carries a sha256 of its own body in its header comment, so a hand edit that bypasses the generator (and doesn't update the hash) is detectable without abi.yaml or a Go/BPF toolchain. `make verify-generated` and a matching unit test check the embedded hash against current file content. Also strengthens abi.yaml validation: rejects duplicate (group, event) tracepoint pairs, duplicate index names, duplicate kernel constants, and non-positive array counts (whether a literal or a constant reference), none of which were previously caught.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/storage/query.go (1)
273-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
depthtracking inhasMultipleStatementshas no effect on the result — likely dead code.
depthis incremented/decremented on(/)but never consulted by the;branch (or anywhere else), so it doesn't influence the return value for any input. The doc comment implies parenthesis-depth affects false-positive reduction, but no behavior actually depends on it — e.g. removing the'('/')'cases (falling through todefault) would produce identical results for every case inTestRawSQLRejectsMultipleStatements.If there's a scenario meant to be guarded (e.g. some construct where a
;should be tolerated only inside parens), it isn't implemented or tested; otherwise this can be simplified.♻️ Proposed simplification
func hasMultipleStatements(sql string) bool { - depth := 0 for i := 0; i < len(sql); { switch c := sql[i]; { case c == '\'': i = skipQuoted(sql, i+1, '\'') case c == '"': i = skipQuoted(sql, i+1, '"') case c == '[': i = skipUntil(sql, i+1, ']') case c == '-' && i+1 < len(sql) && sql[i+1] == '-': for i < len(sql) && sql[i] != '\n' { i++ } case c == '/' && i+1 < len(sql) && sql[i+1] == '*': i += 2 for i+1 < len(sql) && !(sql[i] == '*' && sql[i+1] == '/') { i++ } if i+1 < len(sql) { i += 2 } - case c == '(': - depth++ - i++ - case c == ')': - if depth > 0 { - depth-- - } - i++ case c == ';': if strings.TrimSpace(sql[i+1:]) != "" { return true } i++ default: i++ } } return false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/query.go` around lines 273 - 313, Remove the unused depth variable and the '(' and ')' cases from hasMultipleStatements, since they do not affect semicolon handling or the function result. Preserve the existing quote, bracket, comment, and trailing-statement detection behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/storage/query.go`:
- Around line 273-313: Remove the unused depth variable and the '(' and ')'
cases from hasMultipleStatements, since they do not affect semicolon handling or
the function result. Preserve the existing quote, bracket, comment, and
trailing-statement detection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88732189-d83d-41a9-8cc2-4708ab0309db
⛔ Files ignored due to path filters (2)
internal/abi/gen/main.gois excluded by!**/gen/**internal/abi/gen/main_test.gois excluded by!**/gen/**
📒 Files selected for processing (21)
AGENTS.mdMakefileREADME.mddocs/cli.mdinternal/abi/kernel_event.gen.hinternal/abi/schema_gen.gointernal/abi/tracepoints_gen.gointernal/cli/commands_query.gointernal/cli/commands_runtime.gointernal/collector/filter.gointernal/daemon/flush.gointernal/diff/engine.gointernal/ebpf/collector_linux.gointernal/query/engine.gointernal/storage/maintenance.gointernal/storage/open.gointernal/storage/query.gointernal/storage/schema_gen.gointernal/storage/store_test.gointernal/storage/types.gointernal/storage/write.go
💤 Files with no reviewable changes (1)
- internal/daemon/flush.go
🚧 Files skipped from review as they are similar to previous changes (13)
- internal/storage/maintenance.go
- internal/abi/kernel_event.gen.h
- internal/abi/schema_gen.go
- internal/collector/filter.go
- README.md
- internal/abi/tracepoints_gen.go
- internal/storage/open.go
- internal/ebpf/collector_linux.go
- internal/storage/write.go
- internal/cli/commands_query.go
- Makefile
- internal/cli/commands_runtime.go
- AGENTS.md
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.
Summary
internal/abisource-of-truth generator for tracepoints, schema docs, storage DDL, and the kernel event headerTesting
Summary by CodeRabbit
eventsschema, plus generation/verification to keep artifacts consistent.