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

Highlight error spans #4

Closed
pavolloffay opened this issue Apr 19, 2017 · 12 comments
Closed

Highlight error spans #4

pavolloffay opened this issue Apr 19, 2017 · 12 comments

Comments

@pavolloffay
Copy link
Member

Hi,

does UI support highlighting error spans?

error-span

@objectiser
Copy link
Contributor

@pavolloffay Yes it does - see the image in our blog post: http://www.hawkular.org/blog/2017/04/19/hawkular-apm-jaeger.html

The selected span at the bottom showing the properties has 'error=true' and the line has a pink/red background.

@pavolloffay
Copy link
Member Author

See attached picture, there is probably something wrong.

Code looks good https://github.com/uber/jaeger-ui/search?utf8=%E2%9C%93&q=isErrorSpan&type=

@saminzadeh
Copy link
Member

I'm wondering if the API is returning the error = "true" value has a string and is missing the equality check.

Can you attach the JSON payload of the trace?

@objectiser
Copy link
Contributor

Is it sensitive to the type of the tag? Is the tag value being stored as a boolean - although possibly it shouldn't matter (just in case).

@pavolloffay
Copy link
Member Author

@objectiser indeed it is type sensitive see attached code above.

JSON from web UI: https://pastebin.com/KfV5R9Y6 so it is string. jaeger client is java 0.18.0

@pavolloffay
Copy link
Member Author

Probably mapping from zipkin thrift to internal model is buggy. I have also noticed that OT component tag is filtered out when using zipkin thrift.

cc @yurishkuro

What do we do with this?

  • keep as it is, current impl respects OT spec
  • try to be more friendly and highlight on error with "true" as a string,
  • or even highlight every span with error tag except error:false.

@yurishkuro
Copy link
Member

we should fix the UI to respect "true" as a string

or even highlight every span with error tag except error:false

mmm, I wouldn't do it just yet.

@yurishkuro
Copy link
Member

or maybe better, we can add an adjuster on the server side, and fix strings to bools on ingestion or on serving to UI

@pavolloffay
Copy link
Member Author

Could we get this fixed by simply testing on a string "true"?

@yurishkuro
Copy link
Member

@pavolloffay yes, but that will only address the UI issue, any future data analysis code would need to keep checking for two values. I think it's best to sanitize the data on the server. Having said that, I am not against the UI fix.

@yurishkuro
Copy link
Member

src/components/TracePage/TraceTimelineViewer/utils.js:107:export const isErrorSpan = span => hasTagKey(span.tags, 'error', true);

easy to fix, but need a unit test

@pavolloffay
Copy link
Member Author

+1 I just wanted to confirm that we agree before opening PR. I will do it tomorrow.

everett980 added a commit that referenced this issue Jul 2, 2019
Add DDG Index with Loading, Error, and temporary header and done views
vvvprabhakar referenced this issue in vvvprabhakar/jaeger-ui Jul 5, 2021
Add DDG Index with Loading, Error, and temporary header and done views
Signed-off-by: vvvprabhakar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants