diff --git a/changelog.d/5-internal/WPB-4406 b/changelog.d/5-internal/WPB-4406 new file mode 100644 index 00000000000..5313ca41b5d --- /dev/null +++ b/changelog.d/5-internal/WPB-4406 @@ -0,0 +1,2 @@ +- Extending the information returned in errors for Federator. Paths and response bodies, if available, are included in error logs. +- Prometheus metrics for outgoing and incoming federation requests added. diff --git a/integration/integration.cabal b/integration/integration.cabal index 57ae450ee8f..0041bca3610 100644 --- a/integration/integration.cabal +++ b/integration/integration.cabal @@ -90,6 +90,7 @@ library API.BrigInternal API.Cargohold API.Common + API.Federator API.Galley API.GalleyInternal API.Gundeck diff --git a/integration/test/API/Federator.hs b/integration/test/API/Federator.hs new file mode 100644 index 00000000000..089b79c45c7 --- /dev/null +++ b/integration/test/API/Federator.hs @@ -0,0 +1,24 @@ +module API.Federator where + +import Data.Function +import GHC.Stack +import Network.HTTP.Client qualified as HTTP +import Testlib.Prelude + +getMetrics :: + (HasCallStack, MakesValue domain) => + domain -> + (ServiceMap -> HostPort) -> + App Response +getMetrics domain service = do + req <- rawBaseRequestF domain service "i/metrics" + submit "GET" req + +rawBaseRequestF :: (HasCallStack, MakesValue domain) => domain -> (ServiceMap -> HostPort) -> String -> App HTTP.Request +rawBaseRequestF domain getService path = do + domainV <- objDomain domain + serviceMap <- getServiceMap domainV + + liftIO . HTTP.parseRequest $ + let HostPort h p = getService serviceMap + in "http://" <> h <> ":" <> show p <> ("/" <> joinHttpPath (splitHttpPath path)) diff --git a/integration/test/Test/Federator.hs b/integration/test/Test/Federator.hs index 6e90308940c..99bdf155f28 100644 --- a/integration/test/Test/Federator.hs +++ b/integration/test/Test/Federator.hs @@ -1,32 +1,28 @@ +{-# LANGUAGE OverloadedStrings #-} + module Test.Federator where +import API.Brig +import API.Federator (getMetrics) +import Data.Attoparsec.Text import Data.ByteString qualified as BS import Data.String.Conversions -import Network.HTTP.Client qualified as HTTP +import Data.Text +import SetupHelpers (randomUser) import Testlib.Prelude runFederatorMetrics :: (ServiceMap -> HostPort) -> App () runFederatorMetrics getService = do - let req = submit "GET" =<< rawBaseRequestF OwnDomain getService "/i/metrics" - handleRes res = res <$ res.status `shouldMatchInt` 200 - first <- bindResponse req handleRes - second <- bindResponse req handleRes + let handleRes res = res <$ res.status `shouldMatchInt` 200 + first <- bindResponse (getMetrics OwnDomain getService) handleRes + second <- bindResponse (getMetrics OwnDomain getService) handleRes assertBool "Two metric requests should never match" $ first.body /= second.body assertBool "Second metric response should never be 0 length (the first might be)" $ BS.length second.body /= 0 assertBool "The seconds metric response should have text indicating that it is returning metrics" $ - BS.isInfixOf (cs expectedString) second.body + BS.isInfixOf expectedString second.body where expectedString = "# TYPE http_request_duration_seconds histogram" -rawBaseRequestF :: (HasCallStack, MakesValue domain) => domain -> (ServiceMap -> HostPort) -> String -> App HTTP.Request -rawBaseRequestF domain getService path = do - domainV <- objDomain domain - serviceMap <- getServiceMap domainV - - liftIO . HTTP.parseRequest $ - let HostPort h p = getService serviceMap - in "http://" <> h <> ":" <> show p <> ("/" <> joinHttpPath (splitHttpPath path)) - -- The metrics setup for both internal and external federator servers -- are the same, so we can simply run the same test for both. testFederatorMetricsInternal :: App () @@ -34,3 +30,32 @@ testFederatorMetricsInternal = runFederatorMetrics federatorInternal testFederatorMetricsExternal :: App () testFederatorMetricsExternal = runFederatorMetrics federatorExternal + +testFederatorNumRequestsMetrics :: HasCallStack => App () +testFederatorNumRequestsMetrics = do + u1 <- randomUser OwnDomain def + u2 <- randomUser OtherDomain def + incomingBefore <- getMetric parseIncomingRequestCount OtherDomain OwnDomain + outgoingBefore <- getMetric parseOutgoingRequestCount OwnDomain OtherDomain + bindResponse (searchContacts u1 (u2 %. "name") OtherDomain) $ \resp -> + resp.status `shouldMatchInt` 200 + incomingAfter <- getMetric parseIncomingRequestCount OtherDomain OwnDomain + outgoingAfter <- getMetric parseOutgoingRequestCount OwnDomain OtherDomain + assertBool "Incoming requests count should have increased by at least 2" $ incomingAfter >= incomingBefore + 2 + assertBool "Outgoing requests count should have increased by at least 2" $ outgoingAfter >= outgoingBefore + 2 + where + getMetric :: (Text -> Parser Integer) -> Domain -> Domain -> App Integer + getMetric p domain origin = do + m <- getMetrics domain federatorInternal + d <- cs <$> asString origin + pure $ fromRight 0 (parseOnly (p d) (cs m.body)) + + parseIncomingRequestCount :: Text -> Parser Integer + parseIncomingRequestCount d = + manyTill anyChar (string ("com_wire_federator_incoming_requests{origin_domain=\"" <> d <> "\"} ")) + *> decimal + + parseOutgoingRequestCount :: Text -> Parser Integer + parseOutgoingRequestCount d = + manyTill anyChar (string ("com_wire_federator_outgoing_requests{target_domain=\"" <> d <> "\"} ")) + *> decimal diff --git a/libs/wai-utilities/src/Network/Wai/Utilities/Error.hs b/libs/wai-utilities/src/Network/Wai/Utilities/Error.hs index 01b7a4cee8f..6aeb602ede2 100644 --- a/libs/wai-utilities/src/Network/Wai/Utilities/Error.hs +++ b/libs/wai-utilities/src/Network/Wai/Utilities/Error.hs @@ -31,7 +31,7 @@ import Control.Error import Data.Aeson hiding (Error) import Data.Aeson.Types (Pair) import Data.Domain -import Data.Text.Lazy.Encoding (decodeUtf8) +import Data.Text.Lazy.Encoding (decodeUtf8, encodeUtf8) import Imports import Network.HTTP.Types @@ -50,16 +50,18 @@ instance Exception Error data ErrorData = FederationErrorData { federrDomain :: !Domain, - federrPath :: !Text + federrPath :: !Text, + federrResp :: !(Maybe LByteString) } deriving (Eq, Show, Typeable) instance ToJSON ErrorData where - toJSON (FederationErrorData d p) = + toJSON (FederationErrorData d p b) = object [ "type" .= ("federation" :: Text), "domain" .= d, - "path" .= p + "path" .= p, + "response" .= fmap decodeUtf8 b ] instance FromJSON ErrorData where @@ -67,6 +69,7 @@ instance FromJSON ErrorData where FederationErrorData <$> o .: "domain" <*> o .: "path" + <*> (fmap encodeUtf8 <$> (o .: "response")) -- | Assumes UTF-8 encoding. byteStringError :: Status -> LByteString -> LByteString -> Error diff --git a/libs/wai-utilities/src/Network/Wai/Utilities/Server.hs b/libs/wai-utilities/src/Network/Wai/Utilities/Server.hs index a0eaa4d5886..05856b3974b 100644 --- a/libs/wai-utilities/src/Network/Wai/Utilities/Server.hs +++ b/libs/wai-utilities/src/Network/Wai/Utilities/Server.hs @@ -398,9 +398,10 @@ logErrorMsg (Wai.Error c l m md) = . maybe id logErrorData md . msg (val "\"" +++ m +++ val "\"") where - logErrorData (Wai.FederationErrorData d p) = + logErrorData (Wai.FederationErrorData d p b) = field "domain" (domainText d) . field "path" p + . field "response" (fromMaybe "" b) logErrorMsgWithRequest :: Maybe ByteString -> Wai.Error -> Msg -> Msg logErrorMsgWithRequest mr e = diff --git a/libs/wire-api-federation/src/Wire/API/Federation/Client.hs b/libs/wire-api-federation/src/Wire/API/Federation/Client.hs index f2a220a2c3d..f9f0473306f 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/Client.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/Client.hs @@ -254,7 +254,8 @@ mkFailureResponse status domain path body { Wai.federrDomain = domain, Wai.federrPath = "/federation" - <> Text.decodeUtf8With Text.lenientDecode (LBS.toStrict path) + <> Text.decodeUtf8With Text.lenientDecode (LBS.toStrict path), + Wai.federrResp = pure body } } where diff --git a/libs/wire-api-federation/src/Wire/API/Federation/Error.hs b/libs/wire-api-federation/src/Wire/API/Federation/Error.hs index 02fd6403a44..6d057ea3a68 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/Error.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/Error.hs @@ -83,6 +83,7 @@ module Wire.API.Federation.Error ) where +import Data.Domain (Domain (..)) import Data.Text qualified as T import Data.Text.Encoding qualified as T import Data.Text.Lazy qualified as LT @@ -206,27 +207,35 @@ federationClientErrorToWai FederatorClientVersionMismatch = "internal-error" "Endpoint version mismatch in federation client" -federationRemoteHTTP2Error :: FederatorClientHTTP2Error -> Wai.Error -federationRemoteHTTP2Error FederatorClientNoStatusCode = - Wai.mkError - unexpectedFederationResponseStatus - "federation-http2-error" - "No status code in HTTP2 response" -federationRemoteHTTP2Error (FederatorClientHTTP2Exception e) = - Wai.mkError - unexpectedFederationResponseStatus - "federation-http2-error" - (LT.pack (displayException e)) -federationRemoteHTTP2Error (FederatorClientTLSException e) = - Wai.mkError - (HTTP.mkStatus 525 "SSL Handshake Failure") - "federation-tls-error" - (LT.pack (displayException e)) -federationRemoteHTTP2Error (FederatorClientConnectionError e) = - Wai.mkError - federatorConnectionRefusedStatus - "federation-connection-refused" - (LT.pack (displayException e)) +federationRemoteHTTP2Error :: Domain -> Text -> FederatorClientHTTP2Error -> Wai.Error +federationRemoteHTTP2Error domain path FederatorClientNoStatusCode = + let err = + Wai.mkError + unexpectedFederationResponseStatus + "federation-http2-error" + "No status code in HTTP2 response" + in err {Wai.errorData = pure $ Wai.FederationErrorData domain path Nothing} +federationRemoteHTTP2Error domain path (FederatorClientHTTP2Exception e) = + let err = + Wai.mkError + unexpectedFederationResponseStatus + "federation-http2-error" + (LT.pack (displayException e)) + in err {Wai.errorData = pure $ Wai.FederationErrorData domain path Nothing} +federationRemoteHTTP2Error domain path (FederatorClientTLSException e) = + let err = + Wai.mkError + (HTTP.mkStatus 525 "SSL Handshake Failure") + "federation-tls-error" + (LT.pack (displayException e)) + in err {Wai.errorData = pure $ Wai.FederationErrorData domain path Nothing} +federationRemoteHTTP2Error domain path (FederatorClientConnectionError e) = + let err = + Wai.mkError + federatorConnectionRefusedStatus + "federation-connection-refused" + (LT.pack (displayException e)) + in err {Wai.errorData = pure $ Wai.FederationErrorData domain path Nothing} federationClientHTTP2Error :: FederatorClientHTTP2Error -> Wai.Error federationClientHTTP2Error (FederatorClientConnectionError e) = @@ -240,14 +249,21 @@ federationClientHTTP2Error e = "federation-local-error" (LT.pack (displayException e)) -federationRemoteResponseError :: HTTP.Status -> Wai.Error -federationRemoteResponseError status = - Wai.mkError - unexpectedFederationResponseStatus - "federation-remote-error" - ( "A remote federator failed with status code " - <> LT.pack (show (HTTP.statusCode status)) - ) +federationRemoteResponseError :: Domain -> Text -> HTTP.Status -> LByteString -> Wai.Error +federationRemoteResponseError domain path status resp = + err + { Wai.errorData = pure $ Wai.FederationErrorData domain path $ pure resp + } + where + err = + Wai.mkError + unexpectedFederationResponseStatus + "federation-remote-error" + ( "A remote federator (" + <> LT.fromStrict domain._domainText + <> ") failed with status code " + <> LT.pack (show (HTTP.statusCode status)) + ) federationServantErrorToWai :: ClientError -> Wai.Error federationServantErrorToWai (DecodeFailure msg _) = federationInvalidBody msg diff --git a/services/federator/default.nix b/services/federator/default.nix index a6a88ed1d37..77517559a75 100644 --- a/services/federator/default.nix +++ b/services/federator/default.nix @@ -41,6 +41,7 @@ , pem , polysemy , polysemy-wire-zoo +, prometheus-client , QuickCheck , random , servant @@ -105,6 +106,7 @@ mkDerivation { pem polysemy polysemy-wire-zoo + prometheus-client servant servant-client-core servant-server diff --git a/services/federator/federator.cabal b/services/federator/federator.cabal index e3e28a30089..53895622a41 100644 --- a/services/federator/federator.cabal +++ b/services/federator/federator.cabal @@ -43,6 +43,7 @@ library Federator.ExternalServer Federator.Health Federator.InternalServer + Federator.Metrics Federator.MockServer Federator.Monitor Federator.Monitor.Internal @@ -134,6 +135,7 @@ library , pem , polysemy , polysemy-wire-zoo + , prometheus-client , servant , servant-client-core , servant-server diff --git a/services/federator/src/Federator/Env.hs b/services/federator/src/Federator/Env.hs index 52a581891a4..90e8b1c21cf 100644 --- a/services/federator/src/Federator/Env.hs +++ b/services/federator/src/Federator/Env.hs @@ -30,11 +30,17 @@ import Imports import Network.DNS.Resolver (Resolver) import Network.HTTP.Client qualified as HTTP import OpenSSL.Session (SSLContext) +import Prometheus import System.Logger.Class qualified as LC import Util.Options import Wire.API.Federation.Component import Wire.API.Routes.FederationDomainConfig (FederationDomainConfigs) +data FederatorMetrics = FederatorMetrics + { outgoingRequests :: Vector Text Counter, + incomingRequests :: Vector Text Counter + } + data Env = Env { _metrics :: Metrics, _applog :: LC.Logger, @@ -46,7 +52,8 @@ data Env = Env _externalPort :: Word16, _internalPort :: Word16, _httpManager :: HTTP.Manager, - _http2Manager :: IORef Http2Manager + _http2Manager :: IORef Http2Manager, + _federatorMetrics :: FederatorMetrics } makeLenses ''Env diff --git a/services/federator/src/Federator/ExternalServer.hs b/services/federator/src/Federator/ExternalServer.hs index 6ec2178efb7..733a838b8be 100644 --- a/services/federator/src/Federator/ExternalServer.hs +++ b/services/federator/src/Federator/ExternalServer.hs @@ -40,6 +40,7 @@ import Federator.Discovery import Federator.Env import Federator.Error.ServerError import Federator.Health qualified as Health +import Federator.Metrics import Federator.RPC import Federator.Response import Federator.Service @@ -103,7 +104,8 @@ server :: Member (Error ValidationError) r, Member (Error DiscoveryFailure) r, Member (Error ServerError) r, - Member (Input FederationDomainConfigs) r + Member (Input FederationDomainConfigs) r, + Member Metrics r ) => Manager -> Word16 -> @@ -125,7 +127,8 @@ callInward :: Member (Error ValidationError) r, Member (Error DiscoveryFailure) r, Member (Error ServerError) r, - Member (Input FederationDomainConfigs) r + Member (Input FederationDomainConfigs) r, + Member Metrics r ) => Component -> RPC -> @@ -134,6 +137,7 @@ callInward :: Wai.Request -> Sem r Wai.Response callInward component (RPC rpc) originDomain (CertHeader cert) wreq = do + incomingCounterIncr originDomain -- only POST is supported when (Wai.requestMethod wreq /= HTTP.methodPost) $ throw InvalidRoute diff --git a/services/federator/src/Federator/InternalServer.hs b/services/federator/src/Federator/InternalServer.hs index 5be50a2bde3..b9e3d903a36 100644 --- a/services/federator/src/Federator/InternalServer.hs +++ b/services/federator/src/Federator/InternalServer.hs @@ -29,6 +29,7 @@ import Data.Proxy import Federator.Env import Federator.Error.ServerError import Federator.Health qualified as Health +import Federator.Metrics (Metrics, outgoingCounterIncr) import Federator.RPC import Federator.Remote import Federator.Response @@ -44,8 +45,10 @@ import Servant.API import Servant.API.Extended.Endpath import Servant.Server (Tagged (..)) import Servant.Server.Generic +import System.Logger.Class qualified as Log import Wire.API.Federation.Component import Wire.API.Routes.FederationDomainConfig +import Wire.Sem.Logger (Logger, debug) data API mode = API { status :: @@ -75,7 +78,9 @@ server :: Member (Embed IO) r, Member (Error ValidationError) r, Member (Error ServerError) r, - Member (Input FederationDomainConfigs) r + Member (Input FederationDomainConfigs) r, + Member Metrics r, + Member (Logger (Log.Msg -> Log.Msg)) r ) => Manager -> Word16 -> @@ -93,7 +98,9 @@ callOutward :: Member (Embed IO) r, Member (Error ValidationError) r, Member (Error ServerError) r, - Member (Input FederationDomainConfigs) r + Member (Input FederationDomainConfigs) r, + Member Metrics r, + Member (Logger (Log.Msg -> Log.Msg)) r ) => Domain -> Component -> @@ -107,9 +114,15 @@ callOutward targetDomain component (RPC path) req = do -- No query parameters are allowed unless (BS.null . Wai.rawQueryString $ req) $ throw InvalidRoute - ensureCanFederateWith targetDomain + outgoingCounterIncr targetDomain body <- embed $ Wai.lazyRequestBody req + debug $ + Log.msg (Log.val "Federator outward call") + . Log.field "domain" targetDomain._domainText + . Log.field "component" (show component) + . Log.field "path" path + . Log.field "body" body resp <- discoverAndCall targetDomain diff --git a/services/federator/src/Federator/Metrics.hs b/services/federator/src/Federator/Metrics.hs new file mode 100644 index 00000000000..b2f01b6ebd1 --- /dev/null +++ b/services/federator/src/Federator/Metrics.hs @@ -0,0 +1,55 @@ +{-# LANGUAGE RecordWildCards #-} +{-# LANGUAGE TemplateHaskell #-} + +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2022 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Federator.Metrics + ( Metrics (..), + interpretMetrics, + outgoingCounterIncr, + incomingCounterIncr, + ) +where + +import Control.Lens (view) +import Data.Domain (Domain, domainText) +import Federator.Env +import Imports +import Polysemy +import Polysemy.Input (Input, inputs) +import Prometheus + +data Metrics m a where + OutgoingCounterIncr :: Domain -> Metrics m () + IncomingCounterIncr :: Domain -> Metrics m () + +makeSem ''Metrics + +interpretMetrics :: + ( Member (Input Env) r, + Member (Embed IO) r + ) => + Sem (Metrics ': r) a -> + Sem r a +interpretMetrics = interpret $ \case + OutgoingCounterIncr targetDomain -> do + m <- inputs (view federatorMetrics) + liftIO $ withLabel m.outgoingRequests (domainText targetDomain) incCounter + IncomingCounterIncr originDomain -> do + m <- inputs (view federatorMetrics) + liftIO $ withLabel m.incomingRequests (domainText originDomain) incCounter diff --git a/services/federator/src/Federator/Remote.hs b/services/federator/src/Federator/Remote.hs index e72682144b9..3741bad1bf9 100644 --- a/services/federator/src/Federator/Remote.hs +++ b/services/federator/src/Federator/Remote.hs @@ -32,6 +32,7 @@ import Data.Binary.Builder import Data.ByteString.Lazy qualified as LBS import Data.Domain import Data.Text qualified as Text +import Data.Text.Encoding (decodeUtf8) import Data.Text.Encoding qualified as Text import Data.Text.Encoding.Error qualified as Text import Federator.Discovery @@ -55,26 +56,33 @@ import Wire.Network.DNS.SRV data RemoteError = -- | This means that an error occurred while trying to make a request to a -- remote federator. - RemoteError SrvTarget FederatorClientHTTP2Error + RemoteError SrvTarget Text FederatorClientHTTP2Error | -- | This means that a request to a remote federator returned an error -- response. The error response could be due to an error in the remote -- federator itself, or in the services it proxied to. - RemoteErrorResponse SrvTarget HTTP.Status LByteString + RemoteErrorResponse SrvTarget Text HTTP.Status LByteString deriving (Show) instance AsWai RemoteError where - toWai (RemoteError _ e) = federationRemoteHTTP2Error e - toWai (RemoteErrorResponse _ status _) = - federationRemoteResponseError status + toWai (RemoteError target path e) = + let domain = Domain . decodeUtf8 $ target.srvTargetDomain + in federationRemoteHTTP2Error domain path e + toWai (RemoteErrorResponse target path status resp) = + let domain = Domain . decodeUtf8 $ target.srvTargetDomain + in federationRemoteResponseError domain path status resp - waiErrorDescription (RemoteError tgt e) = + waiErrorDescription (RemoteError tgt path e) = "Error while connecting to " <> displayTarget tgt + <> " on path " + <> path <> ": " <> Text.pack (displayException e) - waiErrorDescription (RemoteErrorResponse tgt status body) = + waiErrorDescription (RemoteErrorResponse tgt path status body) = "Federator at " <> displayTarget tgt + <> " on path " + <> path <> " failed with status code " <> Text.pack (show (HTTP.statusCode status)) <> ": " @@ -112,12 +120,13 @@ interpretRemote = interpret $ \case let path = LBS.toStrict . toLazyByteString $ HTTP.encodePathSegments ["federation", componentName component, rpc] + pathT = decodeUtf8 path -- filter out Host header, because the HTTP2 client adds it back headers' = filter ((/= "Host") . fst) headers req' = HTTP2.requestBuilder HTTP.methodPost path headers' body mgr <- input - resp <- mapError (RemoteError target) . (fromEither @FederatorClientHTTP2Error =<<) . embed $ + resp <- mapError (RemoteError target pathT) . (fromEither @FederatorClientHTTP2Error =<<) . embed $ Codensity $ \k -> E.catches (H2Manager.withHTTP2Request mgr (True, hostname, fromIntegral port) req' (consumeStreamingResponseWith $ k . Right)) @@ -132,6 +141,7 @@ interpretRemote = interpret $ \case throw $ RemoteErrorResponse target + pathT (responseStatusCode resp) (toLazyByteString bdy) pure resp diff --git a/services/federator/src/Federator/Response.hs b/services/federator/src/Federator/Response.hs index 8c447cbc2fe..e7089d9a6d5 100644 --- a/services/federator/src/Federator/Response.hs +++ b/services/federator/src/Federator/Response.hs @@ -34,6 +34,7 @@ import Federator.Discovery import Federator.Env import Federator.Error import Federator.Error.ServerError +import Federator.Metrics (Metrics, interpretMetrics) import Federator.Options import Federator.Remote import Federator.Service @@ -137,7 +138,8 @@ serveServant middleware server env port = genericServe server type AllEffects = - '[ Remote, + '[ Metrics, + Remote, DiscoverFederator, DNSLookup, -- needed by DiscoverFederator ServiceStreaming, @@ -175,6 +177,7 @@ runFederator env = . runDNSLookupWithResolver (view dnsResolver env) . runFederatorDiscovery . interpretRemote + . interpretMetrics streamingResponseToWai :: StreamingResponse -> Wai.Response streamingResponseToWai resp = diff --git a/services/federator/src/Federator/Run.hs b/services/federator/src/Federator/Run.hs index e3072294ec6..676d5e233c3 100644 --- a/services/federator/src/Federator/Run.hs +++ b/services/federator/src/Federator/Run.hs @@ -47,6 +47,7 @@ import Federator.Options as Opt import Imports import Network.DNS qualified as DNS import Network.HTTP.Client qualified as HTTP +import Prometheus import System.Logger qualified as Log import System.Logger.Extended qualified as LogExt import Util.Options @@ -104,8 +105,27 @@ newEnv o _dnsResolver _applog _domainConfigs = do _httpManager <- initHttpManager sslContext <- mkTLSSettingsOrThrow _runSettings _http2Manager <- newIORef =<< mkHttp2Manager sslContext + _federatorMetrics <- mkFederatorMetrics pure Env {..} +mkFederatorMetrics :: IO FederatorMetrics +mkFederatorMetrics = + FederatorMetrics + <$> register + ( vector "target_domain" $ + counter $ + Prometheus.Info + "com_wire_federator_outgoing_requests" + "Number of outgoing requests" + ) + <*> register + ( vector "origin_domain" $ + counter $ + Prometheus.Info + "com_wire_federator_incoming_requests" + "Number of incoming requests" + ) + closeEnv :: Env -> IO () closeEnv e = do Log.flush $ e ^. applog diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs index c5c13ea41af..a93a4fa78bf 100644 --- a/services/federator/test/integration/Test/Federator/IngressSpec.hs +++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs @@ -106,9 +106,9 @@ spec env = do (Aeson.fromEncoding (Aeson.toEncoding hdl)) liftToCodensity . embed $ case r of Right _ -> expectationFailure "Expected client certificate error, got response" - Left (RemoteError _ _) -> + Left (RemoteError {}) -> expectationFailure "Expected client certificate error, got remote error" - Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400 + Left (RemoteErrorResponse _ _ status _) -> status `shouldBe` HTTP.status400 -- FUTUREWORK: ORMOLU_DISABLE -- @END diff --git a/services/federator/test/unit/Test/Federator/ExternalServer.hs b/services/federator/test/unit/Test/Federator/ExternalServer.hs index 2f95e96aa24..66961412305 100644 --- a/services/federator/test/unit/Test/Federator/ExternalServer.hs +++ b/services/federator/test/unit/Test/Federator/ExternalServer.hs @@ -27,6 +27,7 @@ import Data.Text.Encoding qualified as Text import Federator.Discovery import Federator.Error.ServerError (ServerError (..)) import Federator.ExternalServer +import Federator.Metrics import Federator.Options import Federator.Response import Federator.Service (Service (..), ServiceStreaming) @@ -70,6 +71,11 @@ tests = testMethod ] +interpretMetricsEmpty :: Sem (Metrics ': r) a -> Sem r a +interpretMetricsEmpty = interpret $ \case + OutgoingCounterIncr _ -> pure () + IncomingCounterIncr _ -> pure () + exampleRequest :: FilePath -> ByteString -> IO Wai.Request exampleRequest certFile path = do cert <- BS.readFile certFile @@ -113,8 +119,15 @@ requestBrigSuccess = "test/resources/unit/localhost.example.com.pem" "/federation/brig/get-user-by-handle" Right cert <- decodeCertificate <$> BS.readFile "test/resources/unit/localhost.example.com.pem" + + let assertMetrics :: Member (Embed IO) r => Sem (Metrics ': r) a -> Sem r a + assertMetrics = interpret $ \case + OutgoingCounterIncr _ -> embed @IO $ assertFailure "Should not increment outgoing counter" + IncomingCounterIncr od -> embed @IO $ od @?= aValidDomain + (actualCalls, res) <- runM + . assertMetrics . runOutputList . mockService HTTP.ok200 . assertNoError @ValidationError @@ -142,6 +155,7 @@ requestBrigFailure = (actualCalls, res) <- runM + . interpretMetricsEmpty . runOutputList . mockService HTTP.notFound404 . assertNoError @ValidationError @@ -172,6 +186,7 @@ requestGalleySuccess = runM $ do (actualCalls, res) <- runOutputList + . interpretMetricsEmpty . mockService HTTP.ok200 . assertNoError @ValidationError . assertNoError @DiscoveryFailure @@ -318,7 +333,8 @@ testMethod = testInterpretter :: IORef [Call] -> Sem - '[ Input FederationDomainConfigs, + '[ Metrics, + Input FederationDomainConfigs, Input RunSettings, DiscoverFederator, Error DiscoveryFailure, @@ -341,6 +357,7 @@ testInterpretter serviceCallsRef = . mockDiscoveryTrivial . runInputConst noClientCertSettings . runInputConst scaffoldingFederationDomainConfigs + . interpretMetricsEmpty exampleDomain :: Text exampleDomain = "localhost.example.com" diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs index 6d0e61cd393..9a433081f94 100644 --- a/services/federator/test/unit/Test/Federator/InternalServer.hs +++ b/services/federator/test/unit/Test/Federator/InternalServer.hs @@ -25,6 +25,7 @@ import Data.Default import Data.Domain import Federator.Error.ServerError import Federator.InternalServer (callOutward) +import Federator.Metrics import Federator.RPC import Federator.Remote import Federator.Validation @@ -86,6 +87,12 @@ federatedRequestSuccess = responseHttpVersion = HTTP.http20, responseBody = source ["\"bar\""] } + + let assertMetrics :: Member (Embed IO) r => Sem (Metrics ': r) a -> Sem r a + assertMetrics = interpret $ \case + OutgoingCounterIncr td -> embed @IO $ td @?= targetDomain + IncomingCounterIncr _ -> embed @IO $ assertFailure "Should not increment incoming counter" + res <- runM . interpretCall @@ -94,6 +101,7 @@ federatedRequestSuccess = . discardTinyLogs . runInputConst settings . runInputConst (FederationDomainConfigs AllowDynamic [FederationDomainConfig (Domain "target.example.com") FullSearch] 10) + . assertMetrics $ callOutward targetDomain Brig (RPC "get-user-by-handle") request Wai.responseStatus res @?= HTTP.status200 body <- Wai.lazyResponseBody res @@ -126,6 +134,9 @@ federatedRequestFailureAllowList = responseHttpVersion = HTTP.http20, responseBody = source ["\"bar\""] } + let interpretMetricsEmpty = interpret $ \case + OutgoingCounterIncr _ -> pure () + IncomingCounterIncr _ -> pure () eith <- runM @@ -136,6 +147,7 @@ federatedRequestFailureAllowList = . discardTinyLogs . runInputConst settings . runInputConst (FederationDomainConfigs AllowDynamic [FederationDomainConfig (Domain "hello.world") FullSearch] 10) + . interpretMetricsEmpty $ callOutward targetDomain Brig (RPC "get-user-by-handle") request eith @?= Left (FederationDenied targetDomain) diff --git a/services/federator/test/unit/Test/Federator/Remote.hs b/services/federator/test/unit/Test/Federator/Remote.hs index f9a1f1161ed..af13e26f1d9 100644 --- a/services/federator/test/unit/Test/Federator/Remote.hs +++ b/services/federator/test/unit/Test/Federator/Remote.hs @@ -142,14 +142,14 @@ testValidatesCertificateWrongHostname = withMockServer certForWrongDomain $ \port -> do tlsSettings <- mkTLSSettingsOrThrow settings runCodensity (mkTestCall tlsSettings "localhost" port) $ \case - Left (RemoteError _ (FederatorClientTLSException _)) -> pure () + Left (RemoteError _ _ (FederatorClientTLSException _)) -> pure () Left x -> assertFailure $ "Expected TLS failure, got: " <> show x Right _ -> assertFailure "Expected connection with the server to fail", testCase "when the server's certificate does not have the server key usage flag" $ withMockServer certWithoutServerKeyUsage $ \port -> do tlsSettings <- mkTLSSettingsOrThrow settings runCodensity (mkTestCall tlsSettings "localhost" port) $ \case - Left (RemoteError _ (FederatorClientTLSException _)) -> pure () + Left (RemoteError _ _ (FederatorClientTLSException _)) -> pure () Left x -> assertFailure $ "Expected TLS failure, got: " <> show x Right _ -> assertFailure "Expected connection with the server to fail" ] @@ -160,7 +160,7 @@ testConnectionError :: TestTree testConnectionError = testCase "connection failures are reported correctly" $ do tlsSettings <- mkTLSSettingsOrThrow settings runCodensity (mkTestCall tlsSettings "localhost" 1) $ \case - Left (RemoteError _ (FederatorClientConnectionError _)) -> pure () + Left (RemoteError _ _ (FederatorClientConnectionError _)) -> pure () Left x -> assertFailure $ "Expected connection error, got: " <> show x Right _ -> assertFailure "Expected connection with the server to fail" diff --git a/services/federator/test/unit/Test/Federator/Response.hs b/services/federator/test/unit/Test/Federator/Response.hs index 8bc559cd9a6..dcc0fec008c 100644 --- a/services/federator/test/unit/Test/Federator/Response.hs +++ b/services/federator/test/unit/Test/Federator/Response.hs @@ -95,6 +95,7 @@ testRemoteError = $ throw ( RemoteError (SrvTarget "example.com" 7777) + "" FederatorClientNoStatusCode ) body <- Wai.lazyResponseBody resp