Subject: Re: [PATCH 2/2] trace2: randomize/timestamp trace2 targets

From: Josh Steadmon <[email-masked]>
Date: Fri, 15 Mar 2019 13:43:27 -0700

On 2019.03.14 00:49, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Mar 14 2019, Josh Steadmon wrote:
> 
> > When the value of a trace2 environment variable contains instances of
> > the string "%ISO8601%", expand them into the current UTC timestamp in
> > ISO 8601 format.
> 
> Any reason not to just support feeding the path to strbuf_addftime(), to
> e.g. support a daily/hourly log?

No reason not to. Seems reasonable to me.

> > When the value of a trace2 environment variable is an absolute path
> > referring to an existing directory, write output to randomly-named
> > files under the given directory. If the value is an absolute path
> > referring to a non-existent file and ends with a dash, use the value as
> > a prefix for randomly named files.
> >
> > The random filenames will consist of the value of the environment
> > variable (after potential timestamp expansion), followed by a 6
> > character random string such as would be produced by mkstemp(3).
> >
> > This makes it more convenient to collect traces for every git
> > invocation by unconditionally setting the relevant trace2 envvar to a
> > constant directory name.
> 
> Hrm, api-trace2.txt already specifies that the "sid" is going to be
> unique, couldn't we just have some mode where we use that?
> 
> But then of course when we have nested processes will contain slashes,
> so we'd either run into deep nesting or need to munge the slashes, in
> which case we might bump against a file length limit (although I haven't
> seen process trees deeper than 3-4).
> 
> Just to pry about the use-case since I'm doing similar collecting, why
> are you finding this easier to process?

Basically, our collection setup prefers smaller files that are
"finished" earlier, rather than long-lived files that are constantly
appended to.

> With the current O_APPEND semantics you're (unless I've missed
> something) guaranteed to get a single process tree in nested order,
> whereas with this they'll all end up in separate files and you'll need
> to slurp them up, sort the whole thing and stitch it together yourself
> without the benefit of stream-parsing it where you can cheat a bit
> knowing that e.g. a "reflog expire" entry is always coming after the
> corresponding "gc" that invoked it.

Yeah, that is not an issue for us, although I can see why others would
prefer single file. I suppose we can just modify the envvar to point to
our newly-generated file before we spawn any child processes?