Skip to content

Commit

Permalink
Merge pull request #125 from NoRedInk/log-when-changing-query-timeout
Browse files Browse the repository at this point in the history
add a tracing span when modifying the query timeout
  • Loading branch information
michaelglass authored Nov 28, 2024
2 parents 25b0516 + 284f2ac commit 54b5b05
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 10 deletions.
13 changes: 9 additions & 4 deletions nri-redis/src/Redis/Handler.hs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import qualified Data.Text.Encoding
import qualified Database.Redis
import qualified Dict
import qualified GHC.Stack as Stack
import qualified Log
import qualified Platform
import qualified Redis.Internal as Internal
import qualified Redis.Script as Script
Expand Down Expand Up @@ -60,14 +61,18 @@ handlerAutoExtendExpire namespace settings = do
|> liftIO

-- | Sets a timeout for the query in milliseconds.
withQueryTimeoutMilliseconds :: Int -> Internal.Handler' x -> Internal.Handler' x
withQueryTimeoutMilliseconds :: Int -> Internal.Handler' x -> Task () (Internal.Handler' x)
withQueryTimeoutMilliseconds timeoutMs handler' =
handler' {Internal.queryTimeout = Settings.TimeoutQueryAfterMilliseconds timeoutMs}
(handler' {Internal.queryTimeout = Settings.TimeoutQueryAfterMilliseconds timeoutMs})
|> Task.succeed
|> Log.withContext "setting redis query timeout" [Log.context "timeoutMilliseconds" (Text.fromInt timeoutMs)]

-- | Disables timeout for query in milliseconds
withoutQueryTimeout :: Internal.Handler' x -> Internal.Handler' x
withoutQueryTimeout :: Internal.Handler' x -> Task () (Internal.Handler' x)
withoutQueryTimeout handler' =
handler' {Internal.queryTimeout = Settings.NoQueryTimeout}
(handler' {Internal.queryTimeout = Settings.NoQueryTimeout})
|> Task.succeed
|> Log.withContext "setting no redis query timeout" []

defaultExpiryKeysAfterSeconds :: Int -> Internal.HandlerAutoExtendExpire -> Internal.HandlerAutoExtendExpire
defaultExpiryKeysAfterSeconds secs handler' =
Expand Down
23 changes: 19 additions & 4 deletions nri-redis/test/Spec/Redis.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import qualified Prelude
-- put this at the top of the file so that adding tests doesn't push
-- the line number of the source location of this file down, which would
-- change golden test results
spanForTask :: (Show e) => Task e () -> Expect.Expectation' Platform.TracingSpan
spanForTask :: (Show e) => Task e a -> Expect.Expectation' Platform.TracingSpan
spanForTask task =
Expect.fromIO <| do
spanVar <- MVar.newEmptyMVar
Expand Down Expand Up @@ -134,15 +134,30 @@ observabilityTests handler =
Test.describe
"with 0 ms timeout"
[ Test.test "Redis.query reports the span data we expect" <| \() -> do
handlerThatExpiresImmediately <- Expect.succeeds (Redis.withQueryTimeoutMilliseconds 0 handler)
span <-
Redis.query (Redis.withQueryTimeoutMilliseconds 0 handler) (Redis.ping api)
Redis.query handlerThatExpiresImmediately (Redis.ping api)
|> spanForFailingTask
span
|> Debug.toString
|> Expect.all
[ Expect.equalToContentsOf (goldenResultsDir ++ "/observability-spec-timeout-reporting-redis-query"),
[ Expect.equalToContentsOf (goldenResultsDir ++ "/observability-spec-reporting-redis-query-timeout"),
\spanText -> Expect.true (Text.contains "Redis Query" spanText)
]
],
Test.test "Redis.withQueryTimeoutMilliseconds reports the span data we expect" <| \() -> do
span <-
Redis.withQueryTimeoutMilliseconds 0 handler
|> spanForTask
span
|> Debug.toString
|> Expect.equalToContentsOf (goldenResultsDir ++ "/observability-spec-reporting-with-query-timout"),
Test.test "Redis.withoutQueryTimeout reports the span data we expect" <| \() -> do
spanSettingTimeout <-
Redis.withoutQueryTimeout handler
|> spanForTask
spanSettingTimeout
|> Debug.toString
|> Expect.equalToContentsOf (goldenResultsDir ++ "/observability-spec-reporting-without-query-timout")
]
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ TracingSpan
{ srcLocPackage = "main"
, srcLocModule = "Spec.Redis"
, srcLocFile = "test/Spec/Redis.hs"
, srcLocStartLine = 138
, srcLocStartLine = 139
, srcLocStartCol = 13
, srcLocEndLine = 138
, srcLocEndLine = 139
, srcLocEndCol = 24
}
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
TracingSpan
{ name = "test-root"
, started = MonotonicTime { inMicroseconds = 0 }
, finished = MonotonicTime { inMicroseconds = 0 }
, frame =
Just
( "rootTracingSpanIO"
, SrcLoc
{ srcLocPackage = "main"
, srcLocModule = "Spec.Redis"
, srcLocFile = "test/Spec/Redis.hs"
, srcLocStartLine = 31
, srcLocStartCol = 7
, srcLocEndLine = 31
, srcLocEndCol = 33
}
)
, details = Nothing
, summary = Nothing
, succeeded = Succeeded
, containsFailures = False
, allocated = 0
, children =
[ TracingSpan
{ name = "setting redis query timeout"
, started = MonotonicTime { inMicroseconds = 0 }
, finished = MonotonicTime { inMicroseconds = 0 }
, frame =
Just
( "withContext"
, SrcLoc
{ srcLocPackage = "main"
, srcLocModule = "Redis.Handler"
, srcLocFile = "src/Redis/Handler.hs"
, srcLocStartLine = 68
, srcLocStartCol = 8
, srcLocEndLine = 68
, srcLocEndCol = 23
}
)
, details = Just "{\"timeoutMilliseconds\":\"0\"}"
, summary = Just "setting redis query timeout"
, succeeded = Succeeded
, containsFailures = False
, allocated = 0
, children = []
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
TracingSpan
{ name = "test-root"
, started = MonotonicTime { inMicroseconds = 0 }
, finished = MonotonicTime { inMicroseconds = 0 }
, frame =
Just
( "rootTracingSpanIO"
, SrcLoc
{ srcLocPackage = "main"
, srcLocModule = "Spec.Redis"
, srcLocFile = "test/Spec/Redis.hs"
, srcLocStartLine = 31
, srcLocStartCol = 7
, srcLocEndLine = 31
, srcLocEndCol = 33
}
)
, details = Nothing
, summary = Nothing
, succeeded = Succeeded
, containsFailures = False
, allocated = 0
, children =
[ TracingSpan
{ name = "setting no redis query timeout"
, started = MonotonicTime { inMicroseconds = 0 }
, finished = MonotonicTime { inMicroseconds = 0 }
, frame =
Just
( "withContext"
, SrcLoc
{ srcLocPackage = "main"
, srcLocModule = "Redis.Handler"
, srcLocFile = "src/Redis/Handler.hs"
, srcLocStartLine = 75
, srcLocStartCol = 8
, srcLocEndLine = 75
, srcLocEndCol = 23
}
)
, details = Just "{}"
, summary = Just "setting no redis query timeout"
, succeeded = Succeeded
, containsFailures = False
, allocated = 0
, children = []
}
]
}

0 comments on commit 54b5b05

Please sign in to comment.