opentelemetry-go-contrib
opentelemetry-go-contrib copied to clipboard
Logging bridge for logr
Towards #5192
Looking to do benchmarks in a different PR
Notes:
- slogSink converts any non string value to
slog.Anywhich is handled with fmt.Sprintf("%+v", v) by otelslog. This PR does not do the above and instead opts to useconvertValueto get the convert based on type which maybe less performant 🤔 convertValue- follows how funcr handle interface{}- currently it's designed to convert nested values recursively with
convertValue, is there a max depth for converted attributes? kvBuffer- is used in otelslog, I propose to move kvBuffer to internal pkg and reused it here too?
Codecov Report
Attention: Patch coverage is 99.45055% with 1 line in your changes missing coverage. Please review.
Project coverage is 65.8%. Comparing base (
b058429) to head (beae842). Report is 350 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| bridges/otellogr/logsink.go | 99.4% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #5357 +/- ##
=======================================
+ Coverage 65.3% 65.8% +0.4%
=======================================
Files 203 204 +1
Lines 12780 12962 +182
=======================================
+ Hits 8348 8529 +181
- Misses 4189 4190 +1
Partials 243 243
| Files with missing lines | Coverage Δ | |
|---|---|---|
| bridges/otellogr/logsink.go | 99.4% <99.4%> (ø) |
Hello, I was wondering what the state of this PR is. @scorpionknifes are you still planning on getting this merged or would you like some help to get it across the finish line? Any out-of-band communication worth documenting here?
Hello, I was wondering what the state of this PR is. @scorpionknifes are you still planning on getting this merged or would you like some help to get it across the finish line? Any out-of-band communication worth documenting here?
Oops, kinda forgot about this PR, shall try get to it by this weekend 🙏
@scorpionknifes After chatting with @pellared we think it would be easier for us to review this PR if you split it into smaller ones, like we did for zap and zerolog.
If you want an example, you can look at the various
- otelzap PRs: https://github.com/open-telemetry/opentelemetry-go-contrib/pulls?q=is%3Apr+is%3Aclosed+otelzap+
- otelzerolog PRs: https://github.com/open-telemetry/opentelemetry-go-contrib/pulls?q=is%3Apr+author%3AAkhigbeEromo+is%3Aclosed+zerolog
Changing this PR to draft per https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5357#issuecomment-2257740377.
Use this PR as a reference for new PRs.
@scorpionknifes, are you able to follow up with creating smaller PRs or can we use your PR to start creating them ourselves (we can still add you as code owner)?
I apologize for being slow on this. I’ll look at it over the weekend. I’ll keep you posted! Thanks for your patience