[receiver/otlp] Return proper http response code based on retryable errors

[TylerHelmuth]

Description:
Updates the receiver's http response to return a proper http status based on whether or not the pipeline returned a retryable error. Builds upon the work done in #8080 and #9307

Link to tracking Issue:

Closes #9337
Closes #8132
Closes #9636
Closes #6725

Testing:

Updated lots of unit tests

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.13%. Comparing base (3da7e16) to head (d4d96ad).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9357   +/-   ##
=======================================
  Coverage   91.13%   91.13%           
=======================================
  Files         353      353           
  Lines       18728    18740   +12     
=======================================
+ Hits        17067    17079   +12     
  Misses       1333     1333           
  Partials      328      328           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bogdandrutu]

I don't like that now we also have a similar but different logic in errorMsgToStatus. Can we consolidate?

[TylerHelmuth]

Agreed. I took a look at consolidating before opening the PR. I believe it can be refactored, but it caused a lot of changes unrelated to the issue I was trying to solve. To keep the PRs smaller and targeted I'd like to use this PR as a solution to the issue and then a future PR that is only a refactor. I can open an issue to track that refactor if you'd like.

[TylerHelmuth]

@bogdandrutu are you ok with that approach?

[TylerHelmuth]

Created #9864

[TylerHelmuth]

@open-telemetry/collector-maintainers please review

[TylerHelmuth]

@open-telemetry/collector-maintainers please review

[mx-psi]

@bogdandrutu PTAL

[0x006EA1E5]

Hi, I was just looking at this issue myself locally, and have found this open issue...

I notice this, #9307 and #8080 are somewhat coupled to GRCP status codes.

I also notice we have consumer/consumererror, which seems to address somewhat similar concerns, i.e. retryable and permanent errors.

Can you clarify if consumer/consumererror would be appropriate here, or is it intended for something else? I see consumererror.New is found in a number of places in the contrib codebase.

Have you considered using consumererror.IsPermanent(err) in the receiver to determine the response status code?

Using consumererror would seem to make it a bit more user friendly to actually create the errors in the downstream processor, e.g., something like return td, consumererror.NewTraces(errDataRefused, td) in memorylimiter.

[TylerHelmuth]

@0x006EA1E5 as you've deduced, consumer/consumererror is normally used downstream from receivers to tell receivers what kind of errors they got from r.nextConsumer.Consume*. In this instance we're in a receiver and the error we're propagating back to the caller is subject to the OTLP specification (which is closely tied to grpc objects even for http) so returning consumererror would not be appropriate. The receiver does use consumererror when initially handling the error returned from downstream components.

[TylerHelmuth]

@bogdandrutu @open-telemetry/collector-approvers please take a look. errors.GetHTTPStatusCodeFromStatus(err) does resemble some other internal functions for handling errors in other code paths, but I think a separate refactor after this is added will be best to keep this PR small and targeted.

[jesusvazquez]

LGTM I've personally tested this change on tempo and has worked as expected, we get proper mappings now. Tempo uses the receiver code for both handling and being up to date with the latest otel changes.

[songy23]

LGTM