log time to play #1853

2018-08-03T06:23:00+02:00

neb-b commented

2018-08-03 06:23:00 +02:00

(Migrated from github.com)

Notes

This ended up being a little more complex after realizing we need to reset analytics data when users navigate directly to another file (where the File page won't be re-mounted)

There are three scenarios:

User has already downloaded a file: do nothing
User started downloading but navigated away before it started playing: log file_view with no time_to_start
User stayed on the page until the file started playing: log file_view with time_to_start

#### Notes This ended up being a little more complex after realizing we need to reset analytics data when users navigate directly to another file (where the File page won't be re-mounted) There are three scenarios: - User has already downloaded a file: do nothing - User started downloading but navigated away before it started playing: log `file_view` with no `time_to_start` - User stayed on the page until the file started playing: log `file_view` with `time_to_start`

neb-b commented

2018-08-03 07:52:37 +02:00

(Migrated from github.com)

Currently outpoint is still not included in the claim information (https://github.com/lbryio/lbry/issues/1306). Previously, we were calling Lbry.get which returns the outpoint and we used that for this api call.

We can't read it from fileInfo because that will not exist if we try to fire the analytics event in response to a user navigating away before the file is downloaded.

I think the simplest approach would be to wait for https://github.com/lbryio/lbry/issues/1306 and grab outpoint along with the rest of the data from the claim information.

@skhameneh @kauffj Thoughts?

Currently outpoint is still not included in the claim information (https://github.com/lbryio/lbry/issues/1306). Previously, we were calling `Lbry.get` which returns the `outpoint` and we used that for this api call. We can't read it from `fileInfo` because that will not exist if we try to fire the analytics event in response to a user navigating away before the file is downloaded. I think the simplest approach would be to wait for https://github.com/lbryio/lbry/issues/1306 and grab `outpoint` along with the rest of the data from the claim information. @skhameneh @kauffj Thoughts?

kauffj commented

2018-08-03 15:50:01 +02:00

(Migrated from github.com)

Three comments:

I think this overlaps some with @daovist work on media history. Please discuss on stand-up if it makes sense to combine this work or for media history to contain this value.
View tracking may currently supports parameters other than outpoints.
Please don't hesitate to point this issue out to a daemon team member and/or comment on that ticket itself. I suspect this is a fairly small one to fix.

Three comments: 1) I think this overlaps some with @daovist work on media history. Please discuss on stand-up if it makes sense to combine this work or for media history to contain this value. 2) View tracking may currently supports parameters other than outpoints. 3) Please don't hesitate to point this issue out to a daemon team member and/or comment on that ticket itself. I suspect this is a fairly small one to fix.

daovist commented

2018-08-03 16:41:59 +02:00

(Migrated from github.com)

A claim's outpoint is just: ${claim.txid}:${claim.nout}. I don't understand why #1306 is an issue.

A claim's outpoint is just: `${claim.txid}:${claim.nout}`. I don't understand why #1306 is an issue.

neb-b commented

2018-08-03 16:52:20 +02:00

(Migrated from github.com)

@daovist Nice. I knew outpoint was some sort of combination of stuff but didn't realize we had all of it already inside of claim. I think that issue is still valid. For other devs, if they see that code snippet they might not know it is an outpoint.

@daovist Nice. I knew outpoint was some sort of combination of stuff but didn't realize we had all of it already inside of `claim`. I think that issue is still valid. For other devs, if they see that code snippet they might not know it is an outpoint.

neb-b (Migrated from github.com) reviewed 2018-08-03 17:57:34 +02:00

src/renderer/redux/actions/content.js

					
				@ -251,0 +258,4 @@

				    }

				  };

				}

neb-b (Migrated from github.com) commented

2018-08-03 17:57:34 +02:00

Lint was telling me to move this up

skhameneh (Migrated from github.com) reviewed 2018-08-05 05:17:03 +02:00

src/renderer/component/fileViewer/view.jsx

					
				@ -165,3 +238,3 @@

				              <Player

				                fileName={fileInfo.file_name}

				                filename={fileInfo.file_name}

				                poster={poster}

skhameneh (Migrated from github.com) commented

2018-08-05 05:16:52 +02:00

Why would we log the same parameters with every analytics event?
There doesn’t appear to be any event name associated as well, this could make things difficult to differentiate events.

Why would we log the same parameters with every analytics event? There doesn’t appear to be any event name associated as well, this could make things difficult to differentiate events.

neb-b (Migrated from github.com) reviewed 2018-08-06 03:43:30 +02:00

