[KED-2804] Heap event tracking #556

rashidakanchwala · 2021-09-09T13:14:53Z

Description

To track click events on heap

below is the list of events and tracking keywords

Focus mode selected : focusMode.checked.true/false
Layers clicked : visible.layers.true/false
Dark/Light mode : theme.dark/light
Sidebar closed/opened : visible.sidebar.false/true
Tree expanded (+ what level) : clicked.modularPipeline.true/false (requires further work)
number of Tags toggled : (to be done)
Nodes true/false : visible.Nodes.true/false
Datasets true/False : visible.Datasets.true/false
Parameters true/false : visible.Parameters.true/false
Node clicked on graph: clicked.graph.task
Node clicked on sidebar: clicked.sidebar.task
Dataset clicked on graph: clicked.graph.Dataset
Dataset clicked on sidebar: clicked.sidebar.Dataset
Parameter clicked on graph: clicked.graph.Parameter
Parameter clicked on sidebar: clicked.sidebar.Parameter
Show code revealed : visible.code.checked
Run command copied : clicked.run_command

Development notes

QA notes

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added new entries to the RELEASE.md file
Added tests to cover my changes

Legal notice

I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

limdauto

Amazing ⭐ Just a quick question. Otherwise LGTM.

limdauto · 2021-09-13T02:35:34Z

src/components/node-list/node-list-row.js

