opentelemetry-go-contrib icon indicating copy to clipboard operation
opentelemetry-go-contrib copied to clipboard

Logging bridge for Zap

Open khushijain21 opened this issue 1 year ago • 5 comments

Part of #5191

Some Notes: Basic Structure - Zapcore implements - Check - determines whether the supplied Entry should be logged - Write - Enabled - Sync - we don't use buffer to write - so no code here - With - returns a child core with provided context

This also implements zapcore.ObjectEncoder and zapcore.ArrayEncoder which decides how we would encode provided fields to log attrs - (the user can also pass their custom encoder) Some Notes about type mapping:

  • All Uint types are converted to Int64
  • All complex types are converted to equiv string
  • Everything else which uses zap.Any to pass context and if does not match supported primitive types - is handled by AddReflected method which converts the field to JSON

khushijain21 avatar Mar 14 '24 13:03 khushijain21

Codecov Report

Attention: Patch coverage is 71.92982% with 80 lines in your changes are missing coverage. Please review.

Project coverage is 62.1%. Comparing base (30ed923) to head (2989fcf). Report is 114 commits behind head on main.

:exclamation: Current head 2989fcf differs from pull request most recent head f60e0c3

Please upload reports for the commit f60e0c3 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5279     +/-   ##
=======================================
- Coverage   62.3%   62.1%   -0.3%     
=======================================
  Files        189     190      +1     
  Lines      11575   11673     +98     
=======================================
+ Hits        7221    7256     +35     
- Misses      4145    4205     +60     
- Partials     209     212      +3     
Files Coverage Δ
bridges/otelzap/config.go 76.6% <76.6%> (ø)
bridges/otelzap/core.go 89.0% <89.0%> (ø)
bridges/otelzap/test_helper.go 73.7% <73.7%> (ø)
bridges/otelzap/encoder.go 61.5% <61.5%> (ø)

... and 7 files with indirect coverage changes

codecov[bot] avatar Mar 14 '24 13:03 codecov[bot]

This module will need to be added to the project versions.yaml under the unreleased section.

MrAlias avatar Mar 22 '24 15:03 MrAlias

Kind of nit, but do the file names need to be prepended with zap_? That's not something we're doing in other packages, and this package only holds the zap bridge. So having the library name in the filename is a bit redundant.

dmathieu avatar Apr 03 '24 09:04 dmathieu

https://github.com/open-telemetry/opentelemetry-go-contrib/actions/runs/8595139722/job/23549314101?pr=5279

Please run make to check for CI build errors.

pellared avatar Apr 08 '24 05:04 pellared

Changing this to a draft PR as I assume this is a "reference/design PR".

pellared avatar May 06 '24 16:05 pellared