-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rely on a Netlink go implementation instead of conntrack command calls #15
Comments
The idea sounds great. What I happen to be doing right now is writing test cases (I expect to push them next weekend), because I want to refactor this software thoroughly. This exporter started as a quick write-once hack and the awful code quality showed its ugly head in the moment the first feature adding PR came in (and made it even worse). Secondary goal of the refactor is to get rid of the prometheus client library, because it is a huge monster of a dependency and its usage is hard to understand and contributes to weird code (in small projects like this). In other closed source projects of mine of similar size I tend to write OpenMetrics handlers directly and I want to transfer code here. Ideally this project has no dependencies at all, and I like the idea of swapping the I don't know how to progress from here right now, yet. The end goal would be to use the conntrack lib, remove prometheus-client and build statically on a distroless or even scratch container. But I'd recon that a first step would be to start converging towards filling "a proper Go struct" as a meeting point of both refactoring streams? I'll contemplate on that for the next days and invite you to do the same. |
Thanks for your answer, regarding Prometheus metrics, we can do a simpler implementation (see my attempt here: criteo-forks@ace1af9 please disregard the things I have commented in the main.go, it's just to give you an overview on how I would implement this). This is a much simpler approach, more readable IMO, and no need to redefine Collect() func etc... If we also use the conntrack lib, the code will be even simpler and easier to understand. |
That looks like a good step forward. I've cleaned up my testing code and pushed it to a new branch dev and used that to also change what the exporter package exports and what it doesn't. That gives a good hint on what I think should be in the main package and what shouldn't. Also it hides the fact that the metrics are generated by the prometheus client library (which I was to get rid of eventually). Hopefully my tests are robust enough to still pass when new labels (netns) are added. Would you be interested to shape your code towards the |
Sure I'll work on this soon. |
I'll push another PR for the Prometheus metrics based on criteo-forks@ace1af9 once #16 is approved and merged, I don't want to deal with a rebase full of merge conflicts :) |
I have pushed a "big refactoring." The sloc rised quite a bit, but despite that I think the code structure is a lot cleaner and easier to do adapt the library approach. I apologise if that ruins your rebase plans! |
This is a proposal to replace usage of
conntrack
tool to fetch statistics on the conntrack table, with a golang lib able to bring this information directly over the Netlink socket. The main benefit would be to get rid of the conntrack's output parsing.There is already a lib allowing to do that: https://github.com/ti-mo/conntrack
I have used it for another project and it seems pretty easy to get stats and have the result in a proper golang struct.
I can work on this but I'd like to have your opinion before rushing into it.
The text was updated successfully, but these errors were encountered: