modoboa/modoboa-stats

Statistics don't include alias domains

mundschenk-at opened this issue · 13 comments

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

@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.

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.

@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?

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 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.

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

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

@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.

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

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).

@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.

Fixed by #22