Ralph, the doc changes are improvements but not ground to veto a release. I haven't actually tried it yet.
On Wed, May 9, 2018 at 11:47 PM, Matt Sicker <[email protected]> wrote: > I've never worked in a domain where audit logging is used, so I won't have > much feedback about that. I will, however, provide a more thorough release > review (similar to Incubator). > > On 9 May 2018 at 00:32, Ralph Goers <[email protected]> wrote: > > > Thanks for this re-review. While I am going to go through this and make > > some changes, my basic question would be is if any of this would make you > > vote -1 on a release candidate? While I think the documentation should > be > > good I don’t think it has to be perfect. > > > > Although I have been using it in production for a while it would be great > > if someone else could give it a try. > > > > Ralph > > > > > On May 7, 2018, at 5:42 PM, Remko Popma <[email protected]> wrote: > > > > > > I had time to look at this during the flight, here it is: > > > > > > ---- > > > index.html > > > > > > typo: Diagnostic logs are critical in aiding in maintaining the > > > servicability -> critical in maintaining? > > > > > > Overall, the first three sections, "What is Audit Logging", What is the > > > difference between audit logging and normal logging?" and "What is > Log4j > > > Audit?" are very good: give good overview of the purpose and don't > assume > > > prior knowledge. > > > > > > From the "Features" section, the narrative changes perspective from > what > > > users would want to what Log4j Audit provides. > > > I would add a few sentences to that transition, something like: > > > > > > {quote} > > > (after Features) > > > Each application has its own audit events. Before using Log4j Audit, > > > applications need to define AuditMessages that capture the exact > > attributes > > > of its audit events. The [Getting Started](link) page provides a > tutorial > > > that explains how to define audit events for an application. > > > > > > (after Audit Event Catalog header) > > > Once audit events are defined, they need to be maintained: as the > > > application evolves, developers will inevitably discover they need to > > add, > > > remove or change attributes of the audit events. Log4j Audit can > persist > > > the audit event definitions in a JSON file. This file becomes the Audit > > > Event Catalog for the application. Log4j Audit is designed to store the > > > event definition file in a Git repository so that the evolution of the > > > audit events themselves have an audit trail in the Git history of the > > file. > > > Log4j Audit provides a web interface for editing the events. > > > > > > Log4j Audit uses the catalog of events to determine ... (continue with > > > current text of Audit Event Catalog) > > > {quote} > > > > > > Question about the Requirements section: it isn't clear to me (and > likely > > > to other readers) why Dynamic Event Catalogs would require a database > > > instead of one or more JSON files. Is that explained somewhere? Perhaps > > > Dynamic Audit Events need a separate page or dedicated section > somewhere. > > > The Getting Started page mentions "manage dynamic catalogs" in the > > > paragraph under "What you will build" but I couldn't find anything on > the > > > topic of dynamic catalogs. > > > > > > > > > > > > ---- > > > catalog.html > > > > > > From the first paragraph, I would remove "The events may be grouped by > > > Products and/or Categories, but at this time nothing in Log4j Audit > makes > > > use of the product or catalog definitions". The same sentences is > > repeated > > > at the bottom of the page and since this feature is not used it is > > > confusing to me that the feature is so prominently mentioned in the > first > > > paragraph of the page. I would consider removing this feature > altogether. > > > > > > Overall this is a very good page. Succinct but complete. Consider > moving > > it > > > above RequestContext in the left-hand navigation menu. > > > > > > > > > > > > ---- > > > gettingStarted.html > > > > > > Overall, this page is only effective for people who actually perform > the > > > steps and execute the commands mentioned in the page. > > > > > > It would be good if the page would also be useful for people who only > > read > > > the page but don't actually perform the steps: > > > > > > * Can the page also show an example of an audit event in JSON format. > > This > > > could be a simple event with few attributes (maybe a login event?) or > the > > > transfer event that is used later in the page. > > > * I would also like to see the Java interface that is generated from > this > > > JSON audit event. > > > * Finally, I would like to see how my application would use this > > generated > > > Java interface. How do I get an instance, how do I populate the > > attributes, > > > and what do I do with the instance after I populated it? > > > > > > I'm sure the above is available in the source code of the sample > > > application, but this page is a good place to show some of the > highlights > > > of that source code with some explanatory text. > > > > > > Secondly, the page mentions remote audit logging and how the war file > > > provides endpoints for remove audit logging. Is it worth dedicating a > > > separate page to show how to configure end points for remote audit > > logging? > > > > > > Finally, about the catalog screenshots: I understand that attributes > are > > > managed separately so they can be reused. The second screenshot shows > the > > > billPay and deposit events. Are these events related to the transfer > > event > > > that is mentioned in the curl example in this page? I was trying to see > > how > > > they could be related but couldn't figure it out. > > > Also, what are the attributes for the billPay and deposit events? If > the > > > Catalog Editor has a screen to show the attributes that are part of an > > > event then it may be good to add a screenshot for this (I guess this > > would > > > be the Edit Event screen) as well. That would tie all these concepts > > > together. > > > > > > > > > ---- > > > requestContext.html > > > > > > typo: typcial -> typical > > > typo: acrossall -> across all > > > typo: datbase -> database > > > > > > About Mapping Annotations: > > > This is still a bit abstract to me. Would it be possible to provide > some > > > more explanation on when applications should use ClientServer, when > > Local, > > > and when Chained annotations? Perhaps some example use cases? Or, if > > > possible, tie this to the use case presented in the sample application > > (if > > > that makes sense)? > > > > > > About Transporting the RequestContext: > > > Until now, the information was generically useful for all applications, > > but > > > this section is specifically useful for web applications. > > > For people who don't work on web applications this transition may be a > > bit > > > jarring. > > > Would it make sense for this section and the following two sections to > be > > > moved to a separate page? Something like "Web Applications" or "Remote > > > Audit Logging"? > > > > > > ---- > > > Remko > > > > > > On Mon, May 7, 2018 at 5:17 AM, Matt Sicker <[email protected]> wrote: > > > > > >> I've been meaning to take a closer look at this. I'll review it over > the > > >> next day or two. > > >> > > >> On 6 May 2018 at 16:26, Ralph Goers <[email protected]> > wrote: > > >> > > >>> I spoke too soon. I made a minor change to add a link to Apache > events > > in > > >>> the site header. > > >>> > > >>> Ralph > > >>> > > >>>> On May 6, 2018, at 2:24 PM, Ralph Goers <[email protected] > > > > >>> wrote: > > >>>> > > >>>> I don’t think anything has changed since I last published but I will > > >>> rebuild it and publish it again. > > >>>> > > >>>> Ralph > > >>>> > > >>>>> On May 6, 2018, at 12:52 PM, Remko Popma <[email protected]> > > >> wrote: > > >>>>> > > >>>>> I’ll be flying back to Tokyo tomorrow but I can take another look > > when > > >>> I’m back. Is there a recent snapshot of the site on your GitHub > > account? > > >>>>> > > >>>>> > > >>>>> > > >>>>>> On May 6, 2018, at 21:35, Ralph Goers <[email protected] > > > > >>> wrote: > > >>>>>> > > >>>>>> I have finished everything I wanted to accomplish for the first > > >>> release of log4j-audit and so I am ready to generate a release > > candidate. > > >>> Before I do that I’d like to provide one more chance for feedback. > > >>>>>> > > >>>>>> Thoughts? > > >>>>>> > > >>>>>> Ralph > > >>>>> > > >>>> > > >>>> > > >>>> > > >>> > > >>> > > >>> > > >> > > >> > > >> -- > > >> Matt Sicker <[email protected]> > > >> > > > > > > > > > -- > Matt Sicker <[email protected]> >
