Skip to content
This repository has been archived by the owner. It is now read-only.

feat(logging): add avatar.get activity event #147

Merged
merged 1 commit into from Sep 23, 2015
Merged

Conversation

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Sep 18, 2015

No description provided.

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 18, 2015

Fixes #146

@vladikoff vladikoff force-pushed the vladikoff:i146 branch from 5b37b5b to 7a45133 Sep 18, 2015
.then(avatarOrEmpty)
.done(function (result) {
var rep = reply(result);
if (result.id) {
rep = rep.etag(result.id);
}
var info = {
event: 'avatar.get',
uid: req.uid

This comment has been minimized.

@philbooth

philbooth Sep 21, 2015
Contributor

Should this just be uid?

This comment has been minimized.

@vladikoff

vladikoff Sep 21, 2015
Author Contributor

👍

@vladikoff vladikoff force-pushed the vladikoff:i146 branch from 7a45133 to deea2e3 Sep 21, 2015
event: 'avatar.get',
uid: uid
};
logger.info('activityEvent', info);

This comment has been minimized.

@rfk

rfk Sep 21, 2015
Member

IIUC this will log an event whenever something attempts to read the user's avatar, even if they don't actually have an avatar set (hence the use of avatarOrEmpty above). We could move it into the if (result.id) block to ensure it only emits if there's actually an avatar to read.

@vladikoff is it better to measure avatar-reads or avatar-writes for our metrics purposes?

This comment has been minimized.

@vladikoff

vladikoff Sep 21, 2015
Author Contributor

@vladikoff is it better to measure avatar-reads or avatar-writes for our metrics purposes?

I think avatar-write should be more useful, but avatar-read also has a use case:
with
avatar-write:
"Users who uploaded an avatar and still using" vs "Users who did not and still using"

avatar-get:
"User fetching avatar after X weeks"

@vladikoff vladikoff self-assigned this Sep 21, 2015
@rfk rfk added this to the FxA-24: retention metrics milestone Sep 21, 2015
@vladikoff vladikoff force-pushed the vladikoff:i146 branch 2 times, most recently from 0351bad to 4b174dc Sep 21, 2015
@vladikoff vladikoff force-pushed the vladikoff:i146 branch from 4b174dc to 18cc9b9 Sep 22, 2015
@vladikoff vladikoff removed the WIP label Sep 22, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 22, 2015

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 22, 2015

We need this one before we cut v0.46.0

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 23, 2015

seanmonstar added a commit that referenced this pull request Sep 23, 2015
feat(logging): add avatar.get activity event
@seanmonstar seanmonstar merged commit 66ebb5e into mozilla:master Sep 23, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants