Use zio-http Client instead of sttp3 #691
Conversation
|
ISSUE #687 |
|
Hey! Thanks for this amazing contribution! Gonna review it soon. |
opentelemetry-example/src/main/scala/zio/telemetry/opentelemetry/example/ProxyApp.scala
Outdated
Show resolved
Hide resolved
opentelemetry-example/src/main/scala/zio/telemetry/opentelemetry/example/http/Client.scala
Outdated
Show resolved
Hide resolved
| package object example { | ||
|
|
||
| type Backend = SttpBackend[Task, ZioStreams with WebSockets] | ||
| type Backend = zio.http.Client |
There was a problem hiding this comment.
I think this one is redundant now. We needed this type alias before to hide the complexity of SttpBackend type.
| ZLayer.scoped { | ||
| ZIO.acquireRelease(AsyncHttpClientZioBackend())(_.close().ignore) | ||
| } | ||
| private val httpBackendLayer: TaskLayer[Backend] = zio.http.Client.default |
There was a problem hiding this comment.
Same as in opentelemetry:
I don't think we need this extra variable here anymore, it would be better to pass ZClient.default directly into .provide
| status <- JsonDecoder[Status] | ||
| .decodeJsonStream(response.body.asStream.map(_.toChar)) | ||
| .catchAll(_ => ZIO.succeed(Status.down("backend"))) |
There was a problem hiding this comment.
Same as in opentelemetry: https://github.com/zio/zio-telemetry/pull/691/files#r1202239119
| package object example { | ||
|
|
||
| type Backend = SttpBackend[Task, ZioStreams with WebSockets] | ||
| type Backend = zio.http.Client |
There was a problem hiding this comment.
Same redundant type alias.
|
Overall, LGTM! Just a few minor comments to address! It would be great to make sure that examples are working by following the instructions in the corresponding doc pages:
Thanks! |
| .safeApply(config.backend.host, config.backend.port) | ||
| .map(_.withPath("status")) | ||
| URL | ||
| .decode(s"http://${config.backend.host}/${config.backend.port}") |
There was a problem hiding this comment.
.decode(s"http://${config.backend.host}/${config.backend.port}")
Should be changed to (semicolon between host and port instead of a slash):
.decode(s"http://${config.backend.host}:${config.backend.port}")
| ) | ||
| status = response.body.getOrElse(Status.down("backend")) | ||
| request = Request | ||
| .get(url) |
There was a problem hiding this comment.
url is missing the path:
.get(url.withPath("status"))
| ) | ||
| status = response.body.getOrElse(Status.down("backend")) | ||
| request = Request | ||
| .get(url) |
There was a problem hiding this comment.
url is missing the path:
.get(url.withPath("status"))
|
I have tested opentelemetry example 
|
|
And I have tested opentracing. Also it correct works. |
|
And I added zio http client to opentelemetry-instrumentation-example |
| @@ -67,7 +68,7 @@ object Dependencies { | |||
| Orgs.jaegertracing % "jaeger-client" % ExampleVersions.jaeger, | |||
| Orgs.jaegertracing % "jaeger-zipkin" % ExampleVersions.jaeger, | |||
| Orgs.softwaremillSttpClient3 %% "zio-json" % ExampleVersions.sttp3, | |||
| Orgs.d11 %% "zhttp" % ExampleVersions.zioHttp, | |||
| Orgs.d11 %% "zhttp" % ExampleVersions.zHttp, | |||
There was a problem hiding this comment.
I do believe we don't need this one anymore since we have just added a zio-http.
No description provided.