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

Handling of expected error codes coming from grpc storage plugins #1741

Closed
sergiimk opened this issue Aug 19, 2019 · 9 comments
Closed

Handling of expected error codes coming from grpc storage plugins #1741

sergiimk opened this issue Aug 19, 2019 · 9 comments
Labels
bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@sergiimk
Copy link

Requirement - what kind of business use case are you trying to solve?

We are implementing a custom gRPC-based storage plugin as per this doc.

Problem - what in Jaeger blocks you from solving the requirement?

When searching in jaeger UI for a non-existing trace ID the GetTrace function in the storage plugin is expected to return spanstore.ErrTraceNotFound. However the error handling on the Jaeger side doesn't seem to handle this error correctly and treats it as a generic internal error:

{"level":"error","ts":1566246187.758402,"caller":"app/http_handler.go:409","msg":"HTTP handler, Internal Server Error","error":"grpc stream error: rpc error: code = Unknown desc = trace was not found", ...

UI displays:

HTTP Error: grpc stream error: rpc error: code = Unknown desc = trace was not found

You can easily reproduce this issue by running Jaeger with the example in-memory storage gRPC plugin and searching for a trace.

Impact: cosmetic in this case, but other expected error types returned by gRPC plugin may be affected as well.

Proposal - what do you suggest to solve the problem or improve the existing situation?

Any open questions to address

@yurishkuro yurishkuro added bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement labels Aug 20, 2019
@Abhi-khandelwal
Copy link

Hi @yurishkuro , Can I work on this issue ?

@yurishkuro
Copy link
Member

@Abhi-khandelwal by all means

@chandresh-pancholi
Copy link
Contributor

@sergiimk I will be picking this up as there is no activity on this thread.

@chandresh-pancholi
Copy link
Contributor

@sergiimk Could you please help me out here with the reproduction of this issue?

@sergiimk
Copy link
Author

@chandresh-pancholi you can reproduce this by compiling the memstore-plugin and running Jaeger with it as described here. Searching for a random traceID in the UI will then return an ugly-looking HTTP error.

The sequence of commands should look roughly like this:

$ go build -o memstore-plugin examples/memstore-plugin/main.go
$ SPAN_STORAGE_TYPE="grpc-plugin" \
	go run -tags ui ./cmd/all-in-one/main.go \
	--grpc-storage-plugin.binary=`pwd`/memstore-plugin

@chandresh-pancholi
Copy link
Contributor

chandresh-pancholi commented Sep 18, 2019

@sergiimk Thanks, I am able to reproduce it.
Please correct me if I am wrong.
Currently getting:

{
  "data": null,
  "total": 0,
  "limit": 0,
  "offset": 0,
  "errors": [
    {
      "code": 500,
      "msg": "grpc stream error: rpc error: code = Unknown desc = trace was not found"
    }
  ]
}

Expecting:

{
  "data": null,
  "total": 0,
  "limit": 0,
  "offset": 0,
  "errors": [
    {
      "code": 500,
      "msg": "trace was not found"
    }
  ]
}

@chandresh-pancholi
Copy link
Contributor

PR Raised. Please review.

chandresh-pancholi added a commit to chandresh-pancholi/jaeger that referenced this issue Oct 9, 2019
chandresh-pancholi added a commit to chandresh-pancholi/jaeger that referenced this issue Oct 9, 2019
yurishkuro pushed a commit that referenced this issue Oct 9, 2019
* Handling of expected error codes coming from grpc storage plugins #1741

Signed-off-by: chandresh-pancholi <[email protected]>

* Handling of expected error codes coming from grpc storage plugins #1741

Signed-off-by: chandresh-pancholi <[email protected]>
@chandresh-pancholi
Copy link
Contributor

PR #1814 merged. please close this issue.

rubenvp8510 pushed a commit to rubenvp8510/jaeger that referenced this issue Oct 10, 2019
…#1741 (jaegertracing#1814)

* Handling of expected error codes coming from grpc storage plugins jaegertracing#1741

Signed-off-by: chandresh-pancholi <[email protected]>

* Handling of expected error codes coming from grpc storage plugins jaegertracing#1741

Signed-off-by: chandresh-pancholi <[email protected]>
Signed-off-by: Ruben Vargas <[email protected]>
@yurishkuro
Copy link
Member

Thanks for the ping. Closing.

In the future, put the word "Resolves" before the issue number in the PR description, so that GitHub can auto-close the ticket.

radekg pushed a commit to Klarrio/jaeger that referenced this issue Oct 28, 2019
…#1741 (jaegertracing#1814)

* Handling of expected error codes coming from grpc storage plugins jaegertracing#1741

Signed-off-by: chandresh-pancholi <[email protected]>

* Handling of expected error codes coming from grpc storage plugins jaegertracing#1741

Signed-off-by: chandresh-pancholi <[email protected]>
Signed-off-by: radekg <[email protected]>
backjo pushed a commit to backjo/jaeger that referenced this issue Dec 19, 2019
…#1741 (jaegertracing#1814)

* Handling of expected error codes coming from grpc storage plugins jaegertracing#1741

Signed-off-by: chandresh-pancholi <[email protected]>

* Handling of expected error codes coming from grpc storage plugins jaegertracing#1741

Signed-off-by: chandresh-pancholi <[email protected]>
Signed-off-by: Jonah Back <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Development

No branches or pull requests

5 participants