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 handle 402 and 413 http error code in Otlp exporter #5674

Closed
mcmho opened this issue Jul 11, 2022 · 4 comments
Closed

Add support to handle 402 and 413 http error code in Otlp exporter #5674

mcmho opened this issue Jul 11, 2022 · 4 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@mcmho
Copy link
Contributor

mcmho commented Jul 11, 2022

Is your feature request related to a problem? Please describe.
Collector currently does retries 402 and 413 error code. 413 is the error code for PAYLOAD TOO LARGE
and 402 is for PAYMENT REQUIRED which a user case of this would be when license token is exhausted.
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 402 and 413 to the permanent error list.

Describe alternatives you've considered
Tweaking the backend to return a different error code; however, that is a hack. It is not a permanent solution.

Additional context
it does not makes sense to keep retrying, for example 413, since it will just keep failing the second time and so forth.

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

Sounds reasonable to me.

@mcmho
Copy link
Contributor Author

mcmho commented Jul 11, 2022

@jpkrohling , can I work on it / assign to me please?

@jpkrohling
Copy link
Member

It's yours

mcmho pushed a commit to mcmho/opentelemetry-collector that referenced this issue Jul 14, 2022
…en-telemetry#5674

 - added StatusRequestEntityTooLarge (413) and StatusPaymentRequired (402) as failure permanent
mcmho pushed a commit to mcmho/opentelemetry-collector that referenced this issue Jul 14, 2022
…en-telemetry#5674

 - added StatusRequestEntityTooLarge (413) and StatusPaymentRequired (402) as failure permanent
 - fixed email address
mcmho pushed a commit to mcmho/opentelemetry-collector that referenced this issue Jul 14, 2022
…en-telemetry#5674

 - added StatusRequestEntityTooLarge (413) and StatusPaymentRequired (402) as failure permanent
mcmho pushed a commit to mcmho/opentelemetry-collector that referenced this issue Jul 14, 2022
…en-telemetry#5674

 - added StatusRequestEntityTooLarge (413) and StatusPaymentRequired (402) as failure permanent
mcmho pushed a commit to mcmho/opentelemetry-collector that referenced this issue Jul 14, 2022
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 14, 2022
…en-telemetry#5674

 - added StatusRequestEntityTooLarge (413) and StatusPaymentRequired (402) as failure permanent
@mcmho
Copy link
Contributor Author

mcmho commented Jul 14, 2022

Sorry, for couple attempts, I didn't realize I typed my email address wrong. Now it is fixed.

mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 17, 2022
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 19, 2022
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 19, 2022
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 19, 2022
… errors in OTLP exporter open-telemetry#5674

 - added 414 and 431 as permanent errors
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 19, 2022
… errors in OTLP exporter open-telemetry#5674

 - change ifs to a switch statement
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 21, 2022
… errors in OTLP exporter open-telemetry#5674

 - fixed according to code review
mcmho added a commit to mcmho/opentelemetry-collector that referenced this issue Jul 21, 2022
… errors in OTLP exporter open-telemetry#5674

 - fixed build-and-test  golangci-lint run
bogdandrutu pushed a commit that referenced this issue Jul 25, 2022
… errors in OTLP exporter #5674 (#5685)

* Add support to handle 402 and 413 http error code in Otlp exporter #5674
 - added StatusRequestEntityTooLarge (413) and StatusPaymentRequired (402) as failure permanent

* Add support to handle 402 and 413 http error code in Otlp exporter #5674
 - added unit tests

* Add support to handle 402 and 413 http error code in Otlp exporter #5674
 - fixed code review comment

* Add support to handle 402 and 413 http error code in Otlp exporter #5674
 - added an entry in CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Juraci Paixão Kröhling <[email protected]>

* Add support to handle 402, 413, 414, 431 http error code as permanent errors in OTLP exporter #5674
 - added 414 and 431 as permanent errors

* Add support to handle 402, 413, 414, 431 http error code as permanent errors in OTLP exporter #5674
 - change ifs to a switch statement

* Add support to handle 402, 413, 414, 431 http error code as permanent errors in OTLP exporter #5674
 - fixed according to code review

* Add support to handle 402, 413, 414, 431 http error code as permanent errors in OTLP exporter #5674
 - fixed build-and-test  golangci-lint run

* Creates a signed commit

Co-authored-by: Juraci Paixão Kröhling <[email protected]>
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

3 participants