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

Show marker across all 3 metric visualisation on dashboards #1240

Open
jyothishjose6190 opened this issue Nov 8, 2021 · 9 comments
Open
Assignees

Comments

@jyothishjose6190
Copy link
Contributor

jyothishjose6190 commented Nov 8, 2021

Show marker across all 3 metric visualizations on dashboards when we are going through any of the metric visualizations. This will help us to correlate latency spike easily with the error or call rate spike and vice versa.
Screenshot 2021-11-08 at 9 17 17 AM

@jyothishjose6190
Copy link
Contributor Author

jyothishjose6190 commented Nov 8, 2021

@JBAhire @anandtiwary @aaron-steinfeld Please share your thoughts here? cc: @subintp

@jyothishjose6190
Copy link
Contributor Author

@aaron-steinfeld @anandtiwary @itssharmasandeep Whenever the mouse changes on a cartesian chart, an observable will emit those changes. All the other cartesian charts will be listening to the observable. If any change occurs, a tooltip will be shown on that particular location. This is how I plan to implement it.

@JBAhire
Copy link
Member

JBAhire commented Nov 10, 2021

@anandtiwary , we need to close this as it's been in requirements for quite some time now. Can we please close this asap?

@aaron-steinfeld
Copy link
Contributor

My thoughts - first propose and settle on what the dashboard JSON would look like. That usually looks like a shared parent that handles it, or a shared identifier that they can use to coordinate (e.g. synchronizedTooltipId: 'service-dash') Given that we're trying to correlate things across containers, my suspicion is the latter makes more sense. Also, radar in the above screenshot wouldn't make sense to include, since the synchronization is based on time. Probably only need it for cartesians.

Then, at the component level, we can figure out how to go from there. With the shared identifier, that likely could operate basically like a topic name, and using a shared service to publish and subscribe to tooltip events.

Anyway, will let @anandtiwary and @itssharmasandeep take it from here :)

@anandtiwary
Copy link
Contributor

@JBAhire In the current implementation, we already show data from all three widgets in a single tooltip. Please refer to this screenshot.
Screenshot 2021-11-11 at 11 17 50 PM

What you have in the mock is very time-consuming to implement and honestly we had explored it in the past before finalizing the current solution (as shown in the screenshot above). Like Aaron explained above, this ask would require us to track and show mouse movements on adjacent widgets. We can definitely come up with a solution, but before we go there, please let us know what you think about the current solution as presented in the screenshot.

@JBAhire
Copy link
Member

JBAhire commented Nov 12, 2021

@anandtiwary , yes I am aware of the current implementation. The use-case Razorpay wants to solve here is being able to visualize and correlate deviations across all three key metrics. With current implementation I can see number but I am not sure if this is higher than usual, lower than usual or just normal pattern. Having 3 correlated markers provides us that capability.

@anandtiwary
Copy link
Contributor

Okay, @jyothishjose6190 please start an implementation proposal. Mainly how would you do it, what changes would be required in cartesian chart, widget... would we need a separate widget? And what would be an overall estimate for this change. We can start from there.

@jyothishjose6190
Copy link
Contributor Author

jyothishjose6190 commented Nov 16, 2021

@aaron-steinfeld @anandtiwary @itssharmasandeep I have created a JSON for the new container component named cartesian-container-widget. Cartesian chart listens for all mouse event( mouseenter and mouseleave). Currently we are calling showTooltip method on mouseenter and hiding it on mouseleave. Instead of this, we can emit on mouse event with locationdata using an interaction handler. The interaction handler will update the service observable. All cartesian charts will be listening to this observable. They will show/hide tooltip based on this emitted value.

changes

  1. Cartesian Chart - emit mouse change events, remove tooltip show and hide
  2. Cartesian component - propagate mouse change events up to the widget, show/hide tooltip on input changes
  3. Cartesian widget - propagate mouse change events up to the container, listens for the observable changes

cartesian-container-widget.txt

@anandtiwary
Copy link
Contributor

@jyothishjose6190 i think this is a good proposal and i would just like to make following small modifications.

  1. Since we still have to show tooltip on the primary widget, we can't remove the default show/hide tooltip behavior. But like you said, we can introduce a new mousemove event and emit to the service, and others charts can listen to this mouse location data.
  2. We won't gain much by exposing the mousemove data to an interaction handler model And cartesian widget. The service can just live at cartesian component level only.
  3. Like Aaron suggested, we can introduce a group id, which we can define as a cartesian widget model property and a boolean for enabling mouse sync with other widgets.
  4. Widget will send these properties to chart component.
  5. Chart component will use sync group id to register itself with the service.
  6. If sync flag is enabled, primary chart component will emit mouse move data to the same service.
  7. If sync flag is enabled, chart will read the mouse move data from service (that is, its own sync group).
  8. For non primary charts, only show marker. But for primary widget, show marker + tooltip.
  9. You may have to support marker move in d3 chart in a similar fashion

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