dhis2-devs team mailing list archive

[Bob Jolliffe <bobjolliffe@xxxxxxxxx>]

Hi Viet

On 19 April 2010 13:47, Viet Nguyen <phamquocviet@xxxxxxxxx> wrote:
>
>
> On Mon, Apr 19, 2010 at 5:51 PM, Bob Jolliffe <bobjolliffe@xxxxxxxxx> wrote:
>>
>> Hi Viet
>>
>> This is excellent.
>>
>> When we first started discussing using logging framework for audit you
>> pointed out that logging is most commonly used for system
>> info/diagnostics/debugging.  Which is true.  Of course logging works
>> pretty well for audit as well, but I was thinking (influenced by
>> http://www.mail-archive.com/general@xxxxxxxxxxxxxxxxxx/msg00747.html)
>> that it makes sense to use a distinct logger, rather than one based
>> after the class name, for this.
>>
>> So for example instead of where you have:
>> private Logger logger = Logger.getLogger( getClass() );
>>
>> I was thinking something more like
>> private Logger audit_logger = Logger.getLogger( AUDIT_LOG ); // where
>> AUDIT_LOG is a globally accessible string
>>
>> Or considering that the audit_logger could be thought of as a
>> singleton throughout the system, just use a setter and configure by
>> spring.  Or a static getLogger method on a simple AuditLogger class.
>>
>> However, I see you have extended Level to produce a new AUDIT_LEVEL
>> and routed that to a distinct log appender which I can see also works
>> well to manage the output of logging in a discriminatory way.  That's
>> pretty cool.  I'm still in two minds whether the enabling of audit
>> should be related to the debug level you want to have on diagnostics
>> though.  I might reasonably want to turn audit off and debug on for
>> example.  For which I'd want a distinct audit logger.  Maybe that's
>> not reasonable.
>>
>> What do you think?
>>
>> Cheers
>> Bob
>
>
> For your first concern, I think using AOP can solve it. Lars also told me
> about that.
> I will try to apply AOP to this.
>
> About distinct audit logger, I don't know how to use two different loggers
> in one application... Unless I manually create a logger class ( create file,
> write logging message to that file....etc.. ) which I did before in my old
> project...

I am no expert on log4j but it seems that fortunately it's not as
complicated as you expect.  And you are already using a great many
different loggers in one application!.  Its just that each one is is
typically named after the class in which it is used.  That is the
normal and natural way to configure logging for diagnostics.  And all
these named loggers form themselves into a hierarchical tree on top of
the root logger.  So typically we have many loggers with names like:

"org.hisp.dhis.dataset.DefaultDataSetService",
"org.hisp.dhis.importexport.DefaultDhis14XMLImportService" etc

The nice thing about this is that one can use the inheritance in the
tree to fine tune what goes on in branches of the tree.  So for
example I should be able to set logging level to DEBUG for all loggers
below org.hisp.dhis.importexport.

So having loggers in a tree is cool, but what is also cool is that you
can structure that tree using whichever strings are appropriate.  For
diagnostic purposes it is generally structured according to
class/package names.  But you can equally have loggers called
"security.audit", "security.alarm" or "security.core.audit" and
"security.web.audit" etc,  There is no difference in the code.  And
the log4j properties will work as before except that you can
(optionally) override the root logger if you want to configure the
security logs separately from the diagnostic system logs.

Anyway I agree that its really not critical and the really good thing
is that you have implemented the audit logging.  I guess I'm just
trying to set your mind at rest that you might not think you know how
to work with multiple loggers but in fact you do.

If at some stage in the future we want to separate the audit logger
from the rest it will be simple.  If we do that I would be tempted to
bring the audit levels and audit utils stuff together into an AuditLog
class.

Cheers
Bob

BTW I am not meaning to be critical.  At least it shows I looked at the code :-)

>
> Anyway...turning debug on and audit off....is not really a common use I
> think ... I always want to see everything when I turn debug on ...
> And when you are debugging , means that you don't work on production server,
> so it will not effect the production logging files...
> However, I will google more about using two different loggers  in one
> application, should be interesting.
>
> Anyway ...I have to back with STQC bugs now ...so could we consider that
> committed code as a beta version of this audit trail logging ...
>
>>
>> On 19 April 2010 10:56,  <noreply@xxxxxxxxxxxxx> wrote:
>> > ------------------------------------------------------------
>> > revno: 1785
>> > committer: Viet <Viet@Viet-Laptop>
>> > branch nick: trunk
>> > timestamp: Mon 2010-04-19 16:54:45 +0700
>> > message:
>> >  Audit trail logging functions for DataSet, OrganisationUnit, User
>> > modules.
>> > added:
>> >
>> >  dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/system/util/AuditLogLevel.java
>> >
>> >  dhis-2/dhis-support/dhis-support-system/src/main/java/org/hisp/dhis/system/util/AuditLogUtil.java
>> > modified:
>> >
>> >  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataset/DefaultDataSetService.java
>> >
>> >  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/DefaultOrganisationUnitService.java
>> >
>> >  dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml
>> >
>> >  dhis-2/dhis-services/dhis-service-patient/src/main/java/org/hisp/dhis/patient/DefaultPatientService.java
>> >
>> >  dhis-2/dhis-services/dhis-service-patient/src/main/resources/META-INF/dhis/beans.xml
>> >
>> >  dhis-2/dhis-services/dhis-service-user-hibernate/src/main/java/org/hisp/dhis/user/hibernate/HibernateUserStore.java
>> >
>> >  dhis-2/dhis-services/dhis-service-user-hibernate/src/main/resources/META-INF/dhis/beans.xml
>> >
>> >  dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/WEB-INF/classes/log4j.properties
>> >
>> >  dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/util/validate/additional-methods.js
>> >
>> >  dhis-2/dhis-web/dhis-web-commons-resources/src/main/webapp/dhis-web-commons/util/validate/jquery.validate.js
>> >
>> >  dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-dataset/src/main/webapp/dhis-web-maintenance-dataset/addSection.vm
>> >
>> >  dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-organisationunit/src/main/webapp/dhis-web-maintenance-organisationunit/addOrganisationUnitGroupSetForm.vm
>> >
>> >  dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-organisationunit/src/main/webapp/dhis-web-maintenance-organisationunit/updateOrganisationUnitGroupSetForm.vm
>> >
>> >  dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-patient/src/main/java/org/hisp/dhis/patient/action/patient/AddPatientAction.java
>> >
>> >  dhis-2/dhis-web/dhis-web-maintenance/dhis-web-maintenance-patient/src/main/java/org/hisp/dhis/patient/action/patient/ValidatePatientAction.java
>> > The size of the diff (1144 lines) is larger than your specified limit of
>> > 1000 lines
>> >
>> > --
>> > lp:dhis2
>> > https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk
>> >
>> > Your team DHIS 2 developers is subscribed to branch lp:dhis2.
>> > To unsubscribe from this branch go to
>> > https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription
>> >
>> > _______________________________________________
>> > Mailing list: https://launchpad.net/~dhis2-devs
>> > Post to     : dhis2-devs@xxxxxxxxxxxxxxxxxxx
>> > Unsubscribe : https://launchpad.net/~dhis2-devs
>> > More help   : https://help.launchpad.net/ListHelp
>> >
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~dhis2-devs
>> Post to     : dhis2-devs@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~dhis2-devs
>> More help   : https://help.launchpad.net/ListHelp
>
>
>
> --
> Viet Nguyen
>
>