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

Merged RPC spans not correctly shown, multi value tags not shown #55

Closed
pavolloffay opened this issue Aug 15, 2017 · 3 comments · Fixed by #53
Closed

Merged RPC spans not correctly shown, multi value tags not shown #55

pavolloffay opened this issue Aug 15, 2017 · 3 comments · Fixed by #53
Labels

Comments

@pavolloffay
Copy link
Member

pavolloffay commented Aug 15, 2017

Following screenshots shows:

  • merged RPC spans are now shown (symbol ->)
  • multi value tags are not shown

screenshot of jaeger ui 3
screenshot of jaeger ui 1
screenshot of jaeger ui 2

JSON: https://paste.fedoraproject.org/paste/1vNDb8L-OXZvZFeZVaBBUg
Jaeger master/79fba12
Discovered while testing https://github.com/uber/jaeger/pull/334

Reproducer:
https://github.com/pavolloffay/opentracing-java-examples/tree/jaeger-unknown-service-error-and-ui-issues
run spring-boot and curl localhost:8080/chaining

@tiffon
Copy link
Member

tiffon commented Aug 21, 2017

@pavolloffay Thanks for reporting this.

I tested things out with the JSON you linked. I was able to reproduce the behavior.

  • merged RPC spans are now shown (symbol ->)

Not sure this is an error. The parent span does not have the span.kind: client tag, it has (multiple) span.kind: server.

@yurishkuro, can you weigh in?

  • multi value tags are not shown

This is a bug that should be resolved in PR #53.

@tiffon tiffon mentioned this issue Aug 21, 2017
9 tasks
@tiffon tiffon added the bug label Aug 21, 2017
@yurishkuro
Copy link
Member

in fact the very first span has span.kind tags with both server and client values. How did that happen? It looks like an instrumentation issue where the child span was not started for downstream call, so the parent server the span was tagged as a client.

tiffon added a commit that referenced this issue Aug 23, 2017
Top level tasks

- Maintenance of tests
- Cleanup unused vars, imports
- Refine utils
- Many styles will be moved to CSS

SpanGraph

- Fix #49 - Span position in graph doesn't not match its position in the
  detail
- Ticks in span graph made to match trace detail (in number and
  formatting)
- Span graph refactored to trim down files and DOM elements
- Styling adjustments

TracePageHeader

- `trace` prop removed
- Added props for various title values instead of deriving them from
  `trace`

Trace Detail

- Several components split out into separate files
- `transformTrace` to use already created span tree to determine span
  depth
- Fix #59 - "Span Name" to "Service & Operation"

Span Bar / Detail

- Fix uber/jaeger#326: extraneous scrollbars in trace views
- Fix #55: Some tags were not being rendered due to clashing keys
  (observed in a log message)
- Tall content scrolls via entire table instead of single table cell
- Horizontal scrolling for wide content (e.g. long log values) (Fix #58)
- Full width of the header is clickable for tags, process, and logs
  headers (instead of header text, only)
- Service and endpoint are shown on mouseover anywhere in span bar row
- Label on span bars no longer off-screen
- Clip or hide span bars when zoomed in (instead of flush left)
- Add shadow to left / right boundary when span bar view is clipped
- Darkened span name column to differentiate from span bar section
- Span detail left column color coded by service
- Clicking span detail left column collapses detail
- Clicking anywhere left of parent span name toggles children visibility
- Prevent collision of logs in log entries table

SearchTracePage

- Scatterplot dots are sized based on number of spans
- Scatterplot dots mouseover shows trace name

Misc

- Several TraceTimelineViewer / utils removed
- `TreeNode` `.walk()` method can now be used to calculate the depth,
  avoiding use of less efficient `.getPath()`
- Removed several `console.error` warnings caused by React key issues
- `yarn upgrade --latest`
- Removed `react-sticky`
- Fix #42 - Support URL prefix via homepage in package.json


Commits

* Update prettier and wrap at 110 instead of 80

* WIP commit for refactoring trace detail view

* WIP refactor trace detail, split out css, start fixing tests

* WIP refactor trace detail, tests working again

* WIP refactor trace, yarn upgrade

Sub-page scrolling used for trace detail (TODO: revert this). This lays
the ground work for using react-virtualized, but unfortunately has major
perf issues, hence the TODO: revert.

yarn upgrade --latest. Notable changes:

- Removed react-sticky
- react-router v4
- react-vis CSS needed to be included via a sym-link

Misc tweaks:

- Styling adjusted on trace mini-map
- Scatterplot dots are sized based on number of spans
- Scatterplot dots mouseover shows trace name

* WIP refactor trace, test maintenance

* remove unused import

* add license to css files

* Revert sub-page scrolling for trace detail

* Support URL prefix via homepage in package.json

* Prevent collision of logs in log entries table

* Add comments to new utils

* Fix #59 - "Span Name" to "Service & Operation"

* Fix unreleased regression - ellipsis on span name

Add back the styling that adds an ellipsis to truncated span name text.

* Address PR comment on search results scatter plot

https://github.com/uber/jaeger-ui/pull/53#discussion_r134313013

* Misc cleanup from PR comment

https://github.com/uber/jaeger-ui/pull/53#discussion_r134314020

* PR comment - Remove ms and use nano seconds

https://github.com/uber/jaeger-ui/pull/53#discussion_r134316418

* PR comment - Adjust export, relates to recompose

https://github.com/uber/jaeger-ui/pull/53#discussion_r134318188

* PR comment - Comment getTraceSpanIdsAsTree()

https://github.com/uber/jaeger-ui/pull/53#discussion_r134321990
@pavolloffay
Copy link
Member Author

in fact the very first span has span.kind tags with both server and client values. How did that happen? It looks like an instrumentation issue where the child span was not started for downstream call, so the parent server the span was tagged as a client.

It's correct OT instrumentation with brave-ot. Maybe it's some brave magic... I wanted to check this again but the link to JSON is not valid anymore.

vvvprabhakar referenced this issue in vvvprabhakar/jaeger-ui Jul 5, 2021
Top level tasks

- Maintenance of tests
- Cleanup unused vars, imports
- Refine utils
- Many styles will be moved to CSS

SpanGraph

- Fix #49 - Span position in graph doesn't not match its position in the
  detail
- Ticks in span graph made to match trace detail (in number and
  formatting)
- Span graph refactored to trim down files and DOM elements
- Styling adjustments

TracePageHeader

- `trace` prop removed
- Added props for various title values instead of deriving them from
  `trace`

Trace Detail

- Several components split out into separate files
- `transformTrace` to use already created span tree to determine span
  depth
- Fix jaegertracing#59 - "Span Name" to "Service & Operation"

Span Bar / Detail

- Fix uber/jaeger#326: extraneous scrollbars in trace views
- Fix #55: Some tags were not being rendered due to clashing keys
  (observed in a log message)
- Tall content scrolls via entire table instead of single table cell
- Horizontal scrolling for wide content (e.g. long log values) (Fix jaegertracing#58)
- Full width of the header is clickable for tags, process, and logs
  headers (instead of header text, only)
- Service and endpoint are shown on mouseover anywhere in span bar row
- Label on span bars no longer off-screen
- Clip or hide span bars when zoomed in (instead of flush left)
- Add shadow to left / right boundary when span bar view is clipped
- Darkened span name column to differentiate from span bar section
- Span detail left column color coded by service
- Clicking span detail left column collapses detail
- Clicking anywhere left of parent span name toggles children visibility
- Prevent collision of logs in log entries table

SearchTracePage

- Scatterplot dots are sized based on number of spans
- Scatterplot dots mouseover shows trace name

Misc

- Several TraceTimelineViewer / utils removed
- `TreeNode` `.walk()` method can now be used to calculate the depth,
  avoiding use of less efficient `.getPath()`
- Removed several `console.error` warnings caused by React key issues
- `yarn upgrade --latest`
- Removed `react-sticky`
- Fix #42 - Support URL prefix via homepage in package.json


Commits

* Update prettier and wrap at 110 instead of 80

* WIP commit for refactoring trace detail view

* WIP refactor trace detail, split out css, start fixing tests

* WIP refactor trace detail, tests working again

* WIP refactor trace, yarn upgrade

Sub-page scrolling used for trace detail (TODO: revert this). This lays
the ground work for using react-virtualized, but unfortunately has major
perf issues, hence the TODO: revert.

yarn upgrade --latest. Notable changes:

- Removed react-sticky
- react-router v4
- react-vis CSS needed to be included via a sym-link

Misc tweaks:

- Styling adjusted on trace mini-map
- Scatterplot dots are sized based on number of spans
- Scatterplot dots mouseover shows trace name

* WIP refactor trace, test maintenance

* remove unused import

* add license to css files

* Revert sub-page scrolling for trace detail

* Support URL prefix via homepage in package.json

* Prevent collision of logs in log entries table

* Add comments to new utils

* Fix jaegertracing#59 - "Span Name" to "Service & Operation"

* Fix unreleased regression - ellipsis on span name

Add back the styling that adds an ellipsis to truncated span name text.

* Address PR comment on search results scatter plot

https://github.com/uber/jaeger-ui/pull/53#discussion_r134313013

* Misc cleanup from PR comment

https://github.com/uber/jaeger-ui/pull/53#discussion_r134314020

* PR comment - Remove ms and use nano seconds

https://github.com/uber/jaeger-ui/pull/53#discussion_r134316418

* PR comment - Adjust export, relates to recompose

https://github.com/uber/jaeger-ui/pull/53#discussion_r134318188

* PR comment - Comment getTraceSpanIdsAsTree()

https://github.com/uber/jaeger-ui/pull/53#discussion_r134321990

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants