RFR: 8204681: Option to include timestamp in hprof filename

[Thomas Stuefe]

On Tue, 13 Aug 2024 15:07:17 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

> Hi all, 
> 
> This PR addresses [8204681](https://bugs.openjdk.org/browse/JDK-8204681) enabling support for timestamp expansion in filenames specified in `-XX:HeapDumpPath` using `%t`. 
> 
> As mentioned in this comments for this issue, this is somewhat related to [8334492](https://bugs.openjdk.org/browse/JDK-8334492) where we enabled support for `%p` for filenames specified in jcmd. 
> 
> With this patch, I propose: 
> - Expanding the utility function `Arguments::copy_expand_pid` to `Arguments::copy_expand_arguments` to deal with `%p` expansions for pid and `%t` expansions for timestamps. 
> - Leveraging the above utility function to enable argument expansion for both heap dump filenames and jcmd output commands. 
> - Though the linked JBS issue only relates to heap dumps generated in case of OOM, I think we can edit it to more broadly support filename expansion to support `%t` for jcmd as well. 
> 
> Testing: 
> - [x] Added test cases pass with all platforms (verified with a GHA job). 
> - [x] Tier 1 passes with GHA. 
> 
> Looking forward to hearing your thoughts!
> 
> Thanks, 
> Sonia

I think this could be very useful, but it needs more preparation and decisions. Possibly a CSR.

- copy_expand_xxx is used in many places. While I think all of these places would benefit from more expansions than just %p, there is a potential backward compatibility issue if clients use %t for whatever reason today
- Do we want the time of the dump or the JVM start? If the JVM runs for a week, then produces a JFR file, should the file be named by the JVM start date? I think in most cases the *current* time makes more sense
- Do we want the printout as a human-readable date or as a numeric timestamp? Both makes sense depending on the post-processing clients want to do.
- Do we want to improve this function further, potentially adding more replacement options?

One possible way to solve this:
- use different characters for timestamp (number) and datetime (human readable date)
- use always the current time
- If we want to add further replacements: 
  - come up with a new replacement character that does not clash with libc sprintf (IMHO using percent was not a good idea in the first place). E.g. `$`
  - Add a new switch to guard this new replacement logic. By default off. If on, the contract is that any character following a `$` may be either now or in the future replaced with something different. Client must not use `$` as a normal character.
  - We probably should remove all non-matching `$` from the input.
  - The first replacements could be: `$p` for pid, `$t` for timestamp (numeric), `$d` for datetime
  - later replacements can be added later. Since we guard the new feature with a switch and forbid the use of `$`, we are then free to do so without breaking backward compatibility.

I would like to hear @dholmes-ora take on this.

We had a similar system at SAP in our proprietary JVM, which was really useful, so I like this idea in general.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20568#issuecomment-2301641288