Proposal: Add ListValue and NoneValue to TraceValue ADT

[bpholt]

I propose adding two new types to the TraceValue ADT, which currently consists of

https://github.com/typelevel/natchez/blob/fcc2c923d6cf8a1af6746fbb94fa77a11a2b5ced/modules/core/shared/src/main/scala/TraceValue.scala#L7-L15

These new types would support two use cases that would be a helpful bridge to otel4s.

ListValue

case class ListValue(value: List[TraceValue]) extends TraceValue

ListValue contains a list of TraceValues and can be used by backends that have first-class support for list/arrays, such as OpenTelemetry and X-Ray.

My immediate use for this is to support adding the aws.xray.annotations attribute list, which X-Ray uses to signal that fields should be indexed and not simply recorded. Indexing makes them searchable, which helps tremendously in trace discoverability. Importantly, it does not work to serialize the list as a stringified JSON array. The backend must see aws.xray.annotations: Slice(["foo","bar","baz"]) and not e.g. aws.xray.annotations: Str(["foo", "bar", "baz"]).

Other backends can serialize these values to a stringified JSON array or just mkString(", ").

NoneValue

case object NoneValue extends TraceValue {
  def value: Nothing = throw new IllegalStateException("Cannot extract value from NoneValue")
}

Adding NoneValue allows generic tracing instrumentation to more easily add optional values to spans using a new [A: TraceableValue]: TraceableValue[Option[A]] instance that uses TraceableValue[A] on Some and NoneValue on None.

This is helpful when semi-autogenerating instrumentation for an final-tagless encoded trait using cats-tagless Aspect[Foo, TraceableValue, TraceableValue], because the compiler requires an instance of TraceableValue for all the domain and codomain types on the trait. e.g.

trait Foo[F[_]]:
  def foo(opt: Option[Int]): F[Option[String]]

would require TraceableValue[Option[Int]] and TraceableValue[Option[String]] in scope. We currently handle this by emitting StringValue("None"), but it would be nice to have the option to omit these values entirely.

Issues

Does this require a major version bump? MiMa doesn't report any issues, but adding new cases to the ADT could result in MatchErrors if any downstream code is pattern matching on TraceValue. I thought I remembered reading somewhere that was discouraged, but it's not forbidden by the types, so maybe we should accept that people might be doing it. This might not be worth doing if it requires a major version bump to Natchez. (I know I would prefer to spend time migrating to otel4s as opposed to managing a major upgrade here.)

NoneValue has a suitable alternative today, but I'm not sure it's possible to do what ListValue would enable without it.