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

Move SpanRunner outside SpanBuilderImpl #122

Merged
merged 3 commits into from
Feb 12, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import com.typesafe.tools.mima.core._

ThisBuild / tlBaseVersion := "0.1"

ThisBuild / organization := "org.typelevel"
Expand Down Expand Up @@ -179,6 +181,26 @@ lazy val `java-trace` = project
"io.opentelemetry" % "opentelemetry-sdk-testing" % OpenTelemetryVersion % Test,
"org.typelevel" %%% "cats-effect-testkit" % CatsEffectVersion % Test,
"co.fs2" %% "fs2-core" % FS2Version % Test
),
mimaBinaryIssueFilters ++= Seq(
ProblemFilters.exclude[MissingClassProblem](
"org.typelevel.otel4s.java.trace.SpanBuilderImpl$Runner"
),
ProblemFilters.exclude[MissingClassProblem](
"org.typelevel.otel4s.java.trace.SpanBuilderImpl$Runner$"
),
ProblemFilters.exclude[MissingClassProblem](
"org.typelevel.otel4s.java.trace.SpanBuilderImpl$TimestampSelect"
),
ProblemFilters.exclude[MissingClassProblem](
"org.typelevel.otel4s.java.trace.SpanBuilderImpl$TimestampSelect$"
),
ProblemFilters.exclude[MissingClassProblem](
"org.typelevel.otel4s.java.trace.SpanBuilderImpl$TimestampSelect$Delegate$"
),
ProblemFilters.exclude[MissingClassProblem](
"org.typelevel.otel4s.java.trace.SpanBuilderImpl$TimestampSelect$Explicit$"
)
Comment on lines +185 to +203
Copy link
Member

Choose a reason for hiding this comment

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

None of this code would be inlined by the macros, right? This just bit Circe on Scala 3...

I have a feeling our next release will be 0.2, but as long as we're not changing the public API, this is a noble effort.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, these classes shouldn't appear in the inline positions

)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ trait Span[F[_]] extends SpanMacro[F] {
*
* Only the timing of the first end call for a given span will be recorded,
* the subsequent calls will be ignored.
*
* The end timestamp is based on the `Clock[F].realTime`.
*/
final def end: F[Unit] =
backend.end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,7 @@ object SpanBuilder {

def addAttributes(attributes: Attribute[_]*): Builder = this

def addLink(
spanContext: SpanContext,
attributes: Attribute[_]*
): Builder = this
def addLink(ctx: SpanContext, attributes: Attribute[_]*): Builder = this

def root: Builder = this

Expand All @@ -196,7 +193,7 @@ object SpanBuilder {

def withStartTimestamp(timestamp: FiniteDuration): Builder = this

def build = new SpanOps[F] {
def build: SpanOps.Aux[F, Result] = new SpanOps[F] {
type Result = Res

def startUnmanaged(implicit ev: Result =:= Span[F]): F[Span[F]] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ trait SpanOps[F[_]] {
* This strategy can be used when it's necessary to end a span outside of the
* scope (e.g. async callback). Make sure the span is ended properly.
*
* If the start timestamp is not configured explicitly in a builder, the
* `Clock[F].realTime` is used to determine the timestamp.
*
* Leaked span:
* {{{
* val tracer: Tracer[F] = ???
Expand Down Expand Up @@ -55,9 +58,13 @@ trait SpanOps[F[_]] {
* The finalization strategy is determined by [[SpanFinalizer.Strategy]]. By
* default, the abnormal termination (error, cancelation) is recorded.
*
* If the start timestamp is not configured explicitly in a builder, the
* `Clock[F].realTime` is used to determine the timestamp.
*
* `Clock[F].realTime` is always used as the end timestamp.
*
* @see
* default finalization strategy [[SpanFinalizer.Strategy.reportAbnormal]]
*
* @example
* {{{
* val tracer: Tracer[F] = ???
Expand All @@ -69,8 +76,33 @@ trait SpanOps[F[_]] {
*/
def use[A](f: Result => F[A]): F[A]

/** Starts a span and ends it immediately.
*
* A shortcut for:
* {{{
* val tracer: Tracer[F] = ???
* val ops: SpanOps.Aux[F, Span[F]] = tracer.spanBuilder("auto-span").build
* ops.use(_ => F.unit) <-> ops.use_
* }}}
*
* @see
* See [[use]] for more details regarding lifecycle strategy
*/
Copy link
Member

Choose a reason for hiding this comment

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

Ah, thank you for adding these, too!

def use_ : F[Unit]

/** Starts a span, runs `fa` and ends the span once `fa` terminates, fails or
* gets interrupted.
*
* A shortcut for:
* {{{
* val tracer: Tracer[F] = ???
* val ops: SpanOps.Aux[F, Span[F]] = tracer.spanBuilder("auto-span").build
* ops.use(_ => fa) <-> ops.surround(fa)
* }}}
*
* @see
* See [[use]] for more details regarding lifecycle strategy
*/
def surround[A](fa: F[A]): F[A]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package java
package trace

import cats.effect.Sync
import cats.syntax.flatMap._
import io.opentelemetry.api.trace.{Span => JSpan}
import io.opentelemetry.api.trace.{StatusCode => JStatusCode}
import org.typelevel.otel4s.Attribute
Expand Down Expand Up @@ -89,7 +90,7 @@ private[java] class SpanBackendImpl[F[_]: Sync](
}

private[otel4s] def end: F[Unit] =
Sync[F].delay(jSpan.end())
Sync[F].realTime.flatMap(now => end(now))
Copy link
Contributor Author

@iRevive iRevive Feb 8, 2023

Choose a reason for hiding this comment

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

Technically, only startUnmanaged is affected. For resource-like API, we were injecting the realtime under the hood. E.g.
https://github.com/typelevel/otel4s/pull/122/files#diff-988bfe14df2f23c008240b2752bde4a711b0c4fa716b62f5416f1de82582bc55L330-L333


private[otel4s] def end(timestamp: FiniteDuration): F[Unit] =
Sync[F].delay(jSpan.end(timestamp.length, timestamp.unit))
Expand Down
Loading