Fix trace name resolution #134

tiffon · 2017-12-14T22:44:30Z

Identify the root span as the span without a parent reference.

Use <cannot-find-trace-name> as a placeholder for when unable to determine the trace name.

Signed-off-by: Joe Farro <joef@uber.com>

codecov · 2017-12-14T22:53:56Z

Codecov Report

Merging #134 into master will increase coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   92.28%   92.39%   +0.11%     
==========================================
  Files          81       81              
  Lines        1789     1789              
  Branches      345      345              
==========================================
+ Hits         1651     1653       +2     
+ Misses        126      124       -2     
  Partials       12       12

Impacted Files	Coverage Δ
src/components/TracePage/index.js	`100% <ø> (ø)`	⬆️
...rc/components/SearchTracePage/TraceSearchResult.js	`100% <ø> (ø)`	⬆️
src/components/TracePage/TracePageHeader.js	`92.85% <ø> (ø)`	⬆️
...ponents/SearchTracePage/TraceResultsScatterPlot.js	`36.36% <ø> (ø)`	⬆️
src/model/trace-viewer.js	`100% <100%> (ø)`	⬆️
src/model/search.js	`100% <100%> (ø)`	⬆️
...neViewer/TimelineHeaderRow/TimelineViewingLayer.js	`90.74% <0%> (+3.7%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffbbcea...c6cc8fe. Read the comment docs.

yurishkuro · 2017-12-14T23:17:28Z

src/constants/fallback-trace-name.js

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+export default '<cannot-find-trace-name>';


will <> show in html?

It shows up verbatim.

yurishkuro

couple comments, otherwise lgtm

yurishkuro · 2017-12-16T04:22:45Z

src/model/trace-viewer.js

-  const span = spans.find(sp => sp.spanID === sp.traceID) || spans[0];
-  return span ? `${processes[span.processID].serviceName}: ${span.operationName}` : '';
+export function getTraceName(spans) {
+  const span = spans.filter(sp => !sp.references || !sp.references.length)[0];


it is possible that the root span is delayed en-route to the storage when the UI tries to show the trace, so [0] might try to dereference an empty array. I'd rather not assume it exists.

Dereference an empty array in JS does not error, it returns undefined; this is a safe operation. The .filter() method will always return an Array.

yurishkuro · 2017-12-16T04:23:15Z

src/model/trace-viewer.js

-  return span ? `${processes[span.processID].serviceName}: ${span.operationName}` : '';
+export function getTraceName(spans) {
+  const span = spans.filter(sp => !sp.references || !sp.references.length)[0];
+  return span ? `${span.process.serviceName}: ${span.operationName}` : '';


should this also fallback to FALLBACK_TRACE_NAME?

It can, but that is deferred, the fallback is used closer to the view — the use of the fallback or not is a presentation decision.

…e-resolution Fix trace name resolution Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>

Fix trace name resolution

c6cc8fe

Signed-off-by: Joe Farro <joef@uber.com>

tiffon requested review from black-adder, vprithvi, yurishkuro and saminzadeh December 14, 2017 22:44

yurishkuro reviewed Dec 14, 2017

View reviewed changes

yurishkuro approved these changes Dec 16, 2017

View reviewed changes

tiffon merged commit cbc3025 into master Dec 16, 2017

tiffon mentioned this pull request Dec 16, 2017

¯\_(ツ)_/¯ is not very clear #129

Closed

tmthrgd mentioned this pull request Oct 7, 2019

Fix trace name resolution for spans with remote parents #461

Closed

yurishkuro deleted the fix-trace-name-resolution branch January 29, 2020 15:11

vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021

Merge pull request jaegertracing#134 from jaegertracing/fix-trace-nam…

36dc263

…e-resolution Fix trace name resolution Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix trace name resolution #134

Fix trace name resolution #134

tiffon commented Dec 14, 2017

codecov bot commented Dec 14, 2017 •

edited

Loading

yurishkuro Dec 14, 2017

tiffon Dec 16, 2017

yurishkuro left a comment

yurishkuro Dec 16, 2017

tiffon Dec 16, 2017

yurishkuro Dec 16, 2017

tiffon Dec 16, 2017

Fix trace name resolution #134

Fix trace name resolution #134

Conversation

tiffon commented Dec 14, 2017

codecov bot commented Dec 14, 2017 • edited Loading

Codecov Report

yurishkuro Dec 14, 2017

Choose a reason for hiding this comment

tiffon Dec 16, 2017

Choose a reason for hiding this comment

yurishkuro left a comment

Choose a reason for hiding this comment

yurishkuro Dec 16, 2017

Choose a reason for hiding this comment

tiffon Dec 16, 2017

Choose a reason for hiding this comment

yurishkuro Dec 16, 2017

Choose a reason for hiding this comment

tiffon Dec 16, 2017

Choose a reason for hiding this comment

codecov bot commented Dec 14, 2017 •

edited

Loading