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

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

Merged
merged 14 commits into from
May 31, 2024

Conversation

andrzejressel
Copy link
Contributor

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.

@andrzejressel andrzejressel requested a review from a team as a code owner May 24, 2024 19:43
@grouzen
Copy link
Contributor

grouzen commented May 24, 2024

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: #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
Copy link
Contributor Author

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

@grouzen
Copy link
Contributor

grouzen commented May 26, 2024

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

Looking into it...

@grouzen
Copy link
Contributor

grouzen commented May 26, 2024

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
Copy link
Contributor Author

[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
Copy link
Contributor Author

Forgot to update documentation, I'll do that today

@grouzen
Copy link
Contributor

grouzen commented May 30, 2024

[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
Copy link
Contributor

grouzen commented May 30, 2024

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

@andrzejressel
Copy link
Contributor Author

I've added this new module to README

@andrzejressel
Copy link
Contributor Author

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

@andrzejressel
Copy link
Contributor Author

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

Copy link
Contributor

@grouzen grouzen left a comment

Choose a reason for hiding this comment

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

Brilliant!

@grouzen grouzen merged commit a18c1cd into zio:series/2.x May 31, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants