-
Notifications
You must be signed in to change notification settings - Fork 546
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 Pino transport for OpenTelemetry #2110
Comments
Hello @mhennoch, what problem would moving the transport to this repo solve for the end users? Is there a way to do it while preserving all the contributions of the contributors to the original repo? |
Probably not, everyone who has contributed their instrumentation to upstream has lost the history. I actually have a somewhat working version of it. I can contribute that and you can review if it works for you. |
I plan to move the repo to pino org https://github.com/pinojs. It was the initial plan when I first created the transport but kept it in a private repo to move things faster. If now trust is becoming an issue maybe it is time to proceed with the plan. The package was already reviewed by pino maintainers. This will enable keeping the history while also adressing your concerns. Your clients are trusting pino org so they should trust the transport as well. I will do that next week P.S. Re trust, you can point your clients that the transport and all its dependencies have provenance https://www.npmjs.com/package/pino-opentelemetry-transport |
As long as quite some log related packages (as of now all) are experimental and in the 0.x.y version range release here and dependency update by consumers need to go hand in hand. If it is here this sort of updates/releases are handled once for all here. Don't know if this is a valid/good reason to add it here. |
Hello, Pino Opentelemetry Transport is now moved to @pinojs org https://github.com/pinojs/pino-opentelemetry-transport I created an issue with the description and the motivation for doing so here: Thanks |
That's a good point to add it to OTel contrib. We're trying to avoid duplicate dependencies as much as possible and having the instrumentation depend on an external release will definitely complicate things. |
Is your feature request related to a problem? Please describe
We should add Logs API support to Pino also now that Bunyan/Winston have it.
Describe the solution you'd like to see
Seems like Pino recommends doing this via transports. Ideally transports operate in a separate thread or a separate process which won't have access to globally set log provider. So we would need to duplicate and setup log provider inside the transport itself together with resource detection, processor/exporter configuration etc. I don't see a sane way of passing in custom processor/exporter to the transporter tho. Also need to make sure this works well with Pino instrumentation and auto instrumentation.
Pino already links to one OTEL transport. Maybe @Vunovati is willing to move it to contrib.
Are there any other ways of doing this besides using a transport?
The text was updated successfully, but these errors were encountered: