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 slog log bridge #5138

Closed
pellared opened this issue Jan 16, 2024 · 11 comments · Fixed by #5335
Closed

Add slog log bridge #5138

pellared opened this issue Jan 16, 2024 · 11 comments · Fixed by #5335
Assignees
Labels
enhancement New feature or request

Comments

@pellared
Copy link
Member

pellared commented Jan 16, 2024

Add slog.Handler log bridge

  • The bridge factory should accept a logger provider and set the name of the bridge (e.g. go.opentelemetry.io/contrib/bridges/sloghandler) and its version as instrumentation scope
  • The bridge should not make any heap allocation when the application emits a log using slog with up to 5 non-complex (no slices, no maps) attributes. There should be "zero allocation" unit tests for this.
  • Calling WithGroup should result in nesting the attributes in a https://pkg.go.dev/go.opentelemetry.io/otel/log#Map so that it has similar behavior to slog.JSONHandler. See: https://go.dev/play/p/bjJp7A-56bf
  • The bridge needs to have benchmarks

Additional resources:

@jpkrohling
Copy link
Member

Recommended reading:

https://github.com/golang/example/blob/master/slog-handler-guide/README.md
https://opentelemetry.io/docs/specs/otel/ (especially the Bridge API part)

And perhaps before that, I'd also recommend getting familiar with OpenTelemetry, if you aren't already:
https://opentelemetry.io/docs/what-is-opentelemetry/

It might also help to understand what OpenTelemetry is by taking a look at the demo:
https://github.com/open-telemetry/opentelemetry-demo

@Sanket-0510
Copy link

I would like to start working over this to get the better and in depth understanding of open-telemetry/community#1865

Till now what I've understood is that too configure a logging handler for slog logging library in go.

The flow goes like this

  1. Select the atleast one logging library to bridge with OTel logger and exporter. ( in our case right now we are using Slog we can use any other third party logging library in go in that sense)
  2. Now here comes the use of Bridge API and its architecture which goes like this
    Logger Provider (in our case slog) ---> ( get the logs from slog logger ) ----> logger ------> (emit the logs ) -----> log Record

Am I on the correct path?

@pellared

This comment was marked as outdated.

@pellared

This comment was marked as outdated.

@MrAlias
Copy link
Contributor

MrAlias commented Feb 22, 2024

SIG meeting notes: plan is to add this bridge to contrib.

@pellared pellared transferred this issue from open-telemetry/opentelemetry-go Feb 22, 2024
@pellared pellared changed the title Add slog.Handler log bridge Add slog log bridge Feb 26, 2024
@jpkrohling
Copy link
Member

I've added this to the scope of a few internships, such as LFX and Outreachy. Please, talk to me before working on this one.

@pellared
Copy link
Member Author

pellared commented Feb 28, 2024

@jpkrohling I updated the description to make the issue more open for new contributors. However, we may want to start working on it sooner to be able to showcase it at KubeCon EU 2024.

@pellared pellared added good first issue Good for newcomers help wanted Extra attention is needed labels Feb 28, 2024
@jpkrohling
Copy link
Member

I think the LFX announcement happens today, and we'll start working on that right away.

@MrAlias MrAlias self-assigned this Feb 29, 2024
@pellared pellared removed good first issue Good for newcomers help wanted Extra attention is needed labels Feb 29, 2024
@pellared
Copy link
Member Author

We are already starting working on it 🚀

@pellared
Copy link
Member Author

pellared commented Apr 8, 2024

@jba, we have an initial version of otelslog.Handler here: https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/bridges/otelslog. GoDoc: https://pkg.go.dev/go.opentelemetry.io/contrib/bridges/otelslog. Are you willing to review it?

You can simply create a new GitHub issue with your feedback or even add some comments here.

You can also review and add comments here: #5314 as this is the PR that introduced most of the implementation.

@jba
Copy link

jba commented Apr 8, 2024

Thanks for including me. I made a couple of performance-related comments on the PR, not really important.
I didn't dig too deeply but everything looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants