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

Fix distributor http status codes #1958

Closed
joe-elliott opened this issue Dec 15, 2022 · 4 comments
Closed

Fix distributor http status codes #1958

joe-elliott opened this issue Dec 15, 2022 · 4 comments
Labels
keepalive Label to exempt Issues / PRs from stale workflow operations type/bug Something isn't working

Comments

@joe-elliott
Copy link
Member

Describe the bug
As reported by the community if the ingester returns trace too large then the returned http error is 500. This error suggests to the otel collector to retry however it should return 400 which suggests to not retry. Currently the GRPC status codes correctly suggest retrying or not.

Review the status codes for various errors and return 400s where appropriate.

@joe-elliott joe-elliott added the type/bug Something isn't working label Dec 15, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had any activity in the past 60 days.
The next time this stale check runs, the stale label will be removed if there is new activity. The issue will be closed after 15 days if there is no new activity.
Please apply keepalive label to exempt this Issue.

@github-actions github-actions bot added the stale Used for stale issues / PRs label Feb 14, 2023
@joe-elliott joe-elliott added keepalive Label to exempt Issues / PRs from stale workflow and removed stale Used for stale issues / PRs labels Feb 14, 2023
@joe-elliott
Copy link
Member Author

@mghildiy is going to take a crack at this.

cc @ie-pham b/c you're also looking at return codes (but for proto/grpc)

@mghildiy
Copy link
Contributor

mghildiy commented Jul 1, 2023

So currently error code returned is 'FailedPrecondition

return status.Errorf(codes.FailedPrecondition, (newTraceTooLargeError(id, i.instanceID, maxBytes, reqSize).Error()))

return status.Errorf(codes.FailedPrecondition, e.Error())

...defined here:

FailedPrecondition Code = 9

A better option can be:

InvalidArgument Code = 3

Surprisingly, grpC codes to http codes mapping says that both map to 400.

@ie-pham
Copy link
Collaborator

ie-pham commented Aug 28, 2023

Hi @mghildiy ,

Thank you so much for your contribution to this issue. Unfortunately, this issue is no longer is no longer relevant due to our shift towards a "partial success" solution, as outlined in #2571. In addition, Otel states that partial successes should return a 200 instead of an error.

Your contribution to this repo is well observed by the team and is highly valued. Don't be deterred to explore other opportunities to contribute!

Closing this issue.

@ie-pham ie-pham closed this as completed Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keepalive Label to exempt Issues / PRs from stale workflow operations type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants