Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add TraceableValue typeclass and base implementations #667

Merged
merged 1 commit into from Nov 29, 2022

Conversation

bpholt
Copy link
Member

@bpholt bpholt commented Nov 28, 2022

Introduces a lawless typeclass that is responsible for converting between the domain types used by a traced application and the TraceValue type that is used to track attribute values that can be attached to spans.

Using a typeclass approach instead of relying on implicit conversions has a couple advantages. My original motivation was to allow for a form of semi-automatic instrumentation using cats-tagless's Weave effect, which requires method parameter types all be constrained by a user-specified typeclass. This requirement means that we can generically convert a value of type A: TraceableValue to a TraceValue in positions where the compiler cannot prove that the existing implicit conversions would have worked.

I recently published Dwolla/natchez-tagless to facilitate this kind of semi-automatic instrumentation. (If this PR is merged, I'd modify that library to remove its ToTraceValue typeclass and use the TraceableValue I'm proposing here.)

Another benefit comes with the use of refinement / opaque types, where the typeclass instance can format or redact a value before its inclusion as a span attribute.

@bpholt
Copy link
Member Author

bpholt commented Nov 28, 2022

I explored trying to use A: TraceableValue in most places where TraceValue is used in Natchez today, but ultimately concluded that it wasn't worth it. For example, the put and log methods on Span both take varargs of (String, TraceValue) tuples. Replacing that with (String, A) works in the simple case, but we'd have to stop supporting varargs in this way to propagate that change everywhere. e.g. the put in DDSpan.attachError could still work if we created several put methods with increasing arities (or introduced some shapeless-style shenanigans, like using ProductArgs to convert varargs to an HList). That gets trickier when the vararg is used to add an arbitrary list of attributes to the span. For the purposes mentioned above, I concluded it's not worth adding that extra complexity to clean up the TraceValue/TraceableValue distinction.

*
* @tparam A The type to be converted to `TraceValue`
*/
trait TraceableValue[A] { outer =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely open to bike-shedding over the names here. Right now I think TraceableValue is slightly better than ToTraceValue, which is what I used in our library. 🤷‍♂️

@mpilquist mpilquist merged commit 2d5aac3 into typelevel:main Nov 29, 2022
@bpholt bpholt deleted the TraceableValue branch November 29, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants