feat(analytics): authentication analytics #4429

vsrivatsa-juspay · 2024-04-22T14:46:47Z

Type of Change

Description

Authentication Analytics - with the following metrics:

AuthenticationAttemptCount
AuthenticationSuccessCount
ChallengeAttemptCount
ChallengeFlowCount
ChallengeSuccessCount
FrictionlessFlowCount
ThreeDsSdkCount

#4252

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

curl --location 'localhost:8080/analytics/v1/metrics/auth_events' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'Authorization: Bearer {token}' \ --data '[ { "timeRange": { "startTime": "2024-01-13T18:30:00Z", "endTime": "2024-04-30T18:30:00Z" }, "metrics": [ "three_ds_sdk_count", "authentication_attempt_count", "authentication_success_count", "challenge_flow_count", "frictionless_flow_count", "challenge_attempt_count", "challenge_success_count" ] } ]'

{
    "queryData": [
        {
            "three_ds_sdk_count": 3,
            "authentication_attempt_count": 3,
            "authentication_success_count": 3,
            "challenge_flow_count": 2,
            "frictionless_flow_count": 1,
            "challenge_attempt_count": 2,
            "challenge_success_count": 2,
            "time_bucket": "2024-01-09T00:00:00Z"
        }
    ],
    "metaData": [
        {
            "current_time_range": {
                "start_time": "2024-01-13T18:30:00.000Z",
                "end_time": "2024-04-30T18:30:00.000Z"
            }
        }
    ]
}

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible
I added a CHANGELOG entry if applicable

…regate enum

lsampras

Have some doubts understanding the implementation here

lsampras · 2024-04-25T09:57:59Z

crates/analytics/src/sqlx.rs

+            Self::AuthEvents => Err(error_stack::report!(ParsingError::UnknownError)
+                .attach_printable("ApiEvents table is not implemented for Sqlx"))?,


Suggested change

Self::AuthEvents => Err(error_stack::report!(ParsingError::UnknownError)

.attach_printable("ApiEvents table is not implemented for Sqlx"))?,

Self::AuthEvents => Ok("authentication".to_string()),

can use the authentication table

Currently the metrics are not from this table, but a combination of 3 clickhouse-only tables

can you share some example clickhouse sql queries that we expect to run for analytics?

crates/analytics/src/query.rs

lsampras · 2024-04-25T10:07:03Z

crates/analytics/src/auth_events/metrics.rs

+pub struct AuthEventMetricRow {
+    pub total: Option<bigdecimal::BigDecimal>,
+    pub count: Option<i64>,
+    pub time_bucket: Option<String>,
+    pub payment_method: Option<String>,
+    pub platform: Option<String>,
+    pub browser_name: Option<String>,
+    pub source: Option<String>,
+    pub component: Option<String>,
+    pub payment_experience: Option<String>,
+}


What is this row backed by?
the fields here seem similar to SDK events?

Forgot to update this

lsampras · 2024-04-25T10:07:23Z

crates/analytics/src/auth_events/core.rs

