Make user-facing logs more consistent and testable

[blairconrad]

In #175 and #182, I changed a bunch of log messages and altered tests to verify log message that exist to provide always-visible user feedback.

@tummychow, at the time you mentioned

i'd like to see a refactor to use SlogValue

and

having typed logging objects would make the user-facing logs more consistent and testable. eg right now you have to write out the log message once at the callsite where it's logged, and again at the place where you're testing it. it would be better to get those things into a single place. if you can't get it working, that's fine, but i think it's worth an attempt

[blairconrad]

I replied to the above comments:

I'll see what I can do

That was nearly a month ago, and you've not seen any progress.

There are two major reasons for this (aside from # of hours in a day):

1. I remain a rust newbie and struggle with the capabilities and limitations of the language. Things that I'd like to try that would be a few minutes' work in C#, python, or even Java take an hour or more to attempt in Rust
2. I do not what exact target to shoot for. "Typed logging objects" is easy to understand on the surface, but the difficulty is in the details. I could imagine this means that the info+ messages (at least) each are different struct, which would determine the overall format of the message, and each struct would have 0 or more mandatory attributes that would provide additional information to the log message. But maybe this is not your vision

These two factors reinforce each other to impede progress. Working quickly and easily toward a slightly nebulous goal would let me show a prototype for critique. Working slowly toward a defined goal might take a while, but at least I'd see where I has headed and likely end up close to your vision once the time was put in.

I don't mind putting the time in. I've done so, trying a couple of approaches before discarding.
From other comments, I know that you have little free time to devote to git-absorb, so asking you to trade your time for mine (in the service of the project, but still) rankles, but I think if I'm going to make progress, I would need a little more guidance about what you're aiming for.

I mentioned a minor reason above. I'll explain it so you have full background. And it does depend on my guess about your preferred direction. If that's off, my whole ball of feelings unravels!

In my heart of hearts, I don't believe in the goal I guessed at above. (That doesn't mean I won't implement it if you do.)

I believe in typed logging objects that help ensure consistency and that would reduce developer error (although if each object type is used in one location, I am not sure it helps that much), but I don't think it would aid testability, unless the tests were able to read the objects off a stream, effectively checking their type. But this approach makes me uneasy. It reminds me of tests I've seen that use constants defined in production code to evaluate an outcome. The test passes because observed output contains the constant. But a developer accidentally introduced a typo in the constant. The test continues to pass because there was a single (now flawed) source of truth, but the production code has an error. I fear that relying on types defined in production would have a similar risk.

(One could argue that the current tests, which check the structure of a json object that's fed into the logger, might fall prey to this as well; the logger could decide not to emit some attributes or combine them in a weird way. But that's maybe a little paranoid, and the only way I think around it would be to write tests that drove git-absorb from the outside.)

Man. That should've been a blog post. All that to say that I've found in the past that being clever trying to reduce duplication between production and test code doesn't always yield the benefits we've hoped it would, and that a change to the production code and a search-and-replace in test code has served me as well. But experiences differ; maybe it depends on the domain, environment, scope of the project, developers involved, etc. For a small project with a benevolent dictator, perhaps code-sharing is the way to go.

It sounds like I'm trying to fight against a change. I'm not. It's your project, and I feel bad that the changes in #175 and #182 have introduced something in the tests that you don't love. I would like to work to improve the code. I just need a nudge.

[tummychow]

I could imagine this means that the info+ messages (at least) each are different struct, which would determine the overall format of the message, and each struct would have 0 or more mandatory attributes that would provide additional information to the log message.

yeah, this is basically what i'm thinking. i want to know, at compile time, what logs are "important" and what their fields are. not having to parse json in tests is a bonus. fwiw, it's not like i'm going to reject all subsequent prs if you can't get this to work. give it the old college try

[blairconrad]

@tummychow, I think the user-facing logs are a little more consistent(ly generated). Whether they're more testable, I don't know. But I think #194 is the end of the road for me, unless you want specific changes. Closing for now.