:Issue 234: Split into reusable packages.

[davidsonff]

Signed-off-by: Frank Davidson frank_davidson@manulife.com

Hi,

I tried to deal with Issue #234. I don't have access to a StatsD server or anything (I'm actually working towards fixing the similar Graphite Exporter problem).

This passed all the tests. Let me know if there are things to fix here.

Thank you for all your hard work!

Frank

[SuperQ]

It looks like you're using an editor or environment that is re-formatting all files with DOS carriage-return. Please update your environment settings and revert those changes.

[davidsonff]

Just tried to fix the line endings.

[matthiasr]

I think you're our first Windows contributor in this repository 😄 sorry for all the things we are not handling properly in that case. Could you also rebase out the statsd_exporter.exe and gitignore it?

[matthiasr]

Thank you so much for tackling this!

The ~ files are making the diff tricky, I'm going to do another review pass when those are out. Could you also take a look at the failing tests?

[matthiasr]

Thank you for fixing the newlines! I'm quite happy with this already, only a few more comments on code organization.

The more I see them the more I think passing all the metrics individually is not viable. Let's make them fields on the relevant structs. I am wondering if there is a nice way to reduce the boilerplate for the caller without making packages mess with global state …

[davidsonff]

Working on all of this. Might take a bit of time... Busy!

[matthiasr]

awesome, thank you for checking in!

[davidsonff]

I don't seem to be able to get the DCO check to work. Not sure what the issue is.

[matthiasr]

I think it is taking issue with your author line:

commit 72c05d2e06e555b3011739d7416ce1bf9d92b2a8 (HEAD -> issue-234)
Author: = <=>
Date:   Wed Apr 8 14:59:19 2020 -0400

    Updated structs and tests.

    Signed-off-by: Frank Davidson <davidfr@americas.manulife.net>
    Signed-off-by: Frank Davidson <ffdavidson@gmail.com>

something seems to be off with your gitconfig.

[matthiasr]

Looks good! I want to merge this as is. We may iterate on the interfaces later; I'm not super happy with the amount of boilerplate around the metrics but also don't have a solution yet.

When fixing the DCO, could you also squash all the commits into one so we don't end up with the .exe in the history?

[matthiasr]

Great job! I did some superficial acceptance testing and found that the unixgram listener crashes the program, but otherwise it seems to be working just as before. Once you add the listener.StatsDUnixgramListener initialization it is ready to merge.

[matthiasr]

Thank you so much for getting this done!

[davidsonff]

@matthiasr I think maybe I should pass line.LineToEvents() as a function value into the listener package, i.e., pass the LineToEvent function as an argument into listener.HandleConn() for use by event.EventHandler()? I don't think I can replace it as is now in the Graphite Exporter? What do you think? That would make LineToEvents pretty swappable for different use cases?

[matthiasr]

That's a good idea, but does the listener have to know about events at all? I think we can disentangle the listener, event parsing, and collector even further. The listener package could only deal in "packages" (single lines or bundles of lines?), sending them through a channel to a parser goroutine (possibly multiple).

The chain would roughly be listener -> (statsd / graphite / influxdb / ???) parser -> event queue -> collector.

We already have a well defined interface from the parser to the event queue. What would be the best way to hand unparsed input to the parser? byte slices or strings? Single lines only, or whole packets? what would "a whole packet" mean in the context of TCP though?