typelevel / otel4s

An OpenTelemetry library for Scala based on Cats-Effect
https://typelevel.org/otel4s
Apache License 2.0
168 stars 34 forks source link

Backend Has Private Abstract Methods #204

Closed ChristopherDavenport closed 4 months ago

ChristopherDavenport commented 1 year ago

https://github.com/typelevel/otel4s/blob/main/core/trace/src/main/scala/org/typelevel/otel4s/trace/Span.scala#L151-L152

These two lines make implementing a Backend which is necessary for creating a Span impossible out of this package.

Quick backdoor I am using as an interrim in case others need similar.

package org.typelevel.otel4s.backdoor

import org.typelevel.otel4s.trace.Span.Backend
import scala.concurrent.duration.FiniteDuration

abstract class BackdoorBackend[F[_]](endF: F[Unit], endF2: FiniteDuration => F[Unit]) extends Backend[F]{
  override def end: F[Unit] = endF
  override def end(timestamp: FiniteDuration): F[Unit] = endF2(timestamp)
}
rossabaker commented 1 year ago

Seems like that should be relaxed.

I'm wondering whether backend on Span should be protected instead. It could still be implemented by third parties, but I'm not clear what the use case for calling it directly from a Span handle would be.

iRevive commented 1 year ago

Unfortunately, backend must be public because it is used by macro: https://github.com/typelevel/otel4s/blob/main/core/trace/src/main/scala-2/org/typelevel/otel4s/trace/SpanMacro.scala#L139.

The unwrapped code is:

if (span.backend.meta.isEnabled) span.backend.meta.addAttributes(..$attributes) 
else span.backend.meta.unit
rossabaker commented 1 year ago

Ah, right.

I wish protected[foo] meant accessible within foo or subclasses of whatever defined it: leave things open to implementation and guard who else can call it.

Anyway, I think we may be stuck making these public.

iRevive commented 1 year ago

In the initial draft, end was private because I had two types of span: Auto and Manual. And Manual span exposed end method publicly. Since we no longer have this separation, I do not see any obstacles toward making end in Backend public.

AprilAtBanno commented 1 year ago

I wish protected[foo] meant accessible within foo or subclasses of whatever defined it

I have good news for you @rossabaker! as long as foo isn't this, it does!

https://github.com/scala/scala/blob/v2.13.10/src/library/scala/collection/mutable/ArrayBuffer.scala#L57-L58

scala> class Demo[A] extends collection.mutable.ArrayBuffer[A]() {
     |   def foo: Array[AnyRef] = array
     | }
class Demo

scala> (new Demo[String]).foo
val res0: Array[AnyRef] = Array(null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null)

In general, the set of scopes from which protected[X] is accessible is the union of the scopes from which private[X] and protected are accessible, with the exception of protected[this], which is... the intersection of the scopes of this and protected? I'm not entirely sure how to define that with set theory