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 all commits
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
29 changes: 23 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,19 @@ 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.

"""Ensure that the SSO handlers are loaded, if they are enabled by configuration.

This is mostly useful to ensure that the CAS/SAML/OIDC handlers register themselves
with the main SsoHandler.

It's safe to call this 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()