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

span name selector UI broken #748

Closed
srijs opened this issue Sep 30, 2015 · 9 comments · Fixed by #749
Closed

span name selector UI broken #748

srijs opened this issue Sep 30, 2015 · 9 comments · Fixed by #749
Labels
bug ui Zipkin UI

Comments

@srijs
Copy link
Contributor

srijs commented Sep 30, 2015

With the merge of the json api branch, the selector in zipkin-web is broken, because it loads data from /api/spans?serviceName=xxx (which returns a nested array of spans), and then tries to display it (as if it was a array of strings).

Previously, the data was loaded from a span names endpoint, which used getSpanNames in the span store to retrieve a list of span names. This approach was superior, because it allows the span store to execute an efficient query to just aggregate all span names across the traces (the aggregates could even be stored separately).

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Sep 30, 2015 via email

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Sep 30, 2015 via email

@codefromthecrypt
Copy link
Member

looking now.. should have a fix shortly. thanks for keeping an eye out!

@codefromthecrypt
Copy link
Member

ps wow.. this did happen!

codefromthecrypt pushed a commit that referenced this issue Sep 30, 2015
The span name selector UI was calling getTraces instead of getSpanNames.
This happened during the switch to the json api. This change fixes that
and clarifies the confusing naming that was used in the web handlers.

Fixes #748
@codefromthecrypt
Copy link
Member

ok here's the fix #749

codefromthecrypt pushed a commit that referenced this issue Sep 30, 2015
The span name selector UI was calling getTraces instead of getSpanNames.
This happened during the switch to the json api. This change fixes that
and clarifies the confusing naming that was used in the web handlers.

Fixes #748
@codefromthecrypt
Copy link
Member

Pushing 1.13.3 immediately. Eta 1hr

@srijs
Copy link
Contributor Author

srijs commented Oct 1, 2015

Yeah, so it was both inefficient and broken ;) Anyways, thanks for the quick fix! 👍

@eirslett
Copy link
Contributor

eirslett commented Oct 1, 2015

Raise your hand if you want test coverage on the JavaScript!

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Oct 1, 2015 via email

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

Successfully merging a pull request may close this issue.

3 participants