Re: [RFC PATCH 1/8] trace2: log fsync stats in trace2 rather than wrapper

[Victoria Dye <vdye@github.com>]

Calvin Wan wrote:
>> An added note re: the commit message - it's extremely important that
>> functions _anywhere in Git_ are able to use the Trace2 API directly. A
>> developer could reasonably want to measure performance, keep track of an
>> interesting metric, log when a region is entered in the larger trace,
>> capture error information, etc. for any function, regardless of where in
>> falls in the internal library organization.
> 
> I don't quite agree that functions _anywhere in Git_ are able to use
> the Trace2 API directly for the same reason that we don't have the
> ability to log functions in external libraries -- logging common,
> low-level functionality creates an unnecessary amount of log churn and
> those logs generally contain practically useless information. 

That may be true in your use cases, but it isn't in mine and may not be for
others'. In fact, I was just using these exact fsync metrics a couple weeks
ago to do some performance analysis; I could easily imagine doing something
similar for another "low level" function. It's unreasonable - and unfair to
future development - to make an absolute declaration about "what's useful
vs. useless" and use that decision to justify severely limiting our future
flexibility on the matter.

> However,
> that does not mean that all of the functions in git-std-lib fall into
> that category (usage has certain functions definitely worth logging).
> This means that files like usage.c could instead be separated into its
> own library and git-std-lib would only contain files that we deem
> "should never be logged".

How do you make that determination? What about if/when someone realizes,
somewhere down the line, that one of those "should never be logged" files
would actually benefit from some aggregate metric, e.g. a Trace2 timer? This
isn't a case of extracting an extraneous dependency (where a function really
doesn't _need_ something it has access to); tracing & logging is a core
functionality in Git, and should not be artificially constrained in the name
of organization. 

>> To that end, I think either the
>> commit message should be rephrased to remove that statement (if the issue is
>> really "we're using a static variable and we want to avoid that"), or the
>> libification effort should be updated to accommodate use of Trace2 anywhere
>> in Git.
> 
> Besides potentially redrawing the boundaries of git-std-lib to
> accommodate Trace2, we're also looking into the possibility of
> stubbing out tracing in git-std-lib so that it and other libraries can
> be built and tested, and then when Trace2 is turned into a library,
> it's full functionality can be linked to.

If that allows you to meet your libification goals without limiting Trace2's
accessibility throughout the codebase, that works for me.