[MM-37324] Add telemetry #230

larkox · 2021-08-04T13:45:30Z

Summary

Add telemetry to the Apps Framework. Right now only the proxy package has access to the telemetry, but it can be easily added to any other package that needs it. We are tracking:

Every Install, with the AppID and AppType of the App installed
Every Uninstall, also with the AppID and AppType
Every call performed, with the AppID and the location (this will help us to know which bindings are more popular)

Feel free to suggest any other event worth tracking.

Ticket Link

https://mattermost.atlassian.net/browse/MM-37324

aaronrothschild · 2021-08-04T14:16:19Z

Adding @enelson720 so this is on his radar.

@larkox can we log when each user has authenticated with each app? (or some indication of how many different users are using an app on a particular server)

ie: "On this server 12% of users are using App X" and
"Looking across all Apps, on average Y% of users are using apps"
"Looking across all servers, App X,Y,Z are most popular amongst users based on authenticated calls"
Track Scopes used (ie: User, act_as_admin), by Apps

larkox · 2021-08-04T14:21:05Z

@aaronrothschild It is possible to log each time a user finish the OAuth steps for App X if the App X is using the Mattermost OAuth abstraction (mainly useful for lambda).

It is also possible for the calls register the acting user ID, and therefore see how many people per server is using the bindings. Nevertheless, we would have to do some filtering (either on code or interpreting the analytics) to filter out the "get bindings" call, since that one will be done by all users.

aaronrothschild · 2021-08-04T14:27:28Z

@larkox I'd like to track those actions so we can track installation vs authentication vs ongoing usage if possible.

catalintomai · 2021-08-04T15:55:19Z

server/telemetry/telemetry.go

