Skip to content

feat: unconditionally send off a final report on LocalReporter drop#125

Open
jlizen wants to merge 3 commits intoasync-profiler:mainfrom
jlizen:main
Open

feat: unconditionally send off a final report on LocalReporter drop#125
jlizen wants to merge 3 commits intoasync-profiler:mainfrom
jlizen:main

Conversation

@jlizen
Copy link
Collaborator

@jlizen jlizen commented Feb 19, 2026

📬 Issue #, if available:
Closes #113

✍️ Description of changes:
We want the local report to unconditionally flush any JFRs prior to shutting down the runtime. And then, for all other reporters, existing behavior of warning should be preserved.

I added a report_blocking method with a default implementation to non-op. That default is slightly better than the old one, since we no longer warn EVERY drop if no JFRs are omitted (previously, we unconditionally warned).

And then, the LocalReport does what you'd expect. As does the multi-reporter, it attempts each internal report's warn call on drop.

For non-local reporter usage, adds about 1 byte of memory overhead per profiler instance between the completed_normally bool + padding (which lets us improve the warning). And then, there is an extra 8 bytes for the new vtable entry, and an extra vtable pointer chase on the cancellation path.

On the non-local-reporter path (e.g. S3 reporter hitting the default no-op), try_final_report does:

  1. self.state.stop() — stops async-profiler (this was going to happen anyway)
  2. jfr_file.active_path() — constructs a /proc/self/fd/N PathBuf from a format string (~20 byte allocation)
  3. jfr_path.metadata()?.len() — one stat syscall to check file size
  4. Builds a ReportMetadata struct on the stack (two Durations + a reference + a Duration)
  5. Calls report_blocking which just logs a message and returns Ok(())

We could get that further down by leaving off ReportMetadata, which we don't really use in the local reporter report_blocking anyway, but it seemed like a good idea to bake in.

🔏 By submitting this pull request

  • I confirm that I've made a best effort attempt to update all relevant documentation.
  • I confirm that my contribution is made under the terms of the Apache 2.0 license.

Adds a synchronous report_blocking method with a default no-op
that logs a warning. LocalReporter overrides it to copy JFR files
via std::fs::copy, and MultiReporter delegates to its children.

The method accepts a &Path instead of Vec<u8> so that reporters
decide whether and how to read the file, avoiding an unconditional
read for reporters that don't support synchronous reporting.
@jlizen jlizen requested a review from rcoh February 19, 2026 21:59
@jlizen
Copy link
Collaborator Author

jlizen commented Feb 25, 2026

I was discussing with @rcoh offline about this seeming fairly complex for what we're trying to do. Here's why it ended up this way.

The JFR data lives in anonymous tempfiles owned by ProfilerState, created inside spawn_inner. LocalReporter only knows its output directory. They're connected through Box<dyn Reporter>, so the profiler can't reach the concrete type.

I looked at doing this entirely inside LocalReporter (scanning the output dir on drop, sharing state at construction time, breadcrumb files) but none work. The tempfiles don't exist when LocalReporter is created, and the data never lands in the output directory on its own. Named tempfiles with a known path convention would solve this, but the current anonymous tempfile approach is deliberate to avoid symlink race vulnerabilities.

The decent options I saw all involved trait methods, just with different control flows.

  1. emergency_output_path() -> Option<PathBuf>: Reporter tells the profiler where to write. Profiler's drop path copies the JFR there. Simplest trait method, but leaky abstraction since the reporter is exposing implementation details

  2. register_jfr_files(&self, active: &File, inactive: &File): Profiler pushes the tempfile handles after creating them. LocalReporter clones the fds and copies whichever is non-empty on drop. Purely additive, old LogOnDrop stays untouched. I could cut the current report_blocking approach down to this.

  3. report_blocking(&self, &Path, &ReportMetadata): Profiler orchestrates stop, read, and synchronous report in its own drop path. Most code on the profiler side (the ProfilerTaskState wrapper), but arguably the cleanest reporter interface.

Do you prefer any of those? Or is there an option I'm missing?

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.

Oneshot process exits before a sample can be captured

1 participant