Support upload trace in Zipkin Lens UI #2424

tacigar · 2019-03-04T06:53:22Z

tacigar · 2019-03-04T06:54:38Z

zipkin-lens/scss/components/_mini-timeline.scss

  color: $dark-2;
  font-size: $font-size-xs;
  position: absolute;
  left: -20px;

-  &.first {
+  &--first {


Use BEM notation

tacigar · 2019-03-04T06:54:59Z

zipkin-lens/scss/components/_mini-timeline.scss

@@ -1,37 +1,37 @@
-.mini-trace-viewer {
+.mini-timeline {


Rename trace-viewer -> timeline

tacigar · 2019-03-04T06:55:46Z

zipkin-lens/scss/components/_trace-page.scss

@@ -0,0 +1,3 @@
+.trace-page {


Sine there is a possibility of writing later, I made a file

tacigar · 2019-03-04T06:58:35Z

zipkin-lens/src/components/App/index.js

@@ -4,8 +4,9 @@ import { BrowserRouter, Route } from 'react-router-dom';

 import Layout from './Layout';
 import BrowserContainer from '../../containers/Browser/BrowserContainer';
-import DetailedTraceSummaryContainer from '../../containers/DetailedTraceSummary/DetailedTraceSummaryContainer';
+import TracePageContainer from '../../containers/TracePage/TracePageContainer';


I refactored to reuse DetailedTraceSummary.

Remove a process of fetching trace data from DetailedTraceSummary component.

Make a new component TracePage, and add that process and DetailedTraceSummary component to the new component.

tacigar · 2019-03-04T07:00:38Z

zipkin-lens/src/components/MiniTimeline/index.js


-const renderTickLines = () => {
+const renderTimeMarkers = () => {


Rename ticks → timeMarker

tacigar · 2019-03-04T07:01:52Z

zipkin-lens/src/components/Timeline/index.js

@@ -98,8 +98,8 @@ class Timeline extends React.Component {
          serviceNameColumnWidth={serviceNameColumnWidth}
          spanNameColumnWidth={spanNameColumnWidth}
          numTimeMarkers={defaultNumTimeMarkers}
-          onServiceNameColumnWidthChange={this.handleServiceNameColumnChange}
-          onSpanNameColumnWidthChange={this.handleSpanNameColumnChange}
+          onServiceNameColumnWidthChange={this.handleServiceNameColumnWidthChange}


I mistook method name...
These are overlooked bugs...
Maybe current lens does not work correctly... 😭

tacigar · 2019-03-04T07:03:49Z

zipkin-lens/src/components/TraceViewer/index.js

+
+      this.setState({
+        startTs: 0,
+        endTs: traceSummary.duration,


startTs and endTs must be set when traceSummary is changed.
If I don't do so, MiniTimeline's range will be displayed as the previous range...

shakuzen · 2019-03-04T11:47:57Z

zipkin-lens/src/components/TraceViewer/index.js

+              style={styles}
+            >
+              <input {...getInputProps()} />
+              <p>Choose file</p>


I'm not sure the right verbosity here, but I think we should guide users more. The classic UI has a "Browse" button at least, which I think gives more an indication that clicking it will allow one to select a file from their computer to upload. An improvement on that might be to reword with something like "Upload a file by drag/drop or click to browse".
As an aside, we haven't gotten any reports of confusion, but maybe even more explanation somehow about what kind of file/contents are expected would be good. Anyways, that could be a separate discussion from this pull request.

shakuzen

I tried it out locally with some of our test data. I left one comment about UI text, but I'm happy to consider it post-merge as well.

jcarres-mdsol · 2019-03-04T18:48:02Z

Does the trace lookup header have any purpose in this screen?

shakuzen · 2019-03-05T02:50:12Z

Does the trace lookup header have any purpose in this screen?

@jcarres-mdsol do you mean the search bar at the top where you can select criteria to search? If so, I agree it doesn't have a use on this page and could be confusing, but I think it is there for consistency as a global search context. I'm not a designer so not sure what's best to do about it (hide it for this view only? show it as disabled in this view?), but thank you for bringing it up. What do others think? Maybe something to spin off into a separate issue so we don't block adding this functionality that helps bring Lens up to feature parity with the classic UI.

jcarres-mdsol · 2019-03-05T17:57:52Z

If it has no use, then should not be there.
Another option would be to, have a "search traceid" component instead, which is also a functionality in the classic UI.

That would make sense, this would be the place where you look at one single trace, either by searching by its traceid in the storage or uploading it from outside storage

tacigar · 2019-03-06T06:36:53Z

@jcarres-mdsol Thank you for your feedback.

Another option would be to, have a "search traceid" component instead, which is also a functionality in the classic UI.

Lens already has that feature.

I tried implementing the feature to upload JSON in the same way as above.
What do you think about this?

cc: @shakuzen

jcarres-mdsol · 2019-03-06T19:31:48Z

It is an intriguing idea, to me it is less discoverable as UI (I did not even see the traceid search option there) but I can see the appeal of a single search bar, something like the browser.

codefromthecrypt · 2019-03-06T19:35:49Z

what I like about this is we can also do a dependency graph of that one trace selected...

…

On Thu, Mar 7, 2019 at 3:31 AM Jordi Polo Carres ***@***.***> wrote: It is an intriguing idea, to me it is less discoverable as UI (I did not even see the traceid search option there) but I can see the appeal of a single search bar, something like the browser. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or mute the thread.

codefromthecrypt · 2019-03-06T19:38:42Z

ohh and the index graph and screen could work equally with multiple files.. ex json + a trace ID, shows in the index screen, and dependency graph

…

On Thu, Mar 7, 2019 at 3:35 AM Adrian Cole ***@***.***> wrote: what I like about this is we can also do a dependency graph of that one trace selected... On Thu, Mar 7, 2019 at 3:31 AM Jordi Polo Carres ***@***.***> wrote: > > It is an intriguing idea, to me it is less discoverable as UI (I did not even see the traceid search option there) but I can see the appeal of a single search bar, something like the browser. > > — > You are receiving this because your review was requested. > Reply to this email directly, view it on GitHub, or mute the thread.

codefromthecrypt · 2019-03-06T20:32:54Z

to be clear.. I like the universal search bar approach with a "traceJSON" button

shakuzen · 2019-03-07T07:46:10Z

Personally, I worry that if we put a json option in the universal search dropdown, we're overloading the meaning of options there too much. Currently you select a property of a span (in some sense). Putting "fromJSON" or something similar there feels a bit confusing. If we want the JSON upload to be part of the universal search instead of a dedicated view/tab, maybe we should make a dedicated upload button in the universal search? Though that might give the impression that the uploaded JSON is saved.

codefromthecrypt · 2019-03-07T13:52:01Z

I guess we need to think on this a bit more?

codefromthecrypt · 2019-03-07T13:56:31Z

grab 15m here? https://cwiki.apache.org/confluence/display/ZIPKIN/2019-03-11+Tapping+into+Zipkin+data

codefromthecrypt · 2019-03-10T16:54:03Z

@shakuzen I agree that there could be some confusion about the query aspect of this. Though it does seem to be in some means related to traceId field... that's how it made sense to me. Also, what we are optimizing for.

For example, by putting the concept of locally sourced (which is indeed a bit icky), into the background.. we benefit by retaining the universal search capability which is what I think we want in the foreground.

Maybe we can down-rank the "from json" criteria to the bottom, or possibly quasi hide it as most users won't be using this? For example, if you "expert mode" or click something, then the from json option appears, but remains in a place where it is universally considered.

wdyt?

codefromthecrypt · 2019-03-10T16:57:16Z

added for 9am tokyo time monday as that's the only time reasonable and in the schedule. We can collect local feedback even if it is too early. https://cwiki.apache.org/confluence/display/ZIPKIN/2019-03-11+Tapping+into+Zipkin+data

tacigar · 2019-03-11T08:07:13Z

Personally I agree with @shakuzen...

Maybe we can down-rank the "from json" criteria to the bottom, or possibly quasi hide it as most users won't be using this? For example, if you "expert mode" or click something, then the from json option appears, but remains in a place where it is universally considered.

umm...
I feel a little complicated.

I think it would be better to add another dropdown menu for traceId and upload JSON like the following.

codefromthecrypt · 2019-03-11T13:17:52Z

sounds good to me. this would move trace id out if the universal search then right? and the action of this form will be like old ui where it takes you to expanded trace screen?

…

On Mon, Mar 11, 2019, 1:07 AM tacigar ***@***.***> wrote: Personally I agree with @shakuzen <https://github.com/shakuzen>... I think it would be better to add another dropdown menu for traceId and upload JSON like the following. [image: dropdown] <https://user-images.githubusercontent.com/19551419/54108718-1268a180-4420-11e9-961e-c6bff23bf6b6.png> — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#2424 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD61yB9xx-pEBKBMbIirnQi24XAjgeXks5vVg6ygaJpZM4bbrHk> .

tacigar · 2019-03-14T09:01:55Z

Yes

tacigar · 2019-03-14T09:02:27Z

I'll close this pr
and make a new pr

tacigar added 4 commits March 4, 2019 15:02

Add react-dropzone

d0d3c66

Add TraceViewer page

663b874

Refactor MiniTimeline

4f36b3b

Fix MiniTimeline's bug

5921924

tacigar requested review from codefromthecrypt, shakuzen and zeagord March 4, 2019 06:53

tacigar commented Mar 4, 2019

View reviewed changes

tacigar added enhancement ui Zipkin UI labels Mar 4, 2019

shakuzen reviewed Mar 4, 2019

View reviewed changes

shakuzen approved these changes Mar 4, 2019

View reviewed changes

tacigar closed this Mar 14, 2019

tacigar mentioned this pull request Mar 18, 2019

Support upload trace in Zipkin Lens UI #2447

Merged

tacigar deleted the upload-json branch August 3, 2019 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support upload trace in Zipkin Lens UI #2424

Support upload trace in Zipkin Lens UI #2424

tacigar commented Mar 4, 2019

tacigar Mar 4, 2019

tacigar Mar 4, 2019

tacigar Mar 4, 2019

tacigar Mar 4, 2019 •

edited

Loading

tacigar Mar 4, 2019 •

edited

Loading

tacigar Mar 4, 2019 •

edited

Loading

tacigar Mar 4, 2019 •

edited

Loading

shakuzen Mar 4, 2019

shakuzen left a comment

jcarres-mdsol commented Mar 4, 2019

shakuzen commented Mar 5, 2019

jcarres-mdsol commented Mar 5, 2019 •

edited

Loading

tacigar commented Mar 6, 2019 •

edited

Loading

jcarres-mdsol commented Mar 6, 2019

codefromthecrypt commented Mar 6, 2019 via email

codefromthecrypt commented Mar 6, 2019 via email

codefromthecrypt commented Mar 6, 2019

shakuzen commented Mar 7, 2019

codefromthecrypt commented Mar 7, 2019 via email

codefromthecrypt commented Mar 7, 2019

codefromthecrypt commented Mar 10, 2019

codefromthecrypt commented Mar 10, 2019

tacigar commented Mar 11, 2019 •

edited

Loading

codefromthecrypt commented Mar 11, 2019 via email

tacigar commented Mar 14, 2019

tacigar commented Mar 14, 2019


		const renderTickLines = () => {
		const renderTimeMarkers = () => {

		@@ -1,37 +1,37 @@
		.mini-trace-viewer {
		.mini-timeline {

Support upload trace in Zipkin Lens UI #2424

Support upload trace in Zipkin Lens UI #2424

Conversation

tacigar commented Mar 4, 2019

tacigar Mar 4, 2019

Choose a reason for hiding this comment

tacigar Mar 4, 2019

Choose a reason for hiding this comment

tacigar Mar 4, 2019

Choose a reason for hiding this comment

tacigar Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

tacigar Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

tacigar Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

tacigar Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

shakuzen Mar 4, 2019

Choose a reason for hiding this comment

shakuzen left a comment

Choose a reason for hiding this comment

jcarres-mdsol commented Mar 4, 2019

shakuzen commented Mar 5, 2019

jcarres-mdsol commented Mar 5, 2019 • edited Loading

tacigar commented Mar 6, 2019 • edited Loading

jcarres-mdsol commented Mar 6, 2019

codefromthecrypt commented Mar 6, 2019 via email

codefromthecrypt commented Mar 6, 2019 via email

codefromthecrypt commented Mar 6, 2019

shakuzen commented Mar 7, 2019

codefromthecrypt commented Mar 7, 2019 via email

codefromthecrypt commented Mar 7, 2019

codefromthecrypt commented Mar 10, 2019

codefromthecrypt commented Mar 10, 2019

tacigar commented Mar 11, 2019 • edited Loading

codefromthecrypt commented Mar 11, 2019 via email

tacigar commented Mar 14, 2019

tacigar commented Mar 14, 2019

tacigar Mar 4, 2019 •

edited

Loading

tacigar Mar 4, 2019 •

edited

Loading

tacigar Mar 4, 2019 •

edited

Loading

tacigar Mar 4, 2019 •

edited

Loading

jcarres-mdsol commented Mar 5, 2019 •

edited

Loading

tacigar commented Mar 6, 2019 •

edited

Loading

tacigar commented Mar 11, 2019 •

edited

Loading