[WPB-10092] open telemetry instrumentation (#3901)

* [feat] add initial otel instrumentatoin for brig and galley - add wai and rpc instrumentation for brig - add wai and rpc instrumentation for galley - create new package wire-otel that houses otel related utils * [wip] start building http2 request and response instrumentatoin * [feat] minimal support for instrumenting http/2 clients * [fix] append headers to the end * [feat] add some surrounding span context in wire api federation * [wip] instrument cannon and gundeck * [chore] add developer documentation for open telemetry instrumentation * [chore] remove http2 directive spam * [chore] revert instrumentation of http/2 requests and responeses in fed * [chore] remove spans in services that are not on our request paths * [chore] changelog entry * [chore] don't instrument requests twice (in galley and in bilge) * [chore] remove http2 stub instrumentation and add futurework instead * [chore] keep vertical export lists Co-authored-by: Igor Ranieri <[email protected]> * Update services/brig/src/Brig/Run.hs Co-authored-by: Igor Ranieri <[email protected]> * Revert "Update services/brig/src/Brig/Run.hs" This reverts commit 4c78275. * regenerate cabal.configs, default.nixs * Fix deps in brig.cabal * Fixup * hi ci --------- Co-authored-by: Igor Ranieri <[email protected]> Co-authored-by: Matthias Fischmann <[email protected]>
wireapp · Sep 18, 2024 · ea4bfc1 · ea4bfc1
1 parent 95ce0d8
commit ea4bfc1
Show file tree

Hide file tree

Showing 43 changed files with 1,210 additions and 262 deletions.
diff --git a/cabal.project b/cabal.project
@@ -29,6 +29,7 @@ packages:
   , libs/wai-utilities/
   , libs/wire-api/
   , libs/wire-api-federation/
+  , libs/wire-otel/
   , libs/wire-message-proto-lens/
   , libs/wire-subsystems/
   , libs/zauth/

diff --git a/changelog.d/2-features/open-telemetry-instrumentation-brig-galley-gundeck-cannon b/changelog.d/2-features/open-telemetry-instrumentation-brig-galley-gundeck-cannon
@@ -0,0 +1 @@
+added open telemetry instrumentation for brig, galley, gundeck and cannon
diff --git a/changelog.d/3-bug-fixes/remove-spam-from-nginx b/changelog.d/3-bug-fixes/remove-spam-from-nginx
@@ -0,0 +1 @@
+removed spam from nginx (nginz) by using the new style http/2 directive
diff --git a/deploy/dockerephemeral/federation-v0/nginz/conf/integration.conf b/deploy/dockerephemeral/federation-v0/nginz/conf/integration.conf
@@ -7,13 +7,15 @@ listen 8081;
 # port.
 # This port is only used for trying out nginx http2 forwarding without TLS locally and should not
 # be ported to any production nginz config.
-listen 8090 http2;
+listen 8090;
 
 ######## TLS/SSL block start ##############
 #
 # Most integration tests simply use the http ports 8080 and 8081
 # But to also test tls forwarding, this port can be used.
 # This applies only locally, as for kubernetes (helm chart) based deployments,
 # TLS is terminated at the ingress level, not at nginz level
-listen 8443 ssl http2;
-listen [::]:8443 ssl http2;
+listen 8443 ssl;
+listen [::]:8443 ssl;
+
+http2 on;
diff --git a/deploy/dockerephemeral/federation-v1/nginz/conf/integration.conf b/deploy/dockerephemeral/federation-v1/nginz/conf/integration.conf
@@ -7,13 +7,15 @@ listen 8081;
 # port.
 # This port is only used for trying out nginx http2 forwarding without TLS locally and should not
 # be ported to any production nginz config.
-listen 8090 http2;
+listen 8090;
 
 ######## TLS/SSL block start ##############
 #
 # Most integration tests simply use the http ports 8080 and 8081
 # But to also test tls forwarding, this port can be used.
 # This applies only locally, as for kubernetes (helm chart) based deployments,
 # TLS is terminated at the ingress level, not at nginz level
-listen 8443 ssl http2;
-listen [::]:8443 ssl http2;
+listen 8443 ssl;
+listen [::]:8443 ssl;
+
+http2 on;
diff --git a/docs/src/developer/developer/open-telemetry.md b/docs/src/developer/developer/open-telemetry.md
@@ -0,0 +1,47 @@
+# OpenTelemetry Instrumentation
+
+## Current Status
+
+The following components have been instrumented:
+- brig
+- galley
+- gundeck
+- cannon
+
+## Known Issues and future work
+
+- Proper HTTP/2 instrumentation is missing for federator & co - this is related to http/2 outobj in the http2 libraray throwing away all structured information
+- Some parts of the service, such as background jobs, may need additional instrumentation. It's currently unclear if these are appearing in the tracing data.
+- we need to ingest the data into grafana tempo
+
+
+## Setup instructions for local use
+
+To view the tracing data:
+
+1. Start Jaeger using Docker:
+   ```bash
+   docker run --rm --name jaeger \
+     -e COLLECTOR_ZIPKIN_HOST_PORT=:9411 \
+     -p 6831:6831/udp \
+     -p 6832:6832/udp \
+     -p 5778:5778 \
+     -p 16686:16686 \
+     -p 4317:4317 \
+     -p 4318:4318 \
+     -p 14250:14250 \
+     -p 14268:14268 \
+     -p 14269:14269 \
+     -p 9411:9411 \
+     jaegertracing/all-in-one:latest
+   ```
+
+2. Start your services or run integration tests.
+3. Open the Jaeger UI at [http://localhost:16686/](http://localhost:16686/)
+
+## Relevant Resources
+
+We're using the `hs-opentelemetry-*` family of haskell packages available [here](https://github.com/iand675/hs-opentelemetry).
+
+- [hs-opentelemetry-instrumentation-wai](https://hackage.haskell.org/package/hs-opentelemetry-instrumentation-wai-0.1.0.0/docs/src/OpenTelemetry.Instrumentation.Wai.html#local-6989586621679045744)
+- [hs-opentelemetry-sdk](https://hackage.haskell.org/package/hs-opentelemetry-sdk-0.0.3.6)
diff --git a/integration/test/Testlib/ModService.hs b/integration/test/Testlib/ModService.hs
@@ -456,9 +456,10 @@ startNginzLocal resource = do
   -- override port configuration
   let portConfigTemplate =
         [r|listen {localPort};
-listen {http2_port} http2;
-listen {ssl_port} ssl http2;
-listen [::]:{ssl_port} ssl http2;
+listen {http2_port};
+listen {ssl_port} ssl;
+listen [::]:{ssl_port} ssl;
+http2 on;
 |]
   let portConfig =
         portConfigTemplate

diff --git a/libs/bilge/bilge.cabal b/libs/bilge/bilge.cabal
@@ -31,6 +31,7 @@ library
   default-extensions:
     AllowAmbiguousTypes
     BangPatterns
+    BlockArguments
     ConstraintKinds
     DataKinds
     DefaultSignatures
@@ -97,5 +98,6 @@ library
     , uri-bytestring
     , wai
     , wai-extra
+    , wire-otel
 
   default-language:   GHC2021
diff --git a/libs/bilge/default.nix b/libs/bilge/default.nix
@@ -26,6 +26,7 @@
 , uri-bytestring
 , wai
 , wai-extra
+, wire-otel
 }:
 mkDerivation {
   pname = "bilge";
@@ -53,6 +54,7 @@ mkDerivation {
     uri-bytestring
     wai
     wai-extra
+    wire-otel
   ];
   description = "Library for composing HTTP requests";
   license = lib.licenses.agpl3Only;

diff --git a/libs/bilge/src/Bilge/RPC.hs b/libs/bilge/src/Bilge/RPC.hs
@@ -36,9 +36,11 @@ import Control.Monad.Catch (MonadCatch, MonadThrow (..), try)
 import Data.Aeson (FromJSON, eitherDecode')
 import Data.CaseInsensitive (original)
 import Data.Text.Lazy (pack)
+import Data.Text.Lazy qualified as T
 import Imports hiding (log)
 import Network.HTTP.Client qualified as HTTP
 import System.Logger.Class
+import Wire.OpenTelemetry (withClientInstrumentation)
 
 class HasRequestId m where
   getRequestId :: m RequestId
@@ -69,7 +71,7 @@ instance Show RPCException where
       . showString "}"
 
 rpc ::
-  (MonadIO m, MonadCatch m, MonadHttp m, HasRequestId m) =>
+  (MonadUnliftIO m, MonadCatch m, MonadHttp m, HasRequestId m) =>
   LText ->
   (Request -> Request) ->
   m (Response (Maybe LByteString))
@@ -81,16 +83,17 @@ rpc sys = rpc' sys empty
 -- Note: 'syncIO' is wrapped around the IO action performing the request
 --       and any exceptions caught are re-thrown in an 'RPCException'.
 rpc' ::
-  (MonadIO m, MonadCatch m, MonadHttp m, HasRequestId m) =>
+  (MonadUnliftIO m, MonadCatch m, MonadHttp m, HasRequestId m) =>
   -- | A label for the remote system in case of 'RPCException's.
   LText ->
   Request ->
   (Request -> Request) ->
   m (Response (Maybe LByteString))
 rpc' sys r f = do
   rId <- getRequestId
-  let rq = f . requestId rId $ r
-  res <- try $ httpLbs rq id
+  let rq = f $ requestId rId r
+  res <- try $ withClientInstrumentation ("intra-call-to-" <> T.toStrict sys) \k -> do
+    k rq \r' -> httpLbs r' id
   case res of
     Left x -> throwM $ RPCException sys rq x
     Right x -> pure x

diff --git a/libs/wire-api-federation/src/Wire/API/Federation/Client.hs b/libs/wire-api-federation/src/Wire/API/Federation/Client.hs
@@ -138,8 +138,8 @@ withNewHttpRequest target req k = do
   sendReqMVar <- newEmptyMVar
   thread <- liftIO . async $ H2Manager.startPersistentHTTP2Connection ctx target cacheLimit sslRemoveTrailingDot tcpConnectionTimeout sendReqMVar
   let newConn = H2Manager.HTTP2Conn thread (putMVar sendReqMVar H2Manager.CloseConnection) sendReqMVar
-  H2Manager.sendRequestWithConnection newConn req $ \resp -> do
-    k resp <* newConn.disconnect
+  H2Manager.sendRequestWithConnection newConn req \resp ->
+    k resp `finally` newConn.disconnect
 
 performHTTP2Request ::
   Http2Manager ->

diff --git a/libs/wire-api-federation/wire-api-federation.cabal b/libs/wire-api-federation/wire-api-federation.cabal
@@ -38,6 +38,7 @@ library
   default-extensions:
     AllowAmbiguousTypes
     BangPatterns
+    BlockArguments
     ConstraintKinds
     DataKinds
     DefaultSignatures

diff --git a/libs/wire-otel/CHANGELOG.md b/libs/wire-otel/CHANGELOG.md
@@ -0,0 +1,5 @@
+# Revision history for wire-otel
+
+## 0.1.0.0 -- YYYY-mm-dd
+
+* First version. Released on an unsuspecting world.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		added open telemetry instrumentation for brig, galley, gundeck and cannon
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		removed spam from nginx (nginz) by using the new style http/2 directive