Report auth events in manage stage usage ping
What does this MR do?
Part 2 of #224102 (closed)
Provide aggregate auth event details in usage ping to help identify how often particular authentication methods are being used. This will help inform decision making about improvements and fixes.
Telemetry example
:usage_activity_by_stage=> {
:manage => {
...
:user_auth_by_provider=>{"ldap"=>10, "smartcard"=>20, "group_saml"=>10}
},
},
:usage_activity_by_stage_monthly=> {
:manage => {
...
:user_auth_by_provider=>{"ldap"=>1, "smartcard"=>2, "group_saml"=>1}
},
}
Database
Query
https://paste.depesz.com/s/Ehy
SELECT COUNT(DISTINCT "authentication_events"."user_id") FROM "authentication_events" WHERE "authentication_events"."success" = true AND "authentication_events"."provider" = 'standard' AND "authentication_events"."created_at" BETWEEN '2020-08-12 19:05:45.606188' AND '2020-09-09 19:05:45.606328' AND "authentication_events"."user_id" BETWEEN 0 AND 1000;
The above will happen once per provider configured on the system. For any given instance this is likely just one or two distinct providers, but it can be any of dozens we support. Of course, the last query will depend on how many entries there are. If there are thousands/millions it will be batched accordingly.
Query Plan
https://explain.depesz.com/s/hwuG
Aggregate (cost=2.17..2.18 rows=1 width=8) (actual time=0.017..0.017 rows=1 loops=1)
-> Index Only Scan using index_authentication_events_on_provider_user_id_created_at on authentication_events (cost=0.14..2.17 rows=1 width=8) (actual time=0.011..0.011 rows=0 loops=1)
Index Cond: ((provider = 'standard'::text) AND (user_id >= 0) AND (user_id <= 1000) AND (created_at >= '2020-08-16 19:05:45.606188-05'::timestamp with time zone) AND (created_at <= '2020-09-16 19:05:45.606328-05'::timestamp with time zone))
Heap Fetches: 0
Planning Time: 0.220 ms
Execution Time: 0.039 ms
(6 rows)
Migration up
== 20200916151442 AddResultIndexToAuthenticationEvents: migrating =============
-- transaction_open?()
-> 0.0000s
-- index_exists?(:authentication_events, [:provider, :user_id, :created_at], {:where=>"result = 1", :name=>"index_authentication_events_on_provider_user_id_created_at", :algorithm=>:concurrently})
-> 0.0030s
-- add_index(:authentication_events, [:provider, :user_id, :created_at], {:where=>"result = 1", :name=>"index_authentication_events_on_provider_user_id_created_at", :algorithm=>:concurrently})
-> 0.0093s
== 20200916151442 AddResultIndexToAuthenticationEvents: migrated (0.0127s) ====
Migration down
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Merge request reports
Activity
added database databasereview pending labels
2 Warnings For the following files, a review from the Data team and Telemetry team is recommended
Please check the ~telemetry guide and reach out to @gitlab-org/growth/telemetry/engineers group for a review.lib/gitlab/usage_data.rb
spec/lib/gitlab/usage_data_spec.rb
When adding, changing, or updating metrics, please update the Event dictionary Usage Ping table. Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited, or the chosen person is unavailable.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.
Category Reviewer Maintainer backend Jackie Fraser ( @jackie_fraser
) (UTC-4, 1 hour ahead of@dblessing
)Gabriel Mazetto ( @brodock
) (UTC+2, 7 hours ahead of@dblessing
)database Alex Ives ( @alexives
) (UTC-5, same timezone as@dblessing
)Yannis Roussos ( @iroussos
) (UTC+3, 8 hours ahead of@dblessing
)If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖assigned to @dblessing
changed milestone to %13.4
added devopsmanage featureaddition groupauthentication and authorization [DEPRECATED] labels and removed database databasereview pending ~14067045 ~14067067 labels
added typefeature label
added databasereview pending label
added database label
added sectiondev label
removed database ~14067045 sectiondev labels
added sectiondev label
mentioned in issue #231486 (closed)
added 4051 commits
-
e1c4246a...a729fbbe - 4048 commits from branch
dblessing-auth-events
- 7d0b4028 - Add AuthenticationEvent to track sign-ins
- 5aa96afc - Add AuthEvent to track authentication method
- 77d9eb65 - Report auth events in manage stage usage ping
Toggle commit list-
e1c4246a...a729fbbe - 4048 commits from branch
added 1 commit
- 86cb5441 - Report auth events in manage stage usage ping
added 1 commit
- 0cb89baf - Report auth events in manage stage usage ping
added 1 commit
- 95f7a4c0 - Report auth events in manage stage usage ping
added 1 commit
- cc368ea5 - Report auth events in manage stage usage ping
marked the checklist item Changelog entry as completed
marked the checklist item Code review guidelines as completed
marked the checklist item Merge request performance guidelines as completed
marked the checklist item Style guides as completed
marked the checklist item Database guides as completed
marked the checklist item Separation of EE specific content as completed
marked the checklist item Documentation (if required) as completed
added 1 commit
- 5d42cf69 - Report auth events in manage stage usage ping
- Resolved by Drew Blessing
@gitlab-org/growth/telemetry/engineers Will you please review for ~telemetry?
- Does this require any changes to the version app? I looked at the schema and it looks like usage ping is stored as a JSON blob so I think we're covered?
Also, can you please help me add this to the Event Dictionary sheet? I think we will need multiple lines since there's a hash value. The tricky part is the hash keys can be one of almost 2 dozen providers, and subject to addition at any time.
Edited by Drew Blessing
added 1 commit
- 635700ce - Report auth events in manage stage usage ping
added 1290 commits
-
635700ce...6b639561 - 1289 commits from branch
master
- d853d6d3 - Report auth events in manage stage usage ping
-
635700ce...6b639561 - 1289 commits from branch
assigned to @a_akgun
- Resolved by Drew Blessing
Pipeline failure due to master:broken - #246794 (closed). Will rebase once it's resolved.
assigned to @ali-gitlab and unassigned @a_akgun
- Resolved by Adam Hegyi