[chore]: Add a helper function to send error responses for network receivers.

[VihasMakwana]

This PR adds a helper function designed for network receivers to facilitate proper error handling.

All the receivers should adhere to receiver contract defined

opentelemetry-collector/receiver/doc.go

Lines 23 to 40 in 1c47d89

	// The nextConsumer.Consume*() function may return an error to indicate that the data
	// was not accepted. There are 2 types of possible errors: Permanent and non-Permanent.
	// The receiver must check the type of the error using IsPermanent() helper.
	//
	// If the error is Permanent, then the nextConsumer.Consume*() call should not be
	// retried with the same data. This typically happens when the data cannot be
	// serialized by the exporter that is attached to the pipeline or when the destination
	// refuses the data because it cannot decode it. The receiver must indicate to
	// the source from which it received the data that the received data was bad, if the
	// receiving protocol allows to do that. In case of OTLP/HTTP for example, this means
	// that HTTP 400 response is returned to the sender.
	//
	// If the error is non-Permanent then the nextConsumer.Consume*() call should be retried
	// with the same data. This may be done by the receiver itself, however typically it is
	// done by the original sender, after the receiver returns a response to the sender
	// indicating that the Collector is currently overloaded and the request must be
	// retried. In case of OTLP/HTTP for example, this means that HTTP 429 or 503 response
	// is returned.

Testing

Added

Usage example for a network receiver

err := nextConsumer.ConsumeLogs(...)
if err != nil {
       confighttp.Error(responseWriter, err) // this will update the status code depending upon the error
       // log the error
}

Note:

Currently, many network based receivers don't follow the contract and they return Service Unavailable (without checking for the error type).
Implementing this helper would enhance both the readability and reusability of the code.

[mx-psi]

Thanks for your PR! This feels like it has some overlap with #9041, for which there is active discussion still. I would suggest we wait until that PR gets merged, check if your use case is addressed, and then retake this

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.32%. Comparing base (2166b11) to head (10f3845).
Report is 16 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #11158   +/-   ##
=======================================
  Coverage   92.32%   92.32%           
=======================================
  Files         413      414    +1     
  Lines       19792    19800    +8     
=======================================
+ Hits        18273    18281    +8     
  Misses       1149     1149           
  Partials      370      370           

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

[VihasMakwana]

@mx-psi

Thanks for your PR! This feels like it has some overlap with #9041, for which there is active discussion still. I would suggest we wait until that PR gets merged, check if your use case is addressed, and then retake this

Thank you for sharing the PR link; I wasn’t aware of it before.

I’d like to point out that this PR adds a helper function to allow network receivers to send errors back to the sender more easily. Currently, receivers would need to handle this manually with code like:

if consumererror.IsPermanent(err) {
     response.WriteHeader(http.StatusBadRequests)
} else {
    response.WriteHeader(http.StatusInternalServerError)
}

Implementing this manually in every receiver could limit reusability and make the code less maintainable.

---

The PR you linked is about implementing a new method for consumers to return errors, whereas my PR is specifically designed for use by network receivers. Let me know what do you think?

[mx-psi]

The PR you linked is about implementing a new method for consumers to return errors, whereas my PR is specifically designed for use by network receivers. Let me know what do you think?

I didn't mean to say that your PR was unnecessary if we merge #9041, only that consumers report errors may affect the design of the API on this PR. I may be wrong, do you think these can be dealt with independently?

[VihasMakwana]

@mx-psi thanks for your response. This PR currently uses the existing consumererror API, which differentiates between permanent and non-permanent errors. Referring to the code snippet here, we can easily update this changes of this PR to adopt the new API once it is implemented in the contrib repository. I expect this might take some time.

Therefore, I believe we can handle these PRs independently. After this PR is merged, we can update the receivers to align with the contract.

Let me know your thoughts on this. I appreciate you taking the time to review it! 😄

[VihasMakwana]

@mx-psi To add, I'm working on stabilizing all the network receivers in the contrib repo and ensuring they handle errors properly. That's why I submitted this PR.

[VihasMakwana]

@mx-psi If you have any feedback or if there's anything else you need from me to move forward, please let me know.

[mx-psi]

I won't have time to review this soon, sorry. And if it has any relation to #9041 I would like #9041 to be merged first in any case, it has been blocked for a long time and I wouldn't want to add extra burden

[VihasMakwana]

I won't have time to review this soon, sorry. And if it has any relation to #9041 I would like #9041 to be merged first in any case, it has been blocked for a long time and I wouldn't want to add extra burden

That makes sense. I'll move these changes to contrib repo then and begin working on stabilising the network receivers. Let me know how it sounds.

[mx-psi]

Sounds reasonable!

[VihasMakwana]

Let's revisit this later when #9041 gets merged