Support using Scala propagators with Java backend #365
Conversation
core/common/src/main/scala/org/typelevel/otel4s/context/propagation/PassThroughPropagator.scala
Show resolved
Hide resolved
core/common/src/main/scala/org/typelevel/otel4s/context/propagation/PassThroughPropagator.scala
Outdated
Show resolved
Hide resolved
| * `TextMapPropagator`s, and `ContextPropagators`, available through | ||
| * [[org.typelevel.otel4s.java.context.propagation.PropagatorConverters]]. | ||
| */ | ||
| trait AsJavaExtensions { |
There was a problem hiding this comment.
If we put implicit classes right into the PropagatorConverters object , we can define them as value classes:
implicit class TextMapGetterHasAsJava[A](private val getter: TextMapGetter[A]) extends AnyValIn some cases, it could be more optimized in runtime.
But I don't have a strong opinion on this, to be honest.
There was a problem hiding this comment.
But I don't have a strong opinion on this, to be honest.
neither do I. I just structured it the same way scala.jdk.CollectionConverters is structured. I have no attachment
There was a problem hiding this comment.
I say we leave it as is and change it later if needed
core/common/src/main/scala/org/typelevel/otel4s/context/propagation/PassThroughPropagator.scala
Outdated
Show resolved
Hide resolved
core/common/src/main/scala/org/typelevel/otel4s/context/propagation/PassThroughPropagator.scala
Show resolved
Hide resolved
NthPortal
force-pushed
the
scala-propagator-support/PR
branch
from
1d9d185 to
6fd1320
Compare
|
the docs for the conversions are copied/adapted from the stdlib. what's the correct way to cite that? |
I don't know, to be honest. Perhaps a note would be enough. |
@SethTisue do you know the answer to this? sorry to drag you in here 😬 |
|
I think what you've done — including a credit — is sufficient under the Apache license. |
|
thank you Seth! |
|
Thanks, sorry, don't have bandwidth to review this one. Thanks for all your work, please go ahead :) |
|
@NthPortal seems we are ready to merge. Let me know if there is something else you would like to do within the PR. |
|
I'll autosquash and then once the build succeeds, it's done 👍 |
Add `PassThroughPropagator` that works with any context implementation.
Change `TextMapPropagator` to always return the same instance when passed a `Seq` with exactly one element, even when that element is a `TextMapPropagator.Noop` instance.
Redesign converters between Java and Scala
`TextMap{Getter,Propagator}`s to follow the pattern from the Scala
standard library, supporting converting in either direction between
the types.
Remove unused conversion for `TextMapSetter`.
NthPortal
force-pushed
the
scala-propagator-support/PR
branch
from
58e58be to
c1565ad
Compare
Add conversions from Scala to Java propagators so that the Scala instances can be passed to builders of Java
OpenTelemetryinstances