+            .transpose()
+            .change_context(AnalyticsError::UnknownError)?
+        {
+            logger::info!("Logging Result {:?}", data);


do we need this logger?

if we do add more context here as to which metric was this data for etc.

Removed the logger

lsampras · 2024-04-25T10:08:54Z

crates/analytics/src/auth_events/accumulator.rs

+#[derive(Debug, Default)]
+pub struct AverageAccumulator {
+    pub total: u32,
+    pub count: u32,
+}


not needed, right

lsampras · 2024-04-25T10:09:19Z

crates/analytics/src/auth_events/accumulator.rs

+}
+
+impl AuthEventMetricsAccumulator {
+    #[allow(dead_code)]


can we remove this?

lsampras · 2024-04-25T10:10:36Z

crates/analytics/src/auth_events/metrics/authentication_attempt_count.rs

        granularity: &Option<Granularity>,
        time_range: &TimeRange,
        pool: &T,
-    ) -> MetricsResult<Vec<(SdkEventMetricsBucketIdentifier, SdkEventMetricRow)>> {
+    ) -> MetricsResult<Vec<(AuthEventMetricsBucketIdentifier, AuthEventMetricRow)>> {
        let mut query_builder: QueryBuilder<T> = QueryBuilder::new(AnalyticsCollection::SdkEvents);


we are querying the SdkEvents table but deserializing the results to AuthEventMetricRow...
I'd prefer deserializing to SdkEventMetricRow & then having an Into::into implementation to get AuthEventMetricRow...

Since Authentication Analytics currently does not support filters/dimensions, the AuthEventMetricRow would only have time_bucket and count, so we can retain this

lsampras · 2024-04-25T10:11:17Z

crates/analytics/src/auth_events/metrics/challenge_attempt_count.rs

+        pool: &T,
+    ) -> MetricsResult<Vec<(AuthEventMetricsBucketIdentifier, AuthEventMetricRow)>> {
+        let mut query_builder: QueryBuilder<T> =
+            QueryBuilder::new(AnalyticsCollection::ConnectorEvents);


we are querying for connectorevents & deserializing to authevents? 😕

AuthEventMetricRow would only have time_bucket and count, so we can retain this

lsampras · 2024-04-25T10:13:02Z

crates/analytics/src/auth_events/metrics/challenge_success_count.rs

            .switch()?;

        query_builder
-            .add_filter_clause("category", "USER_EVENT")
+            .add_filter_clause("visitParamExtractRaw(response, 'transStatus')", "\"Y\"")


why not extract this out as a separate field in the mv?

The usecase is too specific to authentication analytics.
Since the other filters anyway restrict the data scanned, this should be fine

lsampras · 2024-04-25T10:13:49Z

crates/analytics/src/clickhouse.rs

@@ -360,7 +373,7 @@ impl ToSql<ClickhouseClient> for AnalyticsCollection {
            Self::Payment => Ok("payment_attempts".to_string()),
            Self::Refund => Ok("refunds".to_string()),
            Self::SdkEvents => Ok("sdk_events_audit".to_string()),
-            Self::ApiEvents => Ok("api_events_audit".to_string()),
+            Self::ApiEvents | Self::AuthEvents => Ok("api_events_audit".to_string()),


Will update this to authentication table in future
😓

lsampras · 2024-04-25T10:17:08Z

@vsrivatsa-juspay Is there going to be a new clickhouse table for this?
can you add a script of the clickhouse table creation if exists?

Co-authored-by: Sampras Lopes <lsampras@pm.me>

vsrivatsa-juspay · 2024-04-26T08:06:30Z

@vsrivatsa-juspay Is there going to be a new clickhouse table for this? can you add a script of the clickhouse table creation if exists?

In a future version. Will add the table scripts as well as refactor the queries as well

…t/authentication_analytics_2

…juspay/hyperswitch into feat/authentication_analytics_2

ShivanshMathurJuspay · 2024-04-25T09:58:40Z

crates/analytics/src/sqlx.rs

@@ -512,6 +512,8 @@ impl ToSql<SqlxClient> for AnalyticsCollection {
            Self::Refund => Ok("refund".to_string()),
            Self::SdkEvents => Err(error_stack::report!(ParsingError::UnknownError)
                .attach_printable("SdkEvents table is not implemented for Sqlx"))?,
+            Self::AuthEvents => Err(error_stack::report!(ParsingError::UnknownError)
+                .attach_printable("ApiEvents table is not implemented for Sqlx"))?,


authentication table right ?

ShivanshMathurJuspay · 2024-05-06T10:20:38Z

crates/analytics/src/clickhouse.rs

@@ -360,7 +373,7 @@ impl ToSql<ClickhouseClient> for AnalyticsCollection {
            Self::Payment => Ok("payment_attempts".to_string()),
            Self::Refund => Ok("refunds".to_string()),
            Self::SdkEvents => Ok("sdk_events_audit".to_string()),
-            Self::ApiEvents => Ok("api_events_audit".to_string()),
+            Self::ApiEvents | Self::AuthEvents => Ok("api_events_audit".to_string()),


api_events_audit is a ID based table , cannot use for time based filtering

If want can use api_events

…t/authentication_analytics_2

vsrivatsa-juspay requested review from a team as code owners April 22, 2024 14:46

vsrivatsa-juspay added 6 commits April 22, 2024 20:18

feat(analytics): Added load time metric for sdk, added percentile agg…

a5084a0

…regate enum

fix: address clippy lints

78b7ab1

fix: modify percentile type to u8

5f19cfd

fix: modified alias in queries

71576d6

Removed authentication metrics from sdk_events

361e0ba

feat:authentication analytics - separated from sdk events

3e58ffb

vsrivatsa-juspay force-pushed the feat/authentication_analytics_2 branch from b241cb5 to 3e58ffb Compare April 22, 2024 14:56

vsrivatsa-juspay added 2 commits April 22, 2024 20:31

Added common filters to the authentication metrics queries

5db9032

fix: compatibility with main branch changes

89b407d

vsrivatsa-juspay changed the title ~~feat: authentication analytics~~ feat(analytics): authentication analytics Apr 23, 2024

Merge branch 'main' into feat/authentication_analytics_2

26f2c05

vsrivatsa-juspay self-assigned this Apr 23, 2024

vsrivatsa-juspay added low-risk label to track PRs which might have less impact on hyperswitch after merge A-Analytics S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Apr 23, 2024

vsrivatsa-juspay linked an issue Apr 23, 2024 that may be closed by this pull request

[FEATURE] Authentication Analytics #4252

Closed

2 tasks

ivor-juspay previously approved these changes Apr 24, 2024

View reviewed changes

lsampras reviewed Apr 25, 2024

View reviewed changes

refactor: Update crates/analytics/src/query.rs

2376dcb

Co-authored-by: Sampras Lopes <lsampras@pm.me>

vsrivatsa-juspay dismissed ivor-juspay’s stale review via 2376dcb April 26, 2024 08:00

chore: run formatter

1c96e03

vsrivatsa-juspay added 3 commits April 26, 2024 13:40

chore: resolve comments

70f2a9f

Merge branch 'main' of https://github.com/juspay/hyperswitch into fea…

7d7c6b5

…t/authentication_analytics_2

Merge branch 'feat/authentication_analytics_2' of https://github.com/…

96a15b1

…juspay/hyperswitch into feat/authentication_analytics_2

vsrivatsa-juspay requested review from ivor-juspay and lsampras April 28, 2024 17:42

vsrivatsa-juspay requested a review from ShivanshMathurJuspay April 28, 2024 17:43

vsrivatsa-juspay and others added 4 commits April 28, 2024 23:13

Merge branch 'main' into feat/authentication_analytics_2

883c165

Merge branch 'main' into feat/authentication_analytics_2

dc2c26d

chore: address clippy lints

d3c1954

Merge branch 'feat/authentication_analytics_2' of https://github.com/…

fa54dd0

…juspay/hyperswitch into feat/authentication_analytics_2

ivor-juspay previously approved these changes May 6, 2024

View reviewed changes

ShivanshMathurJuspay reviewed May 6, 2024

View reviewed changes

fix: updated AnalyticsCollection for connector event metrics

0872ae2

vsrivatsa-juspay dismissed ivor-juspay’s stale review via 0872ae2 May 6, 2024 11:28

Merge branch 'main' of https://github.com/juspay/hyperswitch into fea…

fdfaebe

…t/authentication_analytics_2

vsrivatsa-juspay requested review from ShivanshMathurJuspay and ivor-juspay and removed request for ShivanshMathurJuspay May 7, 2024 12:31

ShivanshMathurJuspay approved these changes May 8, 2024

View reviewed changes

ivor-juspay approved these changes May 8, 2024

View reviewed changes

Merge branch 'main' into feat/authentication_analytics_2

3389032

Gnanasundari24 enabled auto-merge May 9, 2024 10:45

vsrivatsa-juspay added 2 commits May 9, 2024 19:16

Merge branch 'main' into feat/authentication_analytics_2

9771d47

Merge branch 'main' into feat/authentication_analytics_2

ed8eea3

Gnanasundari24 added this pull request to the merge queue May 10, 2024

Merged via the queue into main with commit 24d1542 May 10, 2024
11 of 15 checks passed

Gnanasundari24 deleted the feat/authentication_analytics_2 branch May 10, 2024 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(analytics): authentication analytics #4429

feat(analytics): authentication analytics #4429

vsrivatsa-juspay commented Apr 22, 2024 •

edited

lsampras left a comment

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024

lsampras Apr 26, 2024

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024

lsampras Apr 25, 2024

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024 •

edited

lsampras Apr 25, 2024

vsrivatsa-juspay Apr 26, 2024 •

edited

lsampras commented Apr 25, 2024

vsrivatsa-juspay commented Apr 26, 2024

ShivanshMathurJuspay Apr 25, 2024

ShivanshMathurJuspay May 6, 2024

		Self::AuthEvents => Err(error_stack::report!(ParsingError::UnknownError)
		.attach_printable("ApiEvents table is not implemented for Sqlx"))?,

	Self::AuthEvents => Err(error_stack::report!(ParsingError::UnknownError)
	.attach_printable("ApiEvents table is not implemented for Sqlx"))?,
	Self::AuthEvents => Ok("authentication".to_string()),

feat(analytics): authentication analytics #4429

feat(analytics): authentication analytics #4429

Conversation

vsrivatsa-juspay commented Apr 22, 2024 • edited

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

lsampras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsrivatsa-juspay Apr 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsrivatsa-juspay Apr 26, 2024 • edited

Choose a reason for hiding this comment

lsampras commented Apr 25, 2024

vsrivatsa-juspay commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsrivatsa-juspay commented Apr 22, 2024 •

edited

vsrivatsa-juspay Apr 26, 2024 •

edited

vsrivatsa-juspay Apr 26, 2024 •

edited