bbolt abstraction for statistic tracking

[zackattack01]

This is the final bit of work towards removing the direct bbolt DB connection references from knapsack.
The remaining references are a bit different than the prior abstraction work so far - these are all held to pull stats from bbolt. This is done at a global (not bucket) level for the most part, so utilizing the same KVStore pattern here did not make sense.

Instead, this replaces the direct db connection embedded in knapsack with a new interface type: StorageStatTracker.
The interface is intentionally vague (requires only that implementers can return a SizeBytes int64 and GetStats json blob, because all current utilizations were tightly coupled to the bbolt logic itself. For now this seemed like the cleanest way to get where we want without overcomplicating potential future work (e.g. replacing bbolt usage in tests, changing storage methodology, etc.)

[directionless]

If I read this correct, I think:

1. there's a new statsstore thing
2. which has a GetStates method that returns []byte I assume this is intended for use in a table/flare/etc.
3. (It's really the old logic from agent.GetStats)
4. This is wired into knapsack, passed in via the initialization, and returned bare with StorageStatTracker()

This feels a little weird to me. I'm not sure I have a simple suggestion though, I think it's highlighting something awkward that already existed. But it feels like this logic is spanning a bunch of places. And I wonder:

• Is it cleaner to keep passing the DB to knapsack, and puts the GetStats implementation there? (Probably not, knapsack tends to be a simple wrapper)
• What would it look like if GetStats was part of the stores interface, they already have the db there. This feels good, but also that it might be a little awkward if we have multiple databases

I'm curious if you have more thoughts

[directionless]

Suggested change

	type Stats struct {
	type stats struct {

I think? Or maybe even make it anonymous in the GetStats method?

[zackattack01]

yeah i'll get that cleaned up, ty!

[zackattack01]

@directionless

If I read this correct, I think...

This is all correct, the stats are currently used in slightly different ways across 2 flare checkups and in a table

Is it cleaner to keep passing the DB to knapsack, and puts the GetStats implementation there? (Probably not, knapsack tends to be a simple wrapper)

I came to the same conclusion here- it seemed like an unnecessarily complex bit of logic to push up there and makes it just as easy to abuse usage going forward

What would it look like if GetStats was part of the stores interface, they already have the db there. This feels good, but also that it might be a little awkward if we have multiple databases

This was my instinct too (and initial approach). Things got messy for a couple reasons:

• all of the KVStore abstractions so far are easily swappable with our current in memory implementation. For either the bbolt or in memory implementations, the implementer cares about a single bucket
• The checkup info will include bucket level statistics, but there is also a notion of global bbolt db statistics - keeping it with Stores ended up meaning we needed one store that acted as a virtual "no-bucket" store just to read global statistics without breaking all existing patterns.
  • while there are ways to do this so the code looks clean, I could not find ways that I would want to debug in a year 😀
  • none of the functionality here seems critical path so I really tried to avoid overcomplicating the nice clean Stores

Note that my opinion might be different if we decided that we could do away with the global stats, and instead just ask each store for GetStats - This logic assumes that the kolide_launcher_db_info should not be changed at all

[directionless]

I think if this all feels too complex to both of us, we should explore some other ways to get at it.

My hunch is that we should expose it via knapsack. But I agree with you about all the ways it's messy