search

krzysztofslusarski / continuous-async-profiler

Spring boot library for continuous profiling with async-profiler
Apache License 2.0
30 stars 5 forks source link

Colon ':' in file name is confusing #37

Closed michaldo closed 8 months ago

michaldo commented 8 months ago

https://github.com/krzysztofslusarski/continuous-async-profiler/blob/e70bd61e639969d13f7452f8899af708dfe3d241/continuous-async-profiler/src/main/java/com/github/krzysztofslusarski/asyncprofiler/ContinuousAsyncProfilerRunner.java#L33

I noticed few times colon can be confusing:

kubectl cp /app/profiler-output/wall-2024-02-21_12:26:18.jfr wall-2024-02-21_12:26:18.jfr error: one of src or dest must be a local file specification

Windows Explorer image

Intellij image

My proposul is replace ':' by '_', huh?

krzysztofslusarski commented 8 months ago

Is this only an issue on Windows? The colon is a valid filename character and it's kind of standard way of using it in time. On linux you just need to escape it:

$ ls wall-2021-07-17_13\:05\:33.jfr.gz 
wall-2021-07-17_13:05:33.jfr.gz

If you need we can add the formatter pattern as a property so you could overwrite it, but I'm against changing the default.

michaldo commented 8 months ago

kubectl which rejects colon is installed on WSL, but fails also on Mac. Intellij screenshot is Windows

I agree colon is standard time separator but not necessary good choice for filename

krzysztofslusarski commented 8 months ago

Yeah, but I know that there are production systems that use this library and rely on that the filename has a colon. So it basically too late for changing it as default.

Again, I'm ok with making it configurable.

michaldo commented 8 months ago

I prefer modify my kubectl script than implement configurable map and then propagate configuration to many microservices

Maybe 3.x will be opportunity to improve defaults