zio / zio-telemetry

ZIO-powered OpenTelemetry library
https://zio.dev/zio-telemetry
Apache License 2.0
112 stars 55 forks source link

Allow direct access to trace and span id for zio-logging's LogFormat #842

Closed andrzejressel closed 3 months ago

andrzejressel commented 3 months ago

My usecase is https://togithub.com/zio/zio-telemetry/issues/558 but the other way around. I'm using zio-telemetry for trace/span management, but I use zio-logging to do actual logging.

I want to add trace-id and span-id to my logs, but there is no way to receive it from pure code (which LogFormat requires).

I also see some solution here: https://togithub.com/zio/zio-telemetry/pull/319#issuecomment-783105181, but I'm not sure which would be the prefered one.

grouzen commented 3 months ago

Hey, @andrzejressel! My opinion is that it should be done in a separate module (i.e. opentelemetry-zio-logging). Another person is working on adding a bunch of modules to integrate with different parts of the ZIO ecosystem: https://github.com/zio/zio-telemetry/issues/815. So, I think the zio-logging integration is a good candidate for extending the basic zio-opentelemetry module. This also will allow us to have a more seamless and tight API for zio-logging. - you may implement traceIdLogFormat and spanIdLogFormat right inside this new module.

andrzejressel commented 3 months ago

I don't understand why there are errors in OpenTelemetry.scala ...

grouzen commented 3 months ago

I don't understand why there are errors in OpenTelemetry.scala ...

Looking into it...

grouzen commented 3 months ago

I don't understand why there are errors in OpenTelemetry.scala ...

The errors disappear if you remove all mentions of opentelemetryZioLogging from the docs module. It can't find zio in the classpath during the execution of unidoc command. I don't know why. I think it is fine not to generate java/scala docs for this module for now. We will figure out it later, just need to leave a TODO comment in the build.sbt

andrzejressel commented 3 months ago
[error] Category: Class being called not found
[error]   In artifact: zio-parser_2.13-0.1.9.jar
[error]     In class: zio.parser.TupleConversion$
[error]       In method:  $anonfun$genTupleConversion$2(scala.reflect.macros.whitebox.Context, scala.reflect.api.Types$TypeApi):46
[error]       Call to: scala.reflect.api.Liftables$Liftable$.liftType()
[error]       Problem: Class not found: scala.reflect.api.Liftables$Liftable$
[error]       --------
[error]       In method:  $anonfun$genTupleConversion$2(scala.reflect.macros.whitebox.Context, scala.reflect.api.Types$TypeApi):46
[error]       Call to: scala.reflect.api.Liftables$Liftable.apply(java.lang.Object)
[error]       Problem: Class not found: scala.reflect.api.Liftables$Liftable
[error]       --------
[error]       In method:  $anonfun$genTupleConversion$2(scala.reflect.macros.whitebox.Context, scala.reflect.api.Types$TypeApi):46
[error]       Call to: scala.reflect.macros.whitebox.Context.universe()
[error]       Problem: Class not found: scala.reflect.macros.whitebox.Context

Cool ...

andrzejressel commented 3 months ago

Forgot to update documentation, I'll do that today

grouzen commented 3 months ago
[error] Category: Class being called not found
[error]   In artifact: zio-parser_2.13-0.1.9.jar
[error]     In class: zio.parser.TupleConversion$
[error]       In method:  $anonfun$genTupleConversion$2(scala.reflect.macros.whitebox.Context, scala.reflect.api.Types$TypeApi):46
[error]       Call to: scala.reflect.api.Liftables$Liftable$.liftType()
[error]       Problem: Class not found: scala.reflect.api.Liftables$Liftable$
[error]       --------
[error]       In method:  $anonfun$genTupleConversion$2(scala.reflect.macros.whitebox.Context, scala.reflect.api.Types$TypeApi):46
[error]       Call to: scala.reflect.api.Liftables$Liftable.apply(java.lang.Object)
[error]       Problem: Class not found: scala.reflect.api.Liftables$Liftable
[error]       --------
[error]       In method:  $anonfun$genTupleConversion$2(scala.reflect.macros.whitebox.Context, scala.reflect.api.Types$TypeApi):46
[error]       Call to: scala.reflect.macros.whitebox.Context.universe()
[error]       Problem: Class not found: scala.reflect.macros.whitebox.Context

Cool ...

sb-missinglink can't recognize the links during macro expansion for some reason, so we just need to ignore the entire scala.reflect package:

.settings(
    missinglinkIgnoreDestinationPackages += IgnoredPackage("scala.reflect")
  )
grouzen commented 3 months ago

@andrzejressel Thanks for your contribution! I'm ready to merge it once the above comments are addressed :+1:

andrzejressel commented 3 months ago

I've added this new module to README

andrzejressel commented 3 months ago

I don't like the zio logging docs now to be honest, I'll add simpler usage with the standalone example

andrzejressel commented 3 months ago

All right, should be fine now - we can always refine it later.