Skip to content

Conversation

@smklein
Copy link
Collaborator

@smklein smklein commented Dec 9, 2025

My top-level goal: Make it easier for devs who don't care about "how support bundles work" to add data to them.

This PR:

  • Adds a README describing support bundle structure, and the steps framework
  • Pulls a bunch of stuff out of nexus/src/app/background/tasks/support_bundle_collector.rs, into modules

New structure:

  • support_bundle/cache.rs: Contains OnceCell structures that are constructed lazily and shared by steps
  • support_bundle/collection.rs: Contains BundleCollection, which encapsulates the raw data necessary to construct the bundle itself. This handles bundle lifetime management, with minimal "step-specific" info.
  • support_bundle/request.rs: Contains filters for what is included in the bundle.
  • support_bundle/step.rs: Contains generic step instrumentation, types.
  • support_bundle/steps/...: All step-specific information, one per-module.

@smklein smklein requested review from hawkw and wfchandler December 10, 2025 20:37
@smklein
Copy link
Collaborator Author

smklein commented Dec 10, 2025

I'm happy to do whatever is desired to make it clear "what is moving code" vs "what is new code" - here's how I prompted claude to do a little self-review:

PROMPT: run "jj diff --from trace-bundle" to see my current diff. This should show you the contents of                                                                                                                                                                                                                                                                                                                                                                                             
https://github.com/oxidecomputer/omicron/pull/9498. This PR claims it is refactoring, compared to the base branch                                                                                                                                                                                                                                                                                                                                                                            
 (jj revision 'oxw'). Can look through all changes, compared with the base branch, and confirm that it's only                                                                                                                                                                                                                                                                                                                                                                                
doing refactoring? If there are any changes beyond 'moving code', I'd like to know. 

<-- REDACTED: lots of `jj diff`, `jj file show` commands -->

RESPONSE: Based on my analysis of the diff from trace-bundle, this PR is mostly refactoring but includes some changes
  beyond just "moving code":

  Pure Refactoring (Moving Code):

  ✅ Splits support_bundle_collector.rs into modular files:
  - cache.rs - caching logic
  - collection.rs - main collection logic
  - request.rs - request types and data selection
  - step.rs - step execution framework
  - steps/* - individual collection steps as separate modules
  ✅ Converts methods on BundleCollection to standalone functions
  ✅ Wraps OnceCell variables in a Cache struct

  Changes Beyond Moving Code:

  1. Visibility Changes (API expansion):

  Many previously private types/methods are now pub:
  - BundleCollection, BundleRequest, BundleDataCategory, BundleData, BundleDataSelection, SledSelection
  - CollectionStep, CollectionStepOutput, CompletedCollectionStep
  - Constants: TEMPDIR, CHUNK_SIZE
  - All methods on BundleRequest (nexus/src/app/background/tasks/support_bundle/request.rs:153-185)

  2. New Public API:

  - BundleCollection::new() constructor (nexus/src/app/background/tasks/support_bundle/collection.rs:59)
  - Accessor methods on BundleCollection: datastore(), resolver(), log(), opctx(), request(), bundle()
  (nexus/src/app/background/tasks/support_bundle/collection.rs:79-101)

  3. Function Signature Change:

  - CollectionStep::run() now takes an explicit log: &slog::Logger parameter instead of accessing collection.log
  internally (nexus/src/app/background/tasks/support_bundle/step.rs:47-52)
    - Functionally equivalent since callers pass collection.log(), but changes the API

  4. Documentation:

  - New README.md for developers (66 lines)
  - Converted // comments to /// doc comments throughout

  Verdict: This is primarily refactoring with some intentional API changes (making things public, adding
  accessors) to support the new modular structure. The core collection logic remains unchanged.

Base automatically changed from trace-bundle to main December 11, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants