modoboa / modoboa-stats

Graphical statistics for Modoboa
MIT License
9 stars 12 forks source link

Statistics don't include alias domains #20

Closed mundschenk-at closed 7 years ago

mundschenk-at commented 7 years ago

Mails to alias domains are currently ignored by the log parser. They should be counted and filed in their parent domains bucket.

mundschenk-at commented 7 years ago

@tonioo What's your opinion regarding alias domains. Should they be counted as part of the domain they are aliasing or as separate domains in their own right? (I've got a partial patch for the latter behavior - i.e. the logfiles parsing has them as first-class citizens, but the corresponding graphs can't be selected via the GUI yet).

I'm not sure what would be more useful, either way needs some additional work.

mundschenk-at commented 7 years ago

Thinking some more, making the domain aliases full citizens doesn't mesh well with the way postfix handles them (orig_to= vs to=), so I dropped the idea in PR #21.

tonioo commented 7 years ago

@mundschenk-at I'd say we should make a distinction between domains and domain aliases. It could be useful in some cases, for example to remove an old domain alias when there is no more traffic on it. But maybe that's just an edge case... What do you think?

mundschenk-at commented 7 years ago

It's kinda hard to handle, because postfix/dovecot rewrites alias domains like this:

May 10 00:50:15 mail postfix/pipe[7829]: 7EBFB4675D: to=<office@some-other.domain>, orig_to=<office@some.domain>, relay=dovecot, delay=3.9, delays=3.9/0.01/0/0.04, dsn=2.0.0, status=sent (delivered via dovecot service)

We could of course try to match the orig_to first, compare with a self.alias_domains list first and only afterwards try to match to to self.domains. I'm just not sure its worthwhile.

tonioo commented 7 years ago

Indeed, it means the parser should detect lines containing different to and orig_to fields...

On 10/05/2017 18:10, Der Mundschenk & Compagnie wrote:

It's kinda hard to handle, because postfix/dovecot rewrites alias domains like this:

|May 10 00:50:15 mail postfix/pipe[7829]: 7EBFB4675D: to=office@some-other.domain, orig_to=office@some.domain, relay=dovecot, delay=3.9, delays=3.9/0.01/0/0.04, dsn=2.0.0, status=sent (delivered via dovecot service) |

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/modoboa/modoboa-stats/issues/20#issuecomment-300532967, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgc7sO7NOTiPL-eWeXJ8qSeEC7KUEDlks5r4eGSgaJpZM4NSxVX.

mundschenk-at commented 7 years ago

@tonioo Well, I'll see if I come up with something. I'll probably restructure the parser in the process. Or should we do that before adding the alias domain support to separate the concerns more cleanly? Either way is fine by me.

tonioo commented 7 years ago

Restructuring the parser might be a good idea but what's the target? Do you include modifications to address #18?

mundschenk-at commented 7 years ago

That and make the long list of ifs in the _parse_line method more readable by splitting it into more tightly focused units.

mundschenk-at commented 7 years ago

@tonioo I've begun the restructuring in #22. Could you please have a look at line 230? What is the use of storing size zero? Before that dictionary entry ever gets used, it is overwritten by the enqueuing, no? I think the whole if block can be skipped.

tonioo commented 7 years ago

The link is broken, I've you found the answer alone?

mundschenk-at commented 7 years ago

It's a different line now, but I've let it unchanged for the moment. What's more important, displaying or generating the graphs for domain aliases does not seem to work right and I'm not sure if that's a fault in my code or just a quirk in the sampling algorithm.

I've got a domain alias that basically gets no traffic at all, unless I specifically send messages to it. I did that and the line appears to parsed properly from the logfile. However, the graph is still empty (and all y values in the JSON are 0).

mundschenk-at commented 7 years ago

@tonioo I've updated the PR to reflect that I believe it is ready for review/inclusion. The graph problem seems to be gone after sending some more test mails, so I assume it's been a sampling quirk.

mundschenk-at commented 7 years ago

Fixed by #22