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

when trace id is invalid, jaeger-ui send this request forever #556

Closed
meilihao opened this issue Nov 24, 2017 · 9 comments
Closed

when trace id is invalid, jaeger-ui send this request forever #556

meilihao opened this issue Nov 24, 2017 · 9 comments
Labels

Comments

@meilihao
Copy link

jaeger verison: 3136216
jaeger-ui version : 27c3d58

$ query 2>&1 > query.log --query.static-files=jaeger-ui/build/ --span-storage.type=elasticsearch --es.server-urls="${ESServerUrls}"

_ _20171124184124

@yurishkuro
Copy link
Member

cc @tiffon

@tiffon
Copy link
Member

tiffon commented Nov 26, 2017

That's pretty odd. I would expect an invalid ID to have a 404 response, but these are 200.

Does the response have any data? Is there a trace, and if so what is the traceID?

Thanks for reporting this.

@yurishkuro yurishkuro added the ui label Nov 26, 2017
@yurishkuro
Copy link
Member

yurishkuro commented Nov 26, 2017

When I access URL with invalid trace ID I am getting this

image

This happens with Cassandra and in-memory backend. Maybe the issue is with ES storage not returning NotFound.

The UI display could be worded a bit better, "something that doesn't exist" is actually the trace, and the "bad JSON" error is misleading.

@tiffon
Copy link
Member

tiffon commented Nov 26, 2017

Improving the error messages is something I want to look into. I'll create a ticket in jaeger-ui for it if there isn't one, already.

@tiffon
Copy link
Member

tiffon commented Nov 26, 2017

Will also, of course, address this issue, specifically.

@meilihao
Copy link
Author

meilihao commented Nov 27, 2017

_ _20171127095307

invalid trace's respone:

{"data":[{"traceID":"","spans":[],"processes":{},"warnings":null}],"total":0,"limit":0,"offset":0,"errors":null}

this is ok:
_ _20171127095948

@yurishkuro
Copy link
Member

@black-adder I see this code in ES spanstore

// GetTrace takes a traceID and returns a Trace associated with that traceID
func (s *SpanReader) GetTrace(traceID model.TraceID) (*model.Trace, error) {
	currentTime := time.Now()
	traces, err := s.multiRead([]string{traceID.String()}, currentTime.Add(-s.maxLookback), currentTime)
	if err != nil {
		return nil, err
	}
	if len(traces) == 0 {
		return nil, errNoTraces
	}
	return traces[0], nil
}

There are two problems here. First, when no traces are found the error should be spanstore.ErrTraceNotFound, instead of the private error. But it may not help either because in the multiread function there's this code

	var traces []*model.Trace
	for _, result := range results.Responses {
		if result.Hits == nil {
			return nil, errNilHits
		}

If I read it correctly, the method would return errNilHits, so the len(traces) == 0 logic above would never even be run.

@NeoCN
Copy link
Contributor

NeoCN commented Dec 16, 2017

@yurishkuro the flowing multiread function code snippet

	var traces []*model.Trace
	for _, result := range results.Responses {
		if result.Hits == nil {
			return nil, errNilHits
		}
		spans, err := s.collectSpans(result.Hits.Hits)
		if err != nil {
			return nil, err
		}
		traces = append(traces, &model.Trace{Spans: spans})
	}
	return traces, nil

when the traceId is invalid or not in the es store, got result

[]*model.Trace{
    &model.Trace{
        Spans: {
        },
        Warnings: nil,
    },
}

this results that len(traces) == 0 will not trigger, and then no errNoTraces is returned.

when len(result.Hits.Hits) == 0, collectSpans will return []*model.Span with zero length and no error.

I just make a pull request #597 which check len(result.Hits.Hits), when len(result.Hits.Hits) == 0 skip collectSpans

NeoCN pushed a commit to NeoCN/jaeger that referenced this issue Dec 18, 2017
Signed-off-by: 马庆升 <mqs@meitu.com>
@yurishkuro
Copy link
Member

#601 and the UI fix jaegertracing/jaeger-ui#128 resolve this.

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

No branches or pull requests

4 participants