@@ -163,6 +163,11 @@ const NodeListRow = memo(
          <input
            id={id}
            className="pipeline-nodelist__row__checkbox"
+            data-heap-event={
+              kind === 'element'


QQ: why does kind === 'element' return focusMode event?

To further @limdauto 's comment above, kind === 'element' will not indicate a focusMode event - The best way to return a focusMode event would be via the Visibility Icon, and in particular, the kind prop that dictates the type of visibility icon that gets rendered - this will effectively help us to differentiate between the eye icon ( for tracking if users clicked to see the individual visbility of nodes), and focusMode.

studioswong · 2021-09-13T12:18:10Z

src/components/icon-button/index.js

@@ -23,6 +24,7 @@ const IconButton = ({
  return visible ? (
    <Container>
      <button
+        data-heap-event={dataHeapEvent}


Given that this relies on the prop that is passed down from iconButton, we would need a test to indicate the right type of prop gets passed in from the metadata panel for this heap event.

studioswong · 2021-09-13T12:20:31Z

src/components/node-list/node-list-row.js

+            data-heap-event={
+              kind === 'element'
+                ? `focusMode.checked.${checked}`
+                : `visible.${name}.${checked}`


given that we are expecting different heap events to be sent for different node types ( dataset, Parameters, Nodes), we would need to add in the relevant test to ensure the right heap event gets assigned for each node type.

studioswong · 2021-09-13T12:23:46Z

src/components/flowchart/draw.js

@@ -153,6 +153,7 @@ export const drawNodes = function (changed) {
    enterNodes
      .attr('tabindex', '0')
      .attr('class', 'pipeline-node')
+      .attr('data-heap-event', (node) => `clicked.graph.${node.type}`)


Similar to my comment for the visible heap events, we would need to add tests for each type of node to ensure the right heap event gets sent.

studioswong

Thanks for setting this up! While we need to obtain confirmation from @datajoely on receiving the heap events, we would also need to add tests to ensure the right heap events gets assigned on the FE - this would apply to each heap event listed, in particular for the events to indicate the different types of nodes (visible.dataset.true/false, visible.Datasets.true/false, visible.Parameters.true/false ; clicked.sidebar.task, clicked.sidebar.dataset, clicked.sidebar.Parameter, etc).

rashidakanchwala · 2021-09-13T12:58:50Z

Thanks for setting this up! While we need to obtain confirmation from @datajoely on receiving the heap events, we would also need to add tests to ensure the right heap events gets assigned on the FE - this would apply to each heap event listed, in particular for the events to indicate the different types of nodes (visible.dataset.true/false, visible.Datasets.true/false, visible.Parameters.true/false ; clicked.sidebar.task, clicked.sidebar.dataset, clicked.sidebar.Parameter, etc).

Yup, thanks Susanna! I need Joel to approve the names -- and he will also be checking if this works at his end. Once that'd one, I will work on adding the tests.

datajoely · 2022-03-30T15:24:51Z

any reason why this never went in?

tynandebold · 2022-03-30T15:39:44Z

I need Joel to approve the names -- and he will also be checking if this works at his end. Once that'd one, I will work on adding the tests.

I can't say.

Last thing @rashidakanchwala wrote was:

I need Joel to approve the names -- and he will also be checking if this works at his end. Once that'd one, I will work on adding the tests.

Are the names approved and is it working on your end?

If so, perhaps Rashida needs to add some tests?

rashidakanchwala · 2022-03-31T09:02:15Z

any reason why this never went in?

hi @datajoely , shld i push it in anyway?

I remember last heap call we had, we were able to see data-heap-event in the heap dashboard but i remember you weren't able to filter by that tag in your reports.

I don't know who is going to take over the heap stuff after you. Maybe Nero?

datajoely · 2022-03-31T09:39:27Z

I'm happy to keep leading it - @limdauto do you know where this was left?

NeroOkwa · 2022-03-31T13:23:41Z

any reason why this never went in?

hi @datajoely , shld i push it in anyway?

I remember last heap call we had, we were able to see data-heap-event in the heap dashboard but i remember you weren't able to filter by that tag in your reports.

I don't know who is going to take over the heap stuff after you. Maybe Nero?

@rashidakanchwala @datajoely what is required?

datajoely · 2022-03-31T13:24:31Z

Let's wait for @limdauto to respond as I'm not sure where he got to on this

studioswong · 2022-05-03T11:09:52Z

Hello @datajoely and @NeroOkwa - I will be looking into this in the meantime to help push this across the line.

@datajoely - in following up on what @rashidakanchwala mentioned above, let's get on a call to confirm if the heap events as set up here are tracked on the dashboard? We can ideally use this chance to also add in any further events for our newer features ( experiment-tracking, etc).

limdauto · 2022-05-03T17:52:01Z

Hello, I didn't get too far on this one till now due to limited availability after I left. Let me summarise the chain of events here:

What happened

In September last year @rashidakanchwala and I paired on this one and we followed Heap's guideline here for tracking custom web event. That's why we see data-heap-event in a bunch of components.
I verified correctness by looking at Heap Live Event Tracking log. I confirmed that live events with these custom attributes come through. The process to get these events into Snowflake didn't exist back then.
IIRC @datajoely was off for a while so we put this one on hold before we could confirm the naming pattern with him as mentioned in some of the comments above.
When @datajoely is back, he tried to get these events to show in Snowflake but he couldn't find them. But also back then experiment tracking was in full swing so this became a phantom project.
Then I left QB. I promised to take a look before (and after) I left to see if I could fix it quickly but I have never been able to find the time, including and especially now.

So this one is on me. I'm sorry...

Challenges

A major roadblock I faced when trying to fix this problem was to understand how the data flow from heap to snowflake. It has always been quite opague. I still don't know how it works. From my perspective, what Rashida and I did were in accordance with Heap's suggestion, so I believe the problem lies somewhere between Heap and Snowflake.

My plan was to setup a test personal snowflake account and try to get these data across but this is a challenge that requires time which I don't have at the moment. I have also lost the ability to come next to Joel's desk and pair on this.

Suggestion

That said, to debug this end-to-end, I'd need access to the Heap -> Snowflake pipeline somehow OR I'd need to resume trying to setup a snowflake account to test.

studioswong · 2022-05-03T18:18:52Z

@limdauto thank you for your detailed summary of events!

re: events failing to show up on snowflake, I wonder if this is due to the lack of the html script as outlined here?

I will also try to add this script in and do a bit of a dig around to see if the events show up on the dashboard.

limdauto · 2022-05-04T09:39:17Z

@studioswong that script is programmtically injected if and only if user has given consent to telemetry.

Like I said, we saw the events flowing through from Viz to Heap, so this is not the issue. It's the next hop from Heap to Snowflake that's causing problem.

studioswong · 2022-05-04T18:05:27Z

I see, thanks a lot for the clarification @limdauto, totally makes sense!

Just to make sure, Events flow from Kedro App --> Heap (shows up in dashboard) --> snowflake?

In trying to confirm that events are at least flowing to Heap, @rashidakanchwala and I caught up and did some testing to see if the events are tracked on the heap dashboard by running this branch locally, but we are unable to see any events showing up across all 3 Heap environments (dev, prod, qa) for Kedro-App - she mentioned that she recall running this branch locally last time when she was testing with @datajoely when the events showed up on the dashboard.

@limdauto , would love your thoughts on the following few things to help with our testing:

Given that the script is programmatically injected with telemetry consent, does it mean that, in order for the events to be tracked and sent, we need to run Kedro-Viz from the command line in order for the header to be injected ( i.e running the local version of this branch would not inject the script given there is no way for us to give in consent that way?)
If so, I also guess that using a locally ran Kedro-Viz dev server would not work either?
which heap environment ID ( QA, dev, prod) would be injected in the header? This would help in locating which heap project we should look at.

Rashida also mentioned that in the previous testing, Joel had to specify some ID before the events showing up, hence we are also unsure on any further step we need to do on the dashboard side in order to enable those events to show up under "Live" events -would love to get some pointers from @datajoely on that front.

Given the lack of context and documentation on the Heap setup, it does seem counter-productive for us to test this this week until @datajoely is back - this would be a great chance to involve @NeroOkwa for us to get accustomed to the heap dashboard and setup as well.

In the meantime, this reminds us that we definitely need to document our Heap setup - issue filed here.

limdauto · 2022-05-04T19:44:32Z

Kedro App --> Heap (shows up in dashboard)

This has definitely, 100% worked in the past. We checked it multiple times. If it doesn't work now, maybe something else has changed. In fact, I just looked up the PR that implemented this in the first place:

#481 -- you could see a screenshot here with events like Click on svg#pipeline-graph-bla-bla this is definitely a frontend event.

Given that the script is programmatically injected with telemetry consent, does it mean that, in order for the events to be tracked and sent, we need to run Kedro-Viz from the command line in order for the header to be injected ( i.e running the local version of this branch would not inject the script given there is no way for us to give in consent that way?)

If so, I also guess that using a locally ran Kedro-Viz dev server would not work either?

Locally, you can point it to a test project with a .telemetry containing consent: true, or something to this effect. Can't give you the exact details off the top of my head, but this in principle should work. More info on this find can be found in this README.

which heap environment ID ( QA, dev, prod) would be injected in the header? This would help in locating which heap project we should look at.

PROD ID: https://github.com/kedro-org/kedro-plugins/blob/82cf6b7a2fc5471a1e1b8b31a304f3438a97cb29/kedro-telemetry/kedro_telemetry/plugin.py#L24
DEV ID: It's an env var that you need to set called HEAP_APPID_DEV. I forgot where we share the ID now but others in the team can find it for you. @MerelTheisenQB or @AntonyMilneQB

To put it differently, Kedro-Viz uses the same accounts as Kedro framework. That was a hard requirement: we wanted to be able to see in sequence: people run a command, then click on some elements. Refer to my screenshot earlier for an example.

Rashida also mentioned that in the previous testing, Joel had to specify some ID before the events showing up

I have no idea what this means.

studioswong · 2022-05-06T14:55:34Z

awesome, thanks for all the context with the PRs and the setup for the consent @limdauto!

I'm sure the heap setup works as I do see all the events flowing into HEAP - Just wanted to ensure what we had set up in this PR is sending before we merge this in ( sidenote: we will have to add in FE tests for those as well). I'll mock the consent on my local project and try that again, thanks for the pointers 👍

Regardless, we will have to also confirm with product on the naming pattern of the events so best to wait for @datajoely when he's back next week to get this all sorted in one go before we merge this in ( to minimize any redundant efforts on this front.)

Signed-off-by: Tynan DeBold <[email protected]>

…ature/heap-event-tracking#ked2804 Signed-off-by: Tynan DeBold <[email protected]>

tynandebold · 2022-05-18T08:40:57Z

An update: @limdauto and I paired a bit on this one yesterday. As he wrote on Discord: "We found out that most events on standard HTML elements work correctly except SVGs (not sure if it's just the SVG on the flowchart or all SVGs), so that means it's still worth getting the PR in as-is and do another iteration on the elements where data-heap-event doesn't pass thru."

Here are the events coming through in Heap, and they're all there except for the couple that are placed on an SVG element:

So let's get this in, start capturing data, and then iterate on the SVG issue.

It's from last year.

yetudada · 2022-05-18T09:37:31Z

An update: @limdauto and I paired a bit on this one yesterday. As he wrote on Discord: "We found out that most events on standard HTML elements work correctly except SVGs (not sure if it's just the SVG on the flowchart or all SVGs), so that means it's still worth getting the PR in as-is and do another iteration on the elements where data-heap-event doesn't pass thru."

Here are the events coming through in Heap, and they're all there except for the couple that are placed on an SVG element:

So let's get this in, start capturing data, and then iterate on the SVG issue.

You have no idea how happy this makes me 🥹 No idea. I'm so happy to see this. Which elements are affected by this SVG thing? I'm still happy to get this merged. Please do loop in @studioswong on this too because I know she was working on it.

limdauto · 2022-05-18T12:29:20Z

@yetudada @tynandebold I did some more experiment. Data attributes for any SVG element such as g, rect, etc. which basically means everything on the flowchart won't be captured by Heap. I have a feeling that this is a heap bug. We can open a support case with them and try to reproduce with a minimum example, e.g. a blank HTML page with just heap sdk and an svg element.

This should be fixed in the heap sdk.

tynandebold · 2022-05-18T14:22:36Z

You have no idea how happy this makes me 🥹 No idea. I'm so happy to see this. Which elements are affected by this SVG thing? I'm still happy to get this merged. Please do loop in @studioswong on this too because I know she was working on it.

@yetudada it's just the clicked.graph.${node.type} event that doesn't register right now.

rashidakanchwala added 2 commits September 8, 2021 14:56

testing for theme click

bde56b6

Added data-heap-events

50e1e82

rashidakanchwala requested a review from datajoely September 9, 2021 13:14

rashidakanchwala changed the title ~~Feature/heap event tracking#ked2804~~ [KED-2804] Heap event tracking Sep 9, 2021

rashidakanchwala requested a review from limdauto September 9, 2021 13:19

limdauto approved these changes Sep 13, 2021

View reviewed changes

studioswong reviewed Sep 13, 2021

View reviewed changes

studioswong previously requested changes Sep 13, 2021

View reviewed changes

snyk-bot mentioned this pull request Dec 16, 2021

[Snyk] Upgrade reselect from 4.1.4 to 4.1.5 #673

Merged

tynandebold marked this pull request as ready for review May 9, 2022 12:54

tynandebold added 3 commits May 9, 2022 15:39

Merge branch 'main' into feature/heap-event-tracking#ked2804

035ff90

Signed-off-by: Tynan DeBold <[email protected]>

add back events lost from merge

0e17fb7

Signed-off-by: Tynan DeBold <[email protected]>

Merge branch 'main' of https://github.com/kedro-org/kedro-viz into fe…

e926e3a

…ature/heap-event-tracking#ked2804 Signed-off-by: Tynan DeBold <[email protected]>

tynandebold self-assigned this May 17, 2022

tynandebold requested review from studioswong and tynandebold May 18, 2022 08:41

tynandebold approved these changes May 18, 2022

View reviewed changes

tynandebold merged commit 2ab2366 into main May 18, 2022

tynandebold deleted the feature/heap-event-tracking#ked2804 branch May 18, 2022 15:15

tynandebold mentioned this pull request May 20, 2022

Release v4.6.0 #856

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KED-2804] Heap event tracking #556

[KED-2804] Heap event tracking #556

rashidakanchwala commented Sep 9, 2021 •

edited by gitpod-io bot

Loading

limdauto left a comment

limdauto Sep 13, 2021

studioswong Sep 13, 2021

studioswong Sep 13, 2021

studioswong Sep 13, 2021

studioswong Sep 13, 2021

studioswong left a comment

rashidakanchwala commented Sep 13, 2021

datajoely commented Mar 30, 2022

tynandebold commented Mar 30, 2022

rashidakanchwala commented Mar 31, 2022 •

edited

Loading

datajoely commented Mar 31, 2022

NeroOkwa commented Mar 31, 2022

datajoely commented Mar 31, 2022

studioswong commented May 3, 2022

limdauto commented May 3, 2022

studioswong commented May 3, 2022 •

edited

Loading

limdauto commented May 4, 2022

studioswong commented May 4, 2022

limdauto commented May 4, 2022 •

edited

Loading

studioswong commented May 6, 2022

tynandebold commented May 18, 2022

yetudada commented May 18, 2022

limdauto commented May 18, 2022

tynandebold commented May 18, 2022

[KED-2804] Heap event tracking #556

[KED-2804] Heap event tracking #556

Conversation

rashidakanchwala commented Sep 9, 2021 • edited by gitpod-io bot Loading

Description

Development notes

QA notes

Checklist

Legal notice

limdauto left a comment

Choose a reason for hiding this comment

limdauto Sep 13, 2021

Choose a reason for hiding this comment

studioswong Sep 13, 2021

Choose a reason for hiding this comment

studioswong Sep 13, 2021

Choose a reason for hiding this comment

studioswong Sep 13, 2021

Choose a reason for hiding this comment

studioswong Sep 13, 2021

Choose a reason for hiding this comment

studioswong left a comment

Choose a reason for hiding this comment

rashidakanchwala commented Sep 13, 2021

datajoely commented Mar 30, 2022

tynandebold commented Mar 30, 2022

rashidakanchwala commented Mar 31, 2022 • edited Loading

datajoely commented Mar 31, 2022

NeroOkwa commented Mar 31, 2022

datajoely commented Mar 31, 2022

studioswong commented May 3, 2022

limdauto commented May 3, 2022

What happened

Challenges

Suggestion

studioswong commented May 3, 2022 • edited Loading

limdauto commented May 4, 2022

studioswong commented May 4, 2022

limdauto commented May 4, 2022 • edited Loading

studioswong commented May 6, 2022

tynandebold commented May 18, 2022

yetudada commented May 18, 2022

limdauto commented May 18, 2022

tynandebold commented May 18, 2022

rashidakanchwala commented Sep 9, 2021 •

edited by gitpod-io bot

Loading

rashidakanchwala commented Mar 31, 2022 •

edited

Loading

studioswong commented May 3, 2022 •

edited

Loading

limdauto commented May 4, 2022 •

edited

Loading