Re: Custom log file

[Marco Ermini]

On 6/3/05, Jan-Henrik Haukeland <address@hidden> wrote:
> Here is a more detailed description of my objections to you patch Marco,
> 
> 1. The custom log function does not integrate with the monit code, it
> intercept monit's event handling by inserting a call out to its own
> logger module. This is not integration, this is a control flow hack
> to inject your own code. This is what you do when you have 5 days to
> hack monit to provide a certain functionality you need.

This is what I would do if I don't want to mess up too much with your
code. You call it a "control flow hack", I call it "don't change too
much and follow KISS principle"...

In any case, now you were much more clear and I understand the point.


> 2. The customlog.c module consists mainly of cut&past code from log.c
> and event.c and is a blatant duplication of code. When you duplicate
> code like this, red lights should go off and warn you that you have a
> design problem.

Sure, but it's not _my_ design :-) just to be clear about it, don't
complain me ;-)

As I said, I don't have a magic crystal ball, but a limited time. If
the Monit code was already flexible enought, I would have did it in
another way :-)

Again, I understand the point now.


> The main functionality lays in the main while loop
> that parse the log format. This code may be useful, but not in this
> design. A minor detail is also that the code is peppered with
> insecure buffer handling functions, boundary checks *must* always be
> used, i.e. use strncpy instead of strcpy and so on.
[...]

Sure, but not checking if you are calling the function from an
Event_post() or not. This is an unuseful check, since you are just
checking that you are using your own function correctly :-)

Sincerly, the solution I chose was not only driven by the limited
time. I just wanted to not have too much impact in Monit code. If you
are asking me to have a deeper impact and re-do something using a
delegation pattern, I could try to do it. It depends on having my boss
enroll me in this job. Sincerly, C language is not as good as Java in
this kind of things, but in any case, I think that it should be done
as a professional contribution.

I am now involved in trying to gain this job ;-)


> 3. Finally I also have a major problem with how the custom log stuff
> manifest itself at the control file level. You may recall that I had
> my objections when this was first proposed by you. Here is one example,
> 
>      if failed port 80 protocol http then alert log "message" alarmid 33
>          severity warning
> 
> What's going on here is that an event with a certain alarmid and
> severity are hardcoded to be logged. What if I want this kind of
> events to be sent in an email instead? or as a snmp message? There
> are no flexibility for choosing how you want to handle an event with
> severity warning or with alarmid 33. This is bad design, because
> implementation details shows up at the configuration level.

Sure, but again, it's not MY design :-)

You would be right if you _already_ have a generic implementation of
the actions to be executed by Monit. But since we DID NOT have it, I
can't do it in a different way - without rewriting consistent portions
of Monit - Monit is not my daily job.


> You don't
> want that, to do this proper, you may tag a certain event with e.g. a
> severity and alarmid and allow the user to configure where he want
> those alerts to be sent. For instance,
> 
>      if failed port 80 protocol http then alert with alarmid 33 and
> severity warning
> 
> The user could then configure if events with severity "warning"
> should be logged, sent as a snmp message or posted to m/monit not
> just logged. Well, monit does not support this flexibility yet, but I
> don't particularly want to maintain code that is orthogonal to such a
> design.

This is not so difficult to do. It would just have a deeper impact on
the code, and this is a thing I didn't want to do. But this is a thing
I could definitively cope with a relative simplicity.

I think that it should be done in a similar way:

---
set log default /log/monit.log
set log logfile1 path /log/log1.log
set log logfile2 path /log/log2.log

set log logfile1 format "%t %d [...etc...]"

set alert default alarmid auto severity warning
set alert logfile2 alarmid auto severity fatal

if failed port 80 protocol http then
    log logfile1 alarmid 33 severity warning

if failed port 81 protocol http then
    log logfile1 alarmid 34 severity fatal and
    log logfile 2 alarmid 34 and
    send snmp [etc....] and
    send email [etc....]
---

of course we would need snmp code etc.


> As for the implementation details with regards to logging and using
> several log files. You do not duplicate code, but write one and only
> one general Logger "class" which can be used in different contexts.
> This was what I had hoped for with regards to the patch. Pseudo code
> below shows what I had in mind.
[...]

This is a great way of projecting an application! for sure.


> Finally, I want to stress that your contribution is highly
> appreciated and I really hope that the above is not taken badly by
> you.
[...]

Absolutely not. As I said I am not touchy (not too much, just the
usual programmer's pride ;-)), and I don't need to be flattered or to
recieve compliments or similar things :-)

I love frankly discussion because they are much more usueful - we are
all clever person here.

I think that the value in Open Source is not just "keep the source and
use as you want", I want to share useful code. As I said, I will do my
best to leave a useful contribution.


> We *do* value any input especially those backed up by code.
> Likewise I hope you can appreciate that we as non-paid maintainers of
> the monit code may have a different agenda than you on some issues.
[...]

I really appreciate it. I think that useful programs like Monit should
be supported by paid developers.

Anyway I am really involved in my daily problems, travelling every
week from Florence to Milan and Dusseldorf is not so simple when you
are married with a child ;-) and I definitively need to be assigned to
such tasks since I don't have extended and calm spare time to dedicate
to it...


Bye!
-- 
Marco Ermini
http://www.markoer.org
Dubium sapientiae initium. (Descartes)
address@hidden # mount -t life -o ro /dev/dna /genetic/research
<< This message is for the designated recipient only and may contain
privileged or confidential information. If you have received it in
error, please notify the sender immediately and delete the original.
Any other use of the email by you is prohibited. >>