open-fresh / bomb-squad

Automatic detection and silencing of high cardinality series in Prometheus.
Apache License 2.0
139 stars 12 forks source link

Add tests for current implementation so it can graduate from "Proof of concept" phase #10

Open cboggs opened 6 years ago

cboggs commented 6 years ago

Bomb Squad is currently very "proof of concept," and thus has no tests. At all. That's clearly not ideal, and should be addressed ASAP.

Note that most (all?) PRs will be put on hold until this has been completed. I'll try to devote some time this week, but that "day job" thing and all... :-)

Some key areas for initial tests:

cboggs commented 6 years ago

Sooo my hope with this bit of work was that I could (mostly) write tests without having to refactor much of the existing code, so that I could hopefully avoid introducing new weirdness.

Turns out - I wrote terribly untestable code the first time 'round. Yay! This is lame because it involves a bunch of refactoring just to write basic unit tests, but it's also awesome because now I know what to watch for as I go forward. :-)

I'll get a PR pushed shortly with the beginnings of the refactor + tests. Wish me luck!

pcarranza commented 6 years ago

@cboggs would you be kind enough to add one logging line to dump the metrics that we get from Prometheus, and then run the demo again to capture one runaway metric?

This way we can use this as a test fixture to ensure that whatever we do the tool keeps behaving nicely.

I plan inserting those fixtures here

cboggs commented 6 years ago

Oh man, I totally missed this! Sorry @pcarranza.

Glad to grab whatever log lines you need. ~~ but I'm not sure I understand exactly what you'd like. Not sure what you mean by dumping the metrics we get from Prometheus (I don't recall ever making bomb squad show any prometheus inspection data).~~

Once I bothered to look at the spot you want to insert the expected strings, I think I know what you need. :-) Will try to get that once I get the current PR buttoned up.

The runaway metrics are listed as a simple metricName.labelName concatenation, if that helps.

cboggs commented 6 years ago

23 begins down the path of refactoring bomb squad to use proper k8s client-go packages, and thus makes it possible to test the k8s interactions. This should satisfy the requirement to test the current incarnation of config management (as it's still k8s-only for now).