Removed per-method measurement and aggregate stats to minimize overhead for cheap operations
Recording stack traces, etc. may be useful. I raised those overheads for two reasons. First, that diagnostic data should only be built when necessary. Previous versions of the patch generated diagnostic data that was ultimately discarded. Second, minutiae like "GC-friendliness", whether a wrapper class on Lock adds dispatch overhead, class loading overhead, etc. when this JIRA logs critical sections longer than 300ms are irrelevant, literally by orders of magnitude.
I'll ask again after the motivation. Is the intent to identify subsystems that do too much work (and/or I/O) in a critical section? To measure where the DN is spending cycles? In some of these cases, stack traces could be very helpful. As long as this telemetry can be tuned by configuration, it shouldn't create new problems for operators. To that point, instead of reusing the FsDatasetImpl log, this may want to use its own (so disabling/redirecting it doesn't affect other, critical information written to that log).
Was your concern was with exposing AutoCloseableLock in hadoop utils or just an overall objection to this idiom even if it's not exposed outside the DN.
I have two objections, one to the idiom and one to the design.
AutoCloseableLock is a new abstraction that doesn't elide any details of the thing it obscures. Every path for Lock objects is possible, so it doesn't make it easier to reason about lock correctness. It creates a special type for the simplest possible case. This case is so simple, we have automated tooling to detect it. Further, instead of every Java developer looking at a Lock and scanning for well-worn expectations, we introduce an abstraction that requires investigation. No advantage beyond developer convenience has been raised, and it is a demonstrably inconvenient abstraction.
If the interface were limited so it could only be used in the resource idiom (as proposed earlier), then a developer could know that the Lock acquisition was limited to scope. As semantic sugar it would still be pointless, because Java programmers also know how to use synchronized.
This could be redeemed by changing the design. If the AutoCloseableLock constructor accepted a Lock instance and cut its API to scope-limited changes, then telemetry like this could be added as implementations of the Lock type with all the perceived advantages of ACL. Devs can reason about the simpler API of ACL (scope-based acquisition), but still monitor all acquisitions as required by this JIRA (impractical to implement using synchronized).
I remain skeptical of AutoCloseableLock, since one could just use the Lock in interface and existing tooling to implement the same. But if its virtues are obvious to everyone but me I won't stand in the way of including it.