[Fix] Render datalayer.Model and ScoredModel in structured logs

[davidbreitgand]

/kind bug

What this PR does / why we need it:

Fixes rendering Model and ScoredModel in structured logs.

datalayer.Model and ScoredModel serialized to {} and {"Score":X} with the name dropped in DEBUG log lines because the
concrete types have only unexported / embedded-interface fields.

This PR adds
MarshalJSON on *model projecting name, and on *ScoredModel projecting
name + score so the promoted embedded method does not silently drop
scores.

Attributes are intentionally not projected because AttributeMap is itself an
interface backed by an unexported sync.Map and would recurse into the
same {} problem.

Fixes #280

NONE

[davidbreitgand]

/cc @nirrozenbaum

[nirrozenbaum]

thanks @davidbreitgand,
what are your thoughts about implementing String() instead of MarshalJSON?

[davidbreitgand]

thanks @davidbreitgand, what are your thoughts about implementing String() instead of MarshalJSON?

Thanks @nirrozenbaum. Actually, I considered String() :) But it doesn't address the actual failure mode here. The problem is that the broken log sites all go through structured logging, not %v like format.

It goes like logger.Info(..., "result", result) / "scoredModels", scored), which serializes values with encoding/json. And encoding/json does not consult fmt.Stringer. Rather it only looks at exported fields or a MarshalJSON method. So adding String() on *model would leave rendering {}.

The problem also shows up when datalayer.Model is nested inside another struct (PipelineRunResult.TargetModel, []ScoredModel). The outer struct is JSON-marshaled by the logger, so the child must implement MarshalJSON, because String() on the child is not called in that path.

MarshalJSON also preserves the structured shape ({"name":"..."}, {"name":"...","score":...}), which stays greppable and "jq-able" in log tooling. String() blob inside a JSON field would be actually clumsy for that.

Does this make sense to you?

Having said that, I'm happy to add String() in addition if you have %v format style call sites in mind, but it can't replace MarshalJSON for the reported bug. Do you have a specific site where String() would be the right fix in the scope of this PR?

[davidbreitgand]

@nirrozenbaum can you please take another look?

[github-actions]

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.