+		return
+	}
+
+	_ = t.tracker.TrackEvent("uninstall", map[string]interface{}{


"uninstall"- > "call' ?

catalintomai · 2021-08-04T15:56:15Z

server/telemetry/telemetry.go

+	})
+}
+
+func (t *Telemetry) TrackCall(appID string, location string) {


could this be too noisy - tracking every call? It can boil down to what are our goals with this change (like what metrics are we targeting, what do we want to understand better, etc.). Also, do we envision (in the future) for users/app developers to pick and choose what telemetry they want to record, specific to their app (like having two types of telemetry metrics - ours and theirs)? cc: @aaronrothschild

larkox · 2021-08-09T13:10:59Z

I have added a new event for OAuth complete, and instead of tracking every single call (including bindings calls) we will just be registering "submit calls".

levb · 2021-08-09T14:41:39Z

What's up with cypress tests?

codecov-commenter · 2021-08-10T13:31:26Z

Codecov Report

Merging #230 (99a4aa5) into master (2d91438) will increase coverage by 0.02%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
+ Coverage   24.77%   24.79%   +0.02%     
==========================================
  Files          52       52              
  Lines        3306     3311       +5     
==========================================
+ Hits          819      821       +2     
- Misses       2407     2410       +3     
  Partials       80       80

Impacted Files	Coverage Δ
server/proxy/install.go	`0.00% <0.00%> (ø)`
server/proxy/oauth2.go	`0.00% <0.00%> (ø)`
server/proxy/uninstall.go	`0.00% <0.00%> (ø)`
server/httpin/restapi/call.go	`85.48% <100.00%> (+0.48%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d91438...99a4aa5. Read the comment docs.

hanzei

Good work 👍 Looking forward to seeing the metrics

server/proxy/oauth2.go

server/telemetry/telemetry.go

larkox · 2021-08-25T11:49:38Z

@DHaussermann Gentle reminder to review this. Not sure if you want to test it, since it is just adding telemetry.

larkox · 2021-08-31T08:01:27Z

@DHaussermann Friendly reminder on this.

hanzei · 2021-09-02T07:06:31Z

@larkox Also here

larkox · 2021-09-06T16:34:53Z

Added the environment variable to work with telemetry.

…lugin-cloudapps into AddTelemetry

hanzei · 2021-09-28T13:50:59Z

Discussed offline. QA testing will happen on Community. @larkox Please merge.

aaronrothschild · 2021-09-28T19:24:14Z

FYI @enelson720 this is new Telemetry that will be coming in for Apps Framework (ie: apps installed, uninstalled, etc.)

@larkox for future, would it make sense for us to track "how" an app is installed? (marketplace vs /apps install)

larkox · 2021-09-29T10:01:13Z

@aaronrothschild Yes, it makes sense. Not 100% sure if we have the mechanism to make the distinction (I guess so, but not 100% sure). Feel free to create a ticket to track it.

Add telemetry

7a4e122

larkox added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer labels Aug 4, 2021

larkox requested review from aaronrothschild, levb and catalintomai August 4, 2021 13:45

larkox added the 3: QA Review Requires review by a QA tester label Aug 4, 2021

larkox requested a review from DHaussermann August 4, 2021 13:45

aaronrothschild requested a review from enelson720 August 4, 2021 14:11

catalintomai reviewed Aug 4, 2021

View reviewed changes

larkox added 2 commits August 4, 2021 17:56

Fix copypaste typo

908a91c

Track oauth complete and only submit calls

96fcd65

levb added this to the v0.8.0 milestone Aug 9, 2021

levb approved these changes Aug 9, 2021

View reviewed changes

hanzei added the Awaiting Submitter Action Blocked on the author label Aug 10, 2021

Merge branch 'master' into AddTelemetry

7477982

hanzei approved these changes Aug 12, 2021

View reviewed changes

server/proxy/oauth2.go Outdated Show resolved Hide resolved

server/telemetry/telemetry.go Outdated Show resolved Hide resolved

server/telemetry/telemetry.go Outdated Show resolved Hide resolved

hanzei requested a review from catalintomai August 12, 2021 10:20

larkox added 3 commits August 12, 2021 13:27

Address feedback

773dd92

Fix test

37ceb5d

Fix mocks

a6158ac

larkox removed 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer labels Aug 13, 2021

larkox removed the Awaiting Submitter Action Blocked on the author label Aug 13, 2021

catalintomai approved these changes Aug 16, 2021

View reviewed changes

hanzei approved these changes Aug 19, 2021

View reviewed changes

Merge branch 'master' into AddTelemetry

285a052

hanzei added the Docs/Needed Requires documentation label Aug 25, 2021

Merge remote-tracking branch 'origin/master' into AddTelemetry

7e46f4e

larkox mentioned this pull request Sep 6, 2021

Add telemetry related to Apps Framework mattermost/docs#5025

Merged

larkox added 2 commits September 20, 2021 13:20

Merge branch 'master' into AddTelemetry

4fa9a40

Merge branch 'AddTelemetry' of https://github.com/larkox/mattermost-p…

99a4aa5

…lugin-cloudapps into AddTelemetry

hanzei changed the title ~~Add telemetry~~ [MM-37324] Add telemetry Sep 21, 2021

larkox merged commit 4cd4927 into mattermost:master Sep 28, 2021

wiggin77 mentioned this pull request Aug 27, 2024

Pop-over displays too low on on first attempt if there is not enough vertical space below the post mattermost/mattermost-plugin-github#815

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-37324] Add telemetry #230

[MM-37324] Add telemetry #230

larkox commented Aug 4, 2021

aaronrothschild commented Aug 4, 2021 •

edited

Loading

larkox commented Aug 4, 2021

aaronrothschild commented Aug 4, 2021

catalintomai Aug 4, 2021

catalintomai Aug 4, 2021 •

edited

Loading

larkox commented Aug 9, 2021

levb commented Aug 9, 2021

codecov-commenter commented Aug 10, 2021 •

edited

Loading

hanzei left a comment

larkox commented Aug 25, 2021

larkox commented Aug 31, 2021

hanzei commented Sep 2, 2021

larkox commented Sep 6, 2021

hanzei commented Sep 28, 2021

aaronrothschild commented Sep 28, 2021 •

edited

Loading

larkox commented Sep 29, 2021

[MM-37324] Add telemetry #230

[MM-37324] Add telemetry #230

Conversation

larkox commented Aug 4, 2021

Summary

Ticket Link

aaronrothschild commented Aug 4, 2021 • edited Loading

larkox commented Aug 4, 2021

aaronrothschild commented Aug 4, 2021

catalintomai Aug 4, 2021

Choose a reason for hiding this comment

catalintomai Aug 4, 2021 • edited Loading

Choose a reason for hiding this comment

larkox commented Aug 9, 2021

levb commented Aug 9, 2021

codecov-commenter commented Aug 10, 2021 • edited Loading

Codecov Report

hanzei left a comment

Choose a reason for hiding this comment

larkox commented Aug 25, 2021

larkox commented Aug 31, 2021

hanzei commented Sep 2, 2021

larkox commented Sep 6, 2021

hanzei commented Sep 28, 2021

aaronrothschild commented Sep 28, 2021 • edited Loading

larkox commented Sep 29, 2021

aaronrothschild commented Aug 4, 2021 •

edited

Loading

catalintomai Aug 4, 2021 •

edited

Loading

codecov-commenter commented Aug 10, 2021 •

edited

Loading

aaronrothschild commented Sep 28, 2021 •

edited

Loading