Skip to content

Conversation

@antoniomo
Copy link

Hi,

This is a POC, just to ask if anyone would be interested on it.

Basically it allows me to do:

	// Replace JsonPb logger of zap with the gogo version
	grpc_zap.JsonPbMarshaler = &gateway.JSONPb{}

On my service, so then I can get proper payload logging with gogo types, such as *time.Time when using gogoproto.stdtime = true. In other words, it avoids this bug: gogo/protobuf#212

Since this is go-grpc-middleware, I guess as much compatibility with gogo as possible is desirable?

Let me know if this is interesting and if so, what should I change to make it acceptable to merge (maybe the same for Logrus logger, which would be trivial? Perhaps not import the runtime.JSONPb Marshaler from grpc-gateway to avoid that extra dependency, but have our own version?).

Thanks!

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@antoniomo
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@codecov-io
Copy link

Codecov Report

Merging #149 into master will not change coverage.
The diff coverage is 66.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #149   +/-   ##
=======================================
  Coverage   73.09%   73.09%           
=======================================
  Files          36       36           
  Lines        1349     1349           
=======================================
  Hits          986      986           
  Misses        314      314           
  Partials       49       49
Impacted Files Coverage Δ
logging/zap/payload_interceptors.go 84.61% <66.66%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfaf568...82cb20e. Read the comment docs.

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.

3 participants