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

Add support to avoid retries for 431 and 414 HTTP error codes in the OTLP exporter #5697

Closed
digikar opened this issue Jul 18, 2022 · 6 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@digikar
Copy link

digikar commented Jul 18, 2022

Is your feature request related to a problem? Please describe.
Collector currently does retries for HTTP error codes 414 and 431. This is not necessary since 431 is the HTTP error code for 'Request Header Fields Too Large' and 414 is the HTTP error code for 'URI Too Long'

These error code should be treated as permanent error in the Collector so that they won't get retried.

Describe the solution you'd like
Currently Collector only treats 400 as permanent error.

if resp.StatusCode == http.StatusBadRequest {
	// Report the failure as permanent if the server thinks the request is malformed.
	return consumererror.NewPermanent(formattedErr)
}

The solution is to add 431 and 414 to the permanent error list.

Describe alternatives you've considered
Currently we have an interceptor to handle this.

@rahuldimri
Copy link
Contributor

Hi, Can I work on this issue ?

@digikar
Copy link
Author

digikar commented Jul 18, 2022

Sure, please fix it. Thanks

@jpkrohling jpkrohling added the good first issue Good for newcomers label Jul 18, 2022
@jpkrohling
Copy link
Member

Go for it, @rahuldimri!

@pavankrish123
Copy link
Contributor

@jpkrohling @rahuldimri please see the PR #5685.
@mcmho can you please see if you can pick up the 431 as well. I think it is better to have a set defined explicitly and loop through the list.

var PermenantErrors = []http.Error{
     http.BadRequest 
     ....
}

@rahuldimri
Copy link
Contributor

@pavankrish123 Let me have a look !!

@jpkrohling
Copy link
Member

Sorry for missing that this was a duplicate! I'll close this in favor of the older issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants