Add request, response attributers getter to client tracer #173
Conversation
| import trace4cats.optics.{Getter, Lens} | ||
| import trace4cats.{Span, ToHeaders} | ||
|
|
||
| trait ClientSyntax { | ||
| implicit class TracedClient[F[_]](client: Client[F]) { | ||
|
|
||
| @deprecated("Use liftTraceExtended instead", "0.14.1") |
There was a problem hiding this comment.
Because Scala doesn't allow overloading methods with default arguments, bincompat issue couldn't be solved here by making this method package private and overloading with the new method. Hence, this method is deprecated and the newer method had a different name. This also comes with the cost that people will be forced to change to the new method anyways. The alternate (maybe better) option here would be to introduce a breaking change.
There was a problem hiding this comment.
Given that trace4cats-kafka recently went with a breaking change and a release of the http4s integration should also include the open PR and hence the changes coming from CE 3.5, I think we can go for a breaking change and keep the code clean. If bincompat was taken seriously, we would require MiMa setup anyway.
There was a problem hiding this comment.
That makes sense, let's keep the code clean and go for a breaking change then.
| Getter[Response[G], Map[String, AttributeValue]](_ => Map.empty) | ||
| ) | ||
|
|
||
| def liftTraceExtended[G[_]]( |
There was a problem hiding this comment.
Not too sure of the naming here.
- Results in cleaner code
These changes would give feature parity (and a bit more) with trace4cats-sttp. Example use case needed in my team: set
peer.serviceattribute based on the request URL.Changes in the PR:
This PR is a shared effort with @BjoernJeschke