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

Add IORuntimeMetrics #861

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iRevive
Copy link
Contributor

@iRevive iRevive commented Dec 2, 2024

This implementation utilizes typelevel/cats-effect#3317.

How to use:

object Main extends IOApp.Simple {
  def program(
    meterProvider: MeterProvider[IO],
    tracerProvider: TracerProvider[IO]
  ): IO[Unit] = ???
  
  def run: IO[Unit] =
    OtelJava.autoConfigured[IO]().use { otel4s =>
      implicit val mp: MeterProvider[IO] = otel4s.meterProvider
      IORuntimeMetrics
        .register[IO](runtime.metrics, IORuntimeMetrics.Config.default)
        .surround {
          program(otel4s.meterProvider, otel4s.tracerProvider)
        }
    }
}

A few things to discuss

1. The name org.typelevel.otel4s.instrumentation.ce.IORuntimeMetrics clashes with the cats.effect.unsafe.metrics.IORuntimeMetrics

2. Any alternatives to the org.typelevel.otel4s.instrumentation.ce package?

Perhaps org.typelevel.otel4s.instrumentation.catseffect fits better?

@iRevive iRevive changed the title Add IORuntimeMetrics [WIP] Add IORuntimeMetrics Dec 2, 2024
@mergify mergify bot added documentation Improvements or additions to documentation module:sdk Features and improvements to the sdk module metrics Improvement to metrics module tracing Improvements to tracing module labels Dec 2, 2024
@@ -89,7 +89,7 @@ object AutoConfigure {
* @tparam A
* the type of the component
*/
abstract class WithHint[F[_]: MonadThrow, A](
abstract class WithHint[F[_]: MonadCancelThrow, A](
hint: String,
configKeys: Set[Config.Key[_]]
) extends AutoConfigure[F, A] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iRevive iRevive force-pushed the instrumentation/ce-runtime-metrics branch from bb51631 to 44cc8a8 Compare December 2, 2024 07:46
@iRevive
Copy link
Contributor Author

iRevive commented Dec 2, 2024

@armanbilge wanna take a look?

Comment on lines 43 to 45
def withComputeMetrics: Config
def withoutComputeMetrics: Config
def withComputeMetricsAttributes(attributes: Attributes): Config
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The config feels verbose. On the other hand, we can easily add support for new metrics (e.g., typelevel/cats-effect#4187) in a binary-compatible way.

@iRevive iRevive added the breaking The changes are semantically or binary breaking label Dec 2, 2024
@iRevive iRevive force-pushed the instrumentation/ce-runtime-metrics branch from 44cc8a8 to bd108b5 Compare December 6, 2024 07:56
@iRevive iRevive force-pushed the instrumentation/ce-runtime-metrics branch from bd108b5 to a9b0275 Compare December 6, 2024 11:54
@iRevive iRevive changed the title [WIP] Add IORuntimeMetrics Add IORuntimeMetrics Dec 6, 2024
@iRevive
Copy link
Contributor Author

iRevive commented Dec 6, 2024

I tweaked the configuration definition, looks good so far.

Patiently waiting for CE 3.6.0 release now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking The changes are semantically or binary breaking documentation Improvements or additions to documentation metrics Improvement to metrics module module:sdk Features and improvements to the sdk module tracing Improvements to tracing module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant