Hi Bertrand Thank you for taking a look.
As you observed correctly, the API changes were done to allow more flexibility when rendering the logs. The main objective was avoiding the date/time stamp (e.g. "(2014-10-28 10:49:20)") in each line. Because it does not add any information, because the granularity is too coarse, and we already have the millisecond durations. I have attached another patch to SLING-4114 that does not change the API. However, it drops the date/time from the message format. If dropping the date/time from the messages is not acceptable, I can create a slightly more complicated patch. WDYT? Regards Julian On Fri, Oct 31, 2014 at 1:57 PM, Bertrand Delacretaz <[email protected]> wrote: > Hi Julian, > > On Wed, Oct 29, 2014 at 9:42 AM, Julian Sedding <[email protected]> wrote: > >> ...I propose to reduce the size of the log file by doing the following: >> 1. changing the log format to include less redundant data >> 2. allow filtering by request duration >> 3. allow filtering by request extension(s) > > sounds good to me! > >> ...I created SLING-4114 [0] with a patch attached that implements these >> changes. Note that I made some additions to the RequestProgressTracker >> API. These should be compatible for consumers, so I think they should >> not do any harm.... > > Still, I'm not too happy with requiring changes to the API bundle just for > this. > > IIUC your changes to RequestProgressTracker are only about extracting > data from the tracker, as opposed to providing data to it, which is > what 99% of the code that uses that API does. > > So one way to avoid those API bundle changes would be to create a > SlingEngineRequestProgressTracker interface (exported) in the engine > bundle, which is RequestProgressTracker + niceties that the actual > engine provides (your changes). The SlingRequestProgressTracker > implements this. > > As all the reporting is currently done in the engine bundle that's not > a problem for Sling itself, the engine just gets the tracker and if it > implements SlingEngineRequestProgressTracker uses the additional data. > > If someone wants to access that data they can do the same. > > WDYT? > -Bertrand
