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

fix(otelgrpc): correctly assign grpc status code #4481

Merged
merged 7 commits into from Oct 27, 2023

Conversation

liufuyang
Copy link
Contributor

closes #4480

I tried to write a test as well to capture the issue above, also made a fix in interceptor.go

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 25, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #4481 (d6c8a0d) into main (3232d7e) will not change coverage.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #4481   +/-   ##
=====================================
  Coverage   80.8%   80.8%           
=====================================
  Files        150     150           
  Lines      10245   10245           
=====================================
  Hits        8284    8284           
  Misses      1823    1823           
  Partials     138     138           
Files Coverage Δ
...ion/google.golang.org/grpc/otelgrpc/interceptor.go 86.2% <100.0%> (ø)

@liufuyang liufuyang force-pushed the fix-otelgrpc-interceptor branch 2 times, most recently from 7660db2 to 4184777 Compare October 25, 2023 19:54
@liufuyang
Copy link
Contributor Author

@dashpole Thanks for the fast review. I pushed in again to fix some formatting issues.

@liufuyang
Copy link
Contributor Author

I think the failed test is related to #4416?

@pellared
Copy link
Member

I think the failed test is related to #4416?

90% that you are correct.

I just left 2 minor comments that I would love to have addressed and one bigger for which I created a separate issue.

Thanks for your contribution 👍

@liufuyang
Copy link
Contributor Author

Thanks, I fixed up the 2 minor comments as you suggested :)

@MrAlias MrAlias merged commit d86d6c8 into open-telemetry:main Oct 27, 2023
22 checks passed
@liufuyang liufuyang deleted the fix-otelgrpc-interceptor branch October 27, 2023 07:45
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.

otelgrpc's UnaryServerInterceptor cannot send metrics with correct grpc status code
4 participants