libcnb-test: Stop exposing raw output in LogOutput

[edmorley]

Since:

• There are no common use-cases that require raw output, and buildpacks in the wild aren't using these fields (checked using GitHub code search).
• It saves beginners from having to think about which LogOutput field they should be using.

This cleanup has been split out of the future Bollard migration PR.

GUS-W-13841502.

[Malax]

I'm on the fence with this one. We're doing a lossy conversion for the other ways to access the output. If it would ever cause problems (even tho it's currently fine), it would be impossible to get the correct data for a workaround and/or debugging. Given that the cost of having the raw fields seems pretty low to me, I'd rather keep them around.

[edmorley]

@Malax I'd rather not add (or continue to have) features "just in case", and instead wait until we have an actual use-case, since:

• it's trivial to re-add
• if there is ever a feature request/use-case for this, it might mean there is an even better way to support the use-case than just offering these raw fields, but without someone opening a feature request issue, we'll never know
• there's a cognitive burden to beginners in having multiple fields
• as you mentioned, it's inconsistent with the Pack CLI output, which only exposes the non-raw version

In general, I suspect that nine times out of ten, when we remove a feature like this (or don't add it in the first place), we'll never need it in the future. If we never have to add extra features, or add features back, then IMO it shows we're constantly over-scoping the design.

[Malax]

I'm with you in general wrt to feature creep and not having features "just in case". However, in this case the "feature" is exposing two fields with data we have to acquire regardless of exposing them or not. We're not adding complexity (and therefore maintenance cost) to the implementation by keeping it around. At the same time, if we remove these two fields we remove all ability to get the actual canonical data. If this were just anther convenience helper no-one uses I would remove it too, but this is about the canonical data and for me tips the scale for me towards keeping it.

[edmorley]

What about the other points eg cognitive burden, inconsistency?

We're not adding complexity (and therefore maintenance cost)

In my Bollard rewrite (where the consume_container_log_output function gets removed), having these fields meant having to write another two lines of the new implementation. Whilst doing so is trivial, it is still another thing to think about, so it's not zero maintenance cost.

I feel like adding these fields was the right thing to do initially, since we weren't sure whether they'd be needed in the early days of libcnb-test. However, we've proven now that nobody actually uses them permanently in their tests - and I've heard of zero instances where they've been used for temporary debugging. (People can always run docker run to debug by hand too.)

One downside to us being hesitant to removing unused features/APIs/... is that I think it will mean we end up having to be more strict on adding features/APIs in the first place. ie: It's more likely I'll leave "changes required" on a PR in the future, if I think there's a lower chance of having an API simplified in the future.

[Malax]

After discussing this with @edmorley, I am fine with this PR at this point.

I still think having a way to get the canonical, non modified, data is useful to have. However, we have other bits in libcnb that deal with output streams that do not expose the raw output anyways. Ideally, we would have the ability to access it everywhere.

To unblock progress, I will approve this now.