Skip to content

Commit 9696dcf

Browse files
committed
[server, papi] Track login completed metrics by outcome, type
1 parent 9f51b61 commit 9696dcf

File tree

6 files changed

+57
-33
lines changed

6 files changed

+57
-33
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (c) 2023 Gitpod GmbH. All rights reserved.
2+
// Licensed under the GNU Affero General Public License (AGPL).
3+
// See License.AGPL.txt in the project root for license information.
4+
5+
package oidc
6+
7+
import (
8+
"github.com/prometheus/client_golang/prometheus"
9+
)
10+
11+
var (
12+
loginCompletedTotal = prometheus.NewCounterVec(prometheus.CounterOpts{
13+
Namespace: "gitpod",
14+
Name: "login_completed_total",
15+
Help: "Total number of logins completed into gitpod, by status",
16+
}, []string{"status", "type"})
17+
)
18+
19+
func RegisterMetrics(registry *prometheus.Registry) {
20+
registry.MustRegister(loginCompletedTotal)
21+
}
22+
23+
func reportLoginCompleted(status string, typez string) {
24+
loginCompletedTotal.WithLabelValues(status, typez).Inc()
25+
}

components/public-api-server/pkg/oidc/router.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,18 +111,21 @@ func (s *Service) getCallbackHandler() http.HandlerFunc {
111111
useHttpErrors := state.UseHttpErrors
112112
if err != nil {
113113
log.WithError(err).Warn("Client SSO config not found")
114+
reportLoginCompleted("failed_client", "sso")
114115
respondeWithError(rw, r, "We were unable to find the SSO configuration you've requested. Please verify SSO is configured with your Organization owner.", http.StatusNotFound, useHttpErrors)
115116
return
116117
}
117118
oauth2Result := GetOAuth2ResultFromContext(r.Context())
118119
if oauth2Result == nil {
120+
reportLoginCompleted("failed_client", "sso")
119121
respondeWithError(rw, r, "OIDC precondition failure", http.StatusInternalServerError, useHttpErrors)
120122
return
121123
}
122124

123125
// nonce = number used once
124126
nonceCookie, err := r.Cookie(nonceCookieName)
125127
if err != nil {
128+
reportLoginCompleted("failed_client", "sso")
126129
respondeWithError(rw, r, "There was no nonce present on the request. Please try to sign-in in again.", http.StatusBadRequest, useHttpErrors)
127130
return
128131
}
@@ -133,6 +136,7 @@ func (s *Service) getCallbackHandler() http.HandlerFunc {
133136
})
134137
if err != nil {
135138
log.WithError(err).Warn("OIDC authentication failed")
139+
reportLoginCompleted("failed_client", "sso")
136140
respondeWithError(rw, r, "We've not been able to authenticate you with the OIDC Provider.", http.StatusInternalServerError, useHttpErrors)
137141
return
138142
}
@@ -143,34 +147,35 @@ func (s *Service) getCallbackHandler() http.HandlerFunc {
143147
err = s.activateAndVerifyClientConfig(r.Context(), config)
144148
if err != nil {
145149
log.WithError(err).Warn("Failed to mark OIDC Client Config as active")
150+
reportLoginCompleted("failed", "sso")
146151
respondeWithError(rw, r, "We've been unable to mark the selected OIDC config as active. Please try again.", http.StatusInternalServerError, useHttpErrors)
147152
return
148153
}
149154
}
150155

151156
if state.Verify {
157+
// Skip the sign-in on verify-only requests.
152158
err = s.markClientConfigAsVerified(r.Context(), config)
153159
if err != nil {
154160
log.Warn("Failed to mark config as verified: " + err.Error())
161+
reportLoginCompleted("failed", "sso")
155162
respondeWithError(rw, r, "Failed to mark config as verified", http.StatusInternalServerError, useHttpErrors)
156163
return
157164
}
158-
}
159-
160-
// Skip the sign-in on verify-only requests.
161-
if !state.Verify {
165+
} else {
162166
cookies, _, err := s.createSession(r.Context(), result, config)
163167
if err != nil {
164168
log.WithError(err).Warn("Failed to create session from downstream session provider.")
169+
reportLoginCompleted("failed", "sso")
165170
respondeWithError(rw, r, "We were unable to create a user session.", http.StatusInternalServerError, useHttpErrors)
166171
return
167172
}
168173
for _, cookie := range cookies {
169174
http.SetCookie(rw, cookie)
170175
}
171-
172176
}
173177

178+
reportLoginCompleted("succeeded", "sso")
174179
http.Redirect(rw, r, oauth2Result.ReturnToURL, http.StatusTemporaryRedirect)
175180
}
176181
}

components/server/src/auth/authenticator.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import { AuthFlow, AuthProvider } from "./auth-provider";
1616
import { TokenProvider } from "../user/token-provider";
1717
import { AuthProviderService } from "./auth-provider-service";
1818
import { UserService } from "../user/user-service";
19-
import { increaseLoginCounter } from "../prometheus-metrics";
2019
import { SignInJWT } from "./jwt";
20+
import { reportLoginCompleted } from "../prometheus-metrics";
2121

2222
@injectable()
2323
export class Authenticator {
@@ -166,8 +166,8 @@ export class Authenticator {
166166
}
167167

168168
if (!authProvider.info.verified) {
169-
increaseLoginCounter("failed", authProvider.info.host);
170-
log.info(`Login with "${host}" is not permitted.`, {
169+
reportLoginCompleted("failed_client", "git");
170+
log.warn(`Login with "${host}" is not permitted as the provider has not been verified.`, {
171171
"login-flow": true,
172172
ap: authProvider.info,
173173
});

components/server/src/auth/generic-auth-provider.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ import { TokenProvider } from "../user/token-provider";
2727
import { UserService } from "../user/user-service";
2828
import { AuthProviderService } from "./auth-provider-service";
2929
import { LoginCompletionHandler } from "./login-completion-handler";
30-
import { increaseLoginCounter } from "../prometheus-metrics";
3130
import { OutgoingHttpHeaders } from "http2";
3231
import { trackSignup } from "../analytics";
3332
import { daysBefore, isDateSmaller } from "@gitpod/gitpod-protocol/lib/util/timeutil";
3433
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
3534
import { VerificationService } from "../auth/verification-service";
3635
import { SignInJWT } from "./jwt";
36+
import { reportLoginCompleted } from "../prometheus-metrics";
3737

3838
/**
3939
* This is a generic implementation of OAuth2-based AuthProvider.
@@ -282,7 +282,7 @@ export abstract class GenericAuthProvider implements AuthProvider {
282282
if (!authFlow) {
283283
log.error(`(${strategyName}) Auth flow state is missing.`);
284284

285-
increaseLoginCounter("failed", this.host);
285+
reportLoginCompleted("failed", "git");
286286
response.redirect(this.getSorryUrl(`Auth flow state is missing.`));
287287
return;
288288
}
@@ -302,15 +302,15 @@ export abstract class GenericAuthProvider implements AuthProvider {
302302
// assert additional infomation is attached to current session
303303
if (!authFlow) {
304304
// The auth flow state info is missing in the session: count as client error
305-
increaseLoginCounter("failed_client", this.host);
305+
reportLoginCompleted("failed_client", "git");
306306

307307
log.error(cxt, `(${strategyName}) No session found during auth callback.`, { clientInfo });
308308
response.redirect(this.getSorryUrl(`Please allow Cookies in your browser and try to log in again.`));
309309
return;
310310
}
311311

312312
if (authFlow.host !== this.host) {
313-
increaseLoginCounter("failed", this.host);
313+
reportLoginCompleted("failed", "git");
314314

315315
log.error(cxt, `(${strategyName}) Host does not match.`, { clientInfo });
316316
response.redirect(this.getSorryUrl(`Host does not match.`));
@@ -326,8 +326,8 @@ export abstract class GenericAuthProvider implements AuthProvider {
326326

327327
if (callbackError) {
328328
// e.g. "access_denied"
329+
reportLoginCompleted("failed", "git");
329330

330-
increaseLoginCounter("failed", this.host);
331331
return this.sendCompletionRedirectWithError(response, {
332332
error: callbackError,
333333
description: callbackErrorDescription,
@@ -396,7 +396,7 @@ export abstract class GenericAuthProvider implements AuthProvider {
396396
return this.sendCompletionRedirectWithError(response, { error: err.message });
397397
}
398398

399-
increaseLoginCounter("failed", this.host);
399+
reportLoginCompleted("failed", "git");
400400
log.error(context, `(${strategyName}) Redirect to /sorry from verify callback`, err, {
401401
...defaultLogPayload,
402402
err,

components/server/src/auth/login-completion-handler.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { log, LogContext } from "@gitpod/gitpod-protocol/lib/util/logging";
1111
import { Config } from "../config";
1212
import { HostContextProvider } from "./host-context-provider";
1313
import { AuthProviderService } from "./auth-provider-service";
14-
import { increaseLoginCounter, reportJWTCookieIssued } from "../prometheus-metrics";
14+
import { reportJWTCookieIssued, reportLoginCompleted } from "../prometheus-metrics";
1515
import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
1616
import { trackLogin } from "../analytics";
1717
import { SessionHandler } from "../session-handler";
@@ -47,10 +47,8 @@ export class LoginCompletionHandler {
4747
});
4848
});
4949
} catch (err) {
50-
if (authHost) {
51-
increaseLoginCounter("failed", authHost);
52-
}
53-
log.error(logContext, `Redirect to /sorry on login`, err, { err });
50+
reportLoginCompleted("failed", "git");
51+
log.error(logContext, `Failed to login user. Redirecting to /sorry on login.`, err);
5452
response.redirect(this.config.hostUrl.asSorry("Oops! Something went wrong during login.").toString());
5553
return;
5654
}
@@ -75,8 +73,6 @@ export class LoginCompletionHandler {
7573
}
7674

7775
if (authHost) {
78-
increaseLoginCounter("succeeded", authHost);
79-
8076
/** no await */ trackLogin(user, request, authHost, this.analytics).catch((err) =>
8177
log.error({ userId: user.id }, "Failed to track Login.", err),
8278
);
@@ -87,6 +83,7 @@ export class LoginCompletionHandler {
8783
reportJWTCookieIssued();
8884

8985
log.info(logContext, `User is logged in successfully. Redirect to: ${returnTo}`);
86+
reportLoginCompleted("succeeded", "git");
9087
response.redirect(returnTo);
9188
}
9289

components/server/src/prometheus-metrics.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import * as prometheusClient from "prom-client";
88

99
export function registerServerMetrics(registry: prometheusClient.Registry) {
10-
registry.registerMetric(loginCounter);
10+
registry.registerMetric(loginComletedTotal);
1111
registry.registerMetric(apiConnectionCounter);
1212
registry.registerMetric(apiConnectionClosedCounter);
1313
registry.registerMetric(apiCallCounter);
@@ -36,12 +36,16 @@ export function registerServerMetrics(registry: prometheusClient.Registry) {
3636
registry.registerMetric(updateSubscribersRegistered);
3737
}
3838

39-
const loginCounter = new prometheusClient.Counter({
40-
name: "gitpod_server_login_requests_total",
41-
help: "Total amount of login requests",
42-
labelNames: ["status", "auth_host"],
39+
const loginComletedTotal = new prometheusClient.Counter({
40+
name: "gitpod_login_completed_total",
41+
help: "Total number of logins completed into gitpod, by status",
42+
labelNames: ["status", "type"],
4343
});
4444

45+
export function reportLoginCompleted(status: LoginCounterStatus, type: "git" | "sso") {
46+
loginComletedTotal.labels(status, type).inc();
47+
}
48+
4549
type LoginCounterStatus =
4650
// The login attempt failed due to a system error (picked up by alerts)
4751
| "failed"
@@ -50,13 +54,6 @@ type LoginCounterStatus =
5054
// The login attempt failed, because the client failed to provide complete session information, for instance.
5155
| "failed_client";
5256

53-
export function increaseLoginCounter(status: LoginCounterStatus, auth_host: string) {
54-
loginCounter.inc({
55-
status,
56-
auth_host,
57-
});
58-
}
59-
6057
const apiConnectionCounter = new prometheusClient.Counter({
6158
name: "gitpod_server_api_connections_total",
6259
help: "Total amount of established API connections",

0 commit comments

Comments
 (0)