src/renderer/component/fileViewer/view.jsx

					
				@ -165,3 +238,3 @@

				              <Player

				                fileName={fileInfo.file_name}

				                filename={fileInfo.file_name}

				                poster={poster}

neb-b (Migrated from github.com) commented

2018-08-06 03:43:30 +02:00

Not sure what you mean. We log the same parameters, but only once per file. The actual api call is here https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/analytics.js#L47

tzarebczan (Migrated from github.com) reviewed 2018-08-07 00:24:04 +02:00

src/renderer/component/fileViewer/view.jsx

					
				@ -165,3 +238,3 @@

				              <Player

				                fileName={fileInfo.file_name}

				                filename={fileInfo.file_name}

				                poster={poster}

tzarebczan (Migrated from github.com) commented

2018-08-07 00:24:04 +02:00

With this change, we'll be calling the file_view api if a user simply starts a download, but then leaves the page? Am I understanding that correctly? This would not happen before, and I'm not sure we want it to count as an actual file view (for rewards purposes at least). Previously, you could go into a claim, let it start playing, leave and it would only count a file_view if it started downloading (which counted a file_view correctly). Now it seems like we either count all attempts, or we only count those attempts where they stayed on the page and it started streaming.

neb-b (Migrated from github.com) reviewed 2018-08-07 01:53:42 +02:00

src/renderer/component/fileViewer/view.jsx

					
				@ -165,3 +238,3 @@

				              <Player

				                fileName={fileInfo.file_name}

				                filename={fileInfo.file_name}

				                poster={poster}

neb-b (Migrated from github.com) commented

2018-08-07 01:53:42 +02:00

Whoops. You are right @tzarebczan

I need to make sure it doesn't fire if the video hasn't started downloading. Will be easy to add.

Whoops. You are right @tzarebczan I need to make sure it doesn't fire if the video hasn't started downloading. Will be easy to add.

neb-b commented

2018-08-07 05:05:43 +02:00

(Migrated from github.com)

@tzarebczan Just updated this to only fire if a user leaves the page after it has started downloading. Before it was firing if they clicked play and left even if it wasn't downloading.

@tzarebczan Just updated this to only fire if a user leaves the page _after_ it has started downloading. Before it was firing if they clicked play and left even if it wasn't downloading.

neb-b commented

2018-08-07 05:06:04 +02:00

(Migrated from github.com)

@skhameneh Did you have any concerns before merging?

kauffj commented

2018-08-07 17:04:24 +02:00

(Migrated from github.com)

@seanyesmunt

Before it was firing if they clicked play and left even if it wasn't downloading.

My concern is that we want this information!

@seanyesmunt > Before it was firing if they clicked play and left even if it wasn't downloading. My concern is that we want this information!

neb-b commented

2018-08-07 17:06:04 +02:00

(Migrated from github.com)

@kauffj Ok makes sense. I will revert the last commit.

skhameneh (Migrated from github.com) approved these changes 2018-08-07 17:24:12 +02:00

neb-b commented

2018-08-07 17:26:42 +02:00

(Migrated from github.com)

@kauffj Tom had a comment
my only concern is that it's now even easier to get a "view" - the file doesn't even have to start playing. Maybe we need to only take into account files with time to play for rewards purposes?

If we change the behavior by adding a view event, we may need to change rewards logic.

@kauffj Tom had a comment ` my only concern is that it's now even easier to get a "view" - the file doesn't even have to start playing. Maybe we need to only take into account files with time to play for rewards purposes? ` If we change the behavior by adding a view event, we may need to change rewards logic.

skhameneh commented

2018-08-07 23:06:21 +02:00

(Migrated from github.com)

@seanyesmunt this is why I had mentioned tagging different events explicitly, isn’t the immediate approach to move forward and change how we track analytics to be more explicit?
We need an approach where we capture a lot of raw data and do transforms to SQL for more specific tables that correlate to data captured.

@seanyesmunt this is why I had mentioned tagging different events explicitly, isn’t the immediate approach to move forward and change how we track analytics to be more explicit? We need an approach where we capture a lot of raw data and do transforms to SQL for more specific tables that correlate to data captured.

skhameneh commented

2018-08-07 23:08:02 +02:00

(Migrated from github.com)

@seanyesmunt the rewards logic would ideally be a separate issue

tzarebczan commented

2018-08-13 15:56:25 +02:00

(Migrated from github.com)

We can worry about the rewards logic on the internal-api side. I think this is ready to be merged in right?

Sign in to join this conversation.

No reviewers

skhameneh