Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Correctly initialise the synapse_user_logins metric. #10677

Merged
merged 2 commits into from
Aug 24, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10677.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug which caused the `synapse_user_logins_total` Prometheus metric not to be correctly initialised on restart.
18 changes: 18 additions & 0 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,22 @@
)


def init_counters_for_auth_provider(auth_provider_id: str) -> None:
"""Ensure the prometheus counters for the given auth provider are initialised

This fixes a problem where the counters are not reported for a given auth provider
until the user first logs in/registers.
"""
for is_guest in (True, False):
login_counter.labels(guest=is_guest, auth_provider=auth_provider_id)
for shadow_banned in (True, False):
registration_counter.labels(
guest=is_guest,
shadow_banned=shadow_banned,
auth_provider=auth_provider_id,
)


class LoginDict(TypedDict):
device_id: str
access_token: str
Expand Down Expand Up @@ -96,6 +112,8 @@ def __init__(self, hs: "HomeServer"):
self.session_lifetime = hs.config.session_lifetime
self.access_token_lifetime = hs.config.access_token_lifetime

init_counters_for_auth_provider("")

async def check_username(
self,
localpart: str,
Expand Down
2 changes: 2 additions & 0 deletions synapse/handlers/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from synapse.api.constants import LoginType
from synapse.api.errors import Codes, NotFoundError, RedirectException, SynapseError
from synapse.config.sso import SsoAttributeRequirement
from synapse.handlers.register import init_counters_for_auth_provider
from synapse.handlers.ui_auth import UIAuthSessionDataConstants
from synapse.http import get_request_user_agent
from synapse.http.server import respond_with_html, respond_with_redirect
Expand Down Expand Up @@ -213,6 +214,7 @@ def register_identity_provider(self, p: SsoIdentityProvider):
p_id = p.idp_id
assert p_id not in self._identity_providers
self._identity_providers[p_id] = p
init_counters_for_auth_provider(p_id)

def get_identity_providers(self) -> Mapping[str, SsoIdentityProvider]:
"""Get the configured identity providers"""
Expand Down
22 changes: 16 additions & 6 deletions synapse/rest/client/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ def __init__(self, hs: "HomeServer"):
burst_count=self.hs.config.rc_login_account.burst_count,
)

# ensure the CAS/SAML/OIDC handlers are loaded on this worker instance.
# The reason for this is to ensure that the auth_provider_ids are registered
# with SsoHandler, which in turn ensures that the login/registration prometheus
# counters are initialised for the auth_provider_ids.
_load_sso_handlers(hs)

def on_GET(self, request: SynapseRequest):
flows = []
if self.jwt_enabled:
Expand Down Expand Up @@ -499,12 +505,7 @@ class SsoRedirectServlet(RestServlet):
def __init__(self, hs: "HomeServer"):
# make sure that the relevant handlers are instantiated, so that they
# register themselves with the main SSOHandler.
if hs.config.cas_enabled:
hs.get_cas_handler()
if hs.config.saml2_enabled:
hs.get_saml_handler()
if hs.config.oidc_enabled:
hs.get_oidc_handler()
_load_sso_handlers(hs)
self._sso_handler = hs.get_sso_handler()
self._msc2858_enabled = hs.config.experimental.msc2858_enabled
self._public_baseurl = hs.config.public_baseurl
Expand Down Expand Up @@ -598,3 +599,12 @@ def register_servlets(hs, http_server):
SsoRedirectServlet(hs).register(http_server)
if hs.config.cas_enabled:
CasTicketServlet(hs).register(http_server)


def _load_sso_handlers(hs: "HomeServer"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a quick docstring, mainly its useful to know this is safe to call multiple times.

if hs.config.cas.cas_enabled:
hs.get_cas_handler()
if hs.config.saml2.saml2_enabled:
hs.get_saml_handler()
if hs.config.oidc.oidc_enabled:
hs.get_oidc_handler()