Properly handle error code translation #10574
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10574 +/- ##
==========================================
- Coverage 92.34% 92.33% -0.01%
==========================================
Files 395 395
Lines 18669 18681 +12
==========================================
+ Hits 17240 17250 +10
- Misses 1069 1070 +1
- Partials 360 361 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Verified with NewStatusFromMsgAndHTTPCode and the two conversions should match now.
| @@ -40,6 +40,18 @@ func GetHTTPStatusCodeFromStatus(s *status.Status) int { | |||
| case codes.ResourceExhausted: | |||
| return http.StatusTooManyRequests | |||
| // Not Retryable | |||
| case codes.InvalidArgument: | |||
There was a problem hiding this comment.
@TylerHelmuth maybe we should this function and its tests to the helper i made last time? https://github.com/open-telemetry/opentelemetry-collector/blob/af33ac5acc08dbeb43426f86b526d2cfd29318ab/internal/httphelper/helper.go#L15C6-L15C33
That way the conversions are in the same place
There was a problem hiding this comment.
@jaronoff97 it is all moving (and properly solved by remembering the actual http status code) via #9041, so we should stick with that PR so the permanent solution to this issue. This PR fixes the bug for now.
There was a problem hiding this comment.
cool! I have no problem with that :D
Description
Fixes a bug where the otlpreceiver was not translating grpc error codes to http status codes the same way that the otlp exporter was translating http status codes to grpc error codes.
Link to tracking issue
Fixes #10538
Testing
Added unit tests