Skip to content
This repository has been archived by the owner on May 10, 2023. It is now read-only.

Statistics don't include alias domains #20

Closed
mundschenk-at opened this issue May 6, 2017 · 13 comments
Closed

Statistics don't include alias domains #20

mundschenk-at opened this issue May 6, 2017 · 13 comments

Comments

@mundschenk-at
Copy link
Contributor

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

@mundschenk-at
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Member

tonioo commented May 10, 2017

@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
Copy link
Contributor Author

mundschenk-at commented May 10, 2017

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
Copy link
Member

tonioo commented May 10, 2017 via email

@mundschenk-at
Copy link
Contributor Author

@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
Copy link
Member

tonioo commented May 12, 2017

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

@mundschenk-at
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Member

tonioo commented May 14, 2017

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

@mundschenk-at
Copy link
Contributor Author

mundschenk-at commented May 14, 2017

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

Fixed by #22

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants