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

docs: add logging.InjectFields usage description #541

Merged
merged 1 commit into from
Mar 10, 2023

Conversation

aimuz
Copy link
Contributor

@aimuz aimuz commented Mar 8, 2023

Fixed #521

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this change. Did we remove the tags interceptor? If so, we should just remove it from the list, not keep it strikethrough.

@aimuz
Copy link
Contributor Author

aimuz commented Mar 9, 2023

The implementation of tags has been removed in v2.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, lets just remove this.

README.md Outdated
@@ -58,7 +56,8 @@ myServer := grpc.NewServer(

#### Logging

* [`tags`](interceptors/tags) - a library that adds a `Tag` map to context, with data populated from request body
* ~~[`tags`](interceptors/tags) - a library that adds a `Tag` map to context, with data populated from request body.~~ replace `tags.Extract().Set()` to `logging.InjectFields()`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets just remove this line

@aimuz aimuz force-pushed the fix-521 branch 2 times, most recently from 005a678 to 692a961 Compare March 10, 2023 03:04
Signed-off-by: aimuz <mr.imuz@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2023

Codecov Report

Patch coverage: 61.52% and project coverage change: -25.27 ⚠️

Comparison is base (6e2c2ac) 84.01% compared to head (0ee4ff1) 58.74%.

❗ Current head 0ee4ff1 differs from pull request most recent head 33531ba. Consider uploading reports for the commit 33531ba to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff             @@
##               v2     #541       +/-   ##
===========================================
- Coverage   84.01%   58.74%   -25.27%     
===========================================
  Files          30       31        +1     
  Lines         932     1583      +651     
===========================================
+ Hits          783      930      +147     
- Misses        110      590      +480     
- Partials       39       63       +24     
Impacted Files Coverage Δ
chain.go 0.00% <ø> (-90.91%) ⬇️
interceptors/auth/auth.go 100.00% <ø> (ø)
interceptors/ratelimit/ratelimit.go 60.00% <0.00%> (-40.00%) ⬇️
interceptors/recovery/options.go 78.57% <ø> (ø)
metadata/single_key.go 60.00% <ø> (ø)
testing/testpb/interceptor_suite.go 0.00% <0.00%> (ø)
testing/testpb/test.manual_validator.pb.go 0.00% <0.00%> (ø)
testing/testpb/test.pb.go 35.17% <ø> (ø)
testing/testpb/test_grpc.pb.go 52.21% <ø> (ø)
util/backoffutils/backoff.go 60.00% <ø> (ø)
... and 21 more

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@johanbrandhorst johanbrandhorst merged commit eefb481 into grpc-ecosystem:v2 Mar 10, 2023
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

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.

None yet

3 participants