Update HTTP Semantic Conventions to Stable Versions #118

evanlauer1 · 2024-05-22T19:32:11Z

Context

The OTEL spec declared the new HTTP semantic conventions as stable, so this PR is updating several instrumentation libraries to follow the new convenions. The updated libraries are http-client, wai, postgresql-simple, persistent-mysql, and yesod. Programmers can opt into the new conventions using the environment variable OTEL_SEMCONV_STABILITY_OPT_IN. The stable conventions will replace the old conventions in the next major release of hs-opentelemetry.

Related: Issue #103

Testing

Created and ran test suite for semConvStabilityOptIn.hs to check if the program correctly reads the environment variable based on this specification.
Did not test the changed instrumentation libraries to see if they add the correct attributes. The instrumentation libraries do not have any tests at present.

…e latest stable semantics if the environment variable OTEL_SEMCONV_STABILITY_OPT_IN is "http"

…ration

…_STABILITY_OPT_IN

… a comma-separated list

…le is read correctly

evanlauer1

Finished self-review.

evanlauer1 · 2024-05-22T19:33:41Z

api/hs-opentelemetry-api.cabal

These are all auto-generated changes

evanlauer1 · 2024-05-22T19:35:14Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

I don't know if this file should be in the api/src/OpenTelemetry directory or somewhere else. This seemed like the most appropriate place.

This seems fine. I'd probably prefer expanding the module name to OpenTelemetry.SemanticConventionsStabilityOptIn for clarity.

evanlauer1 · 2024-05-22T19:40:55Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

+getSemConvStabilityOptIn :: IO SemConvStabilityOptIn
+getSemConvStabilityOptIn = do
+  menv <- lookupEnv "OTEL_SEMCONV_STABILITY_OPT_IN"
+  let menvs = fmap T.strip . T.splitOn "," . T.pack <$> menv


There might be a better library to use than Data.Text for parsing. Also, T.strip strips whitespace from the environment variable and I'm not sure if that's necessary.

Looks fine to me

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

evanlauer1 · 2024-05-22T19:46:06Z

hie.yaml

Auto-generated using the command gen-hie > hie.yaml. This allows HLS to work in VSCode for this repository.

evanlauer1 · 2024-05-22T20:09:46Z

instrumentation/persistent-mysql/src/OpenTelemetry/Instrumentation/Persistent/MySQL.hs

      H.union
-        [ ("db.connection_string", fromString $ showsPrecConnectInfoMasked 0 ci "")
+        [ ("db.connection_string" :: Text, fromString $ showsPrecConnectInfoMasked 0 ci "")


:: Text is used because of OverloadedStrings

evanlauer1 · 2024-05-22T20:11:33Z

instrumentation/wai/hs-opentelemetry-instrumentation-wai.cabal

Auto-generated

evanlauer1 · 2024-05-22T20:11:49Z

instrumentation/wai/package.yaml

@@ -28,6 +28,7 @@ dependencies:
 - bytestring
 - network
 - iproute
+- unordered-containers


for Data.HashMap.Strict

evanlauer1 · 2024-05-22T20:14:17Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient/Raw.hs

-      -- , ("http.response_content_length_uncompressed", _)
-      -- , ("net.transport")
-      -- , ("net.peer.name")
-      -- , ("net.peer.ip")


net.peer.ip is deleted because it is superseded by server.address which can either contain the contents of net.peer.name or net.peer.ip.

evanlauer1 · 2024-05-22T20:16:14Z

instrumentation/wai/src/OpenTelemetry/Instrumentation/Wai.hs

+                    Stable ->
+                      usefulCallsite
+                        `H.union` [
+                                    ( "user_agent.original"


This attribute must be provided to the sampler and is therefore has to be added when the span is created and not after.

andrewsmith

Nice work so far, Evan! This looks like excellent progress and is very thoughtful. I'll defer to Ian for approval, but this appears to be going in a good direction to me.

andrewsmith · 2024-05-22T20:57:48Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

+data SemConvStabilityOptIn = Stable | Both | Old deriving (Show, Eq)
+
+
+getSemConvStabilityOptIn :: IO SemConvStabilityOptIn


You could consider splitting this into two functions: a pure function that does the parsing and this that does the lookupEnv. Something like:

getSemConvStabilityOptIn :: IO SemConvStabilityOptIn getSemConvStabilityOptIn = parseSemConvStabilityOptIn <$> lookupEnv "OTEL_SEMCONV_STABILITY_OPT_IN" parseSemConvStabilityOptIn :: Maybe Text -> SemConvStabilityOptIn ...

The pure function can make testing a little bit easier, avoiding the set and unset calls. You could make an argument for skipping tests on getSemConvStabilityOptIn claiming that its implementation is trivial.

I split it into two functions, but I am thinking about keeping the tests the same. I like that the current tests make sure that the program is reading the env var correctly, and the fact that getSemConvStabilityOptIn is trivial means that it is still a good test for parseSemConvStabilityOptIn. It might be better to test the pure function though, not sure.

your tests are pretty nice, compact and legible, but one thing you could do with the pure function is reduce the number of env var tests to two (set/unset), and replace the rest with tests of the pure function that could be one-liners like parseSemConvStabiltyOptIn (Just "http") shouldBe Stable. if the setup was more onerous than setEnv "blah" I might feel more strongly about suggesting that approach, but as it stands I think it's just an aesthetic choice.

I updated the tests to use a looping structure like Max suggested. Does that satisfy you're aesthetic preference?

that's good too :)

andrewsmith · 2024-05-22T20:58:26Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

+getSemConvStabilityOptIn :: IO SemConvStabilityOptIn
+getSemConvStabilityOptIn = do
+  menv <- lookupEnv "OTEL_SEMCONV_STABILITY_OPT_IN"
+  let menvs = fmap T.strip . T.splitOn "," . T.pack <$> menv


Looks fine to me

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

andrewsmith · 2024-05-22T21:03:06Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient.hs

+[New HTTP semantic conventions have been declared stable.](https://opentelemetry.io/blog/2023/http-conventions-declared-stable/#migration-plan) Opt-in by setting the environment variable OTEL_SEMCONV_STABILITY_OPT_IN to
+- "http" - to use the stable conventions
+- "http/dup" - to emit both the old and the stable conventions
+Otherwise, the old conventions will be used. The stable conventions will replace the old conventions in the next major release of this library.


andrewsmith · 2024-05-22T21:06:36Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient.hs

-  t <- httpTracerProvider
-  inSpan'' t "withResponse" (addAttributesToSpanArguments callerAttributes spanArgs) $ \_wrSpan -> do
+  tracer <- httpTracerProvider
+  inSpan'' tracer "withResponse" (addAttributesToSpanArguments callerAttributes spanArgs) $ \_wrSpan -> do


I think the variable rename is reasonable. Haskellers can lean on the side of being terse and I think that partly stems from mathematics backgrounds, where single letter variable naming is common.

andrewsmith · 2024-05-22T21:29:12Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient/Raw.hs

+    case semConvStabilityOptIn of
+      Stable -> addStableAttributes
+      Both -> addStableAttributes >> addOldAttributes
+      Old -> addOldAttributes


I like the structure of this... very clean.

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient/Raw.hs

instrumentation/http-client/test/Spec.hs

andrewsmith · 2024-05-22T22:06:20Z

instrumentation/persistent-mysql/src/OpenTelemetry/Instrumentation/Persistent/MySQL.hs

I was surprised to see changes here, particularly special-casing on the same OTEL_SEMCONV_STABILITY_OPT_IN values. On the face of it, those seem unrelated. However, upon looking at the database conventions, they reference this. It appears that the overall changes aren't HTTP-only. So it seems like you are doing the correct thing according to the spec.

MaxGabriel · 2024-05-22T22:16:27Z

api/test/OpenTelemetry/SemConvStabilityOptInSpec.hs

+    it "defaults to 'Old' when env var has incorrect value" $ do
+      setEnv envVarName "http/du" -- intentionally similar to "http/dup" and "http"
+      semConvStabilityOptIn <- getSemConvStabilityOptIn
+      semConvStabilityOptIn `shouldBe` Old
+    it "is 'Stable' when env var is 'http'" $ do
+      setEnv envVarName "http"
+      semConvStabilityOptIn <- getSemConvStabilityOptIn
+      semConvStabilityOptIn `shouldBe` Stable
+    it "is 'Both' when env var is 'http/dup'" $ do
+      setEnv envVarName "http/dup"
+      semConvStabilityOptIn <- getSemConvStabilityOptIn
+      semConvStabilityOptIn `shouldBe` Both
+    it "is 'Both' when env var is 'http/dup,http'" $ do
+      setEnv envVarName "http/dup,http"
+      semConvStabilityOptIn <- getSemConvStabilityOptIn
+      semConvStabilityOptIn `shouldBe` Both
+    it "is 'Both' when env var is 'http,http/dup'" $ do
+      setEnv envVarName "http,http/dup"
+      semConvStabilityOptIn <- getSemConvStabilityOptIn
+      semConvStabilityOptIn `shouldBe` Both
+    it "is 'Both' when env var is 'http,something-random,http/dup'" $ do
+      setEnv envVarName "http,http/dup"
+      semConvStabilityOptIn <- getSemConvStabilityOptIn
+      semConvStabilityOptIn `shouldBe` Both


I think your way is preferable, just throwing out an option is to do these tests as a loop through [("testname", "envvarname", valueItShouldBe), ...]

Done. I was on the edge but I think it increases readability to do it that way because there is less repeated information to look through.

instrumentation/yesod/src/OpenTelemetry/Instrumentation/Yesod.hs

…StabilityOptIn

iand675

The direction here is really great! Left some feedback on how I'd evolve the interface a bit to future-proof it.

iand675 · 2024-05-23T17:08:35Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

This seems fine. I'd probably prefer expanding the module name to OpenTelemetry.SemanticConventionsStabilityOptIn for clarity.

iand675 · 2024-05-23T17:08:49Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

@@ -0,0 +1,26 @@
+{-# LANGUAGE OverloadedStrings #-}
+
+module OpenTelemetry.SemConvStabilityOptIn (


It would be good to haddock all the things in this module.

iand675 · 2024-05-23T17:09:10Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

+
+module OpenTelemetry.SemConvStabilityOptIn (
+  getSemConvStabilityOptIn,
+  SemConvStabilityOptIn (Stable, Both, Old),


You can do SemConvStabilityOptIn(..) here to export all of the options.

iand675 · 2024-05-23T17:11:54Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

+import System.Environment (lookupEnv)
+
+
+data SemConvStabilityOptIn = Stable | Both | Old deriving (Show, Eq)


Given that the spec for this environment variable says that it could expand to encompass other things besides the current set of values, I think we need to tweak this a bit so that it's a bit more forwards compatible. If something like sql/dup shows up in the future, we'd need some way to recognize it and let users consume it.

data SemanticConventionStabilityOptIn = HttpStableSemantics | HttpOldAndStableSemantics -- note that I'm deleting the current `Old` value here, see the next comment for what I'm thinking. | UnknownSemanticConvention Text deriving (Show, Eq, Generic) instance Hashable SemanticConventionStabilityOptIn

iand675 · 2024-05-23T17:42:55Z

api/src/OpenTelemetry/SemConvStabilityOptIn.hs

+data SemConvStabilityOptIn = Stable | Both | Old deriving (Show, Eq)
+
+
+parseSemConvStabilityOptIn :: Maybe String -> SemConvStabilityOptIn


Given that we need to account for other semantic convention options in the future, I think it'd be nice to produce an opt-in collection data type where we aim to expose functionality via functions that we export.

newtype SemanticConventionStabilityOptIns = SemanticConventionStabilityOptIns -- hide the constructor export { optIns :: HashSet SemanticConventionStabilityOptIn } semanticConventionOptionIsSet :: SemanticConventionStabilityOptIn -> SemanticConventionStabilityOptIns -> Bool semanticConventionOptionIsSet opt (SemanticConventionStabilityOptIn opts) = HashSet.member opt opts useNewHttpSemanticConventions :: SemanticConventionStabilityOptIns -> Bool useNewHttpSemanticConventions opts = semanticConventionOptionIsSet HttpStableSemantics opts || semanticConventionOptionIsSetHashSet.member HttpOldAndStableSemantics optIns useOldHttpSemanticConventions :: SemanticConventionStabilityOptIns -> Bool useOldHttpSemanticConventions SemanticConventionStabilityOptIns{..} = semanticConventionOptionIsSet HttpOldAndStableSemantics optIns || not (semanticConventionOptionIsSet HttpStableSemantics optIns) parseSemanticConventionStabilityOptIns :: Maybe String -> SemanticConventionStabilityOptIns parseSemanticConventionStabilityOptIns Nothing = SemanticConventionStabilityOptIns HashSet.empty parseSemanticConventionStabilityOptIns (Just env) = SemanticConventionStabilityOptIns $ HashSet.fromList $ fmap parseItem envs where envs = fmap T.strip . T.splitOn "," . T.pack $ env parseItem "http/dup" = HttpOldAndStableSemantics parseItem "http" = HttpStableSemantics parseItem other = UnknownSemanticConvention

iand675 · 2024-05-23T17:44:05Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient/Raw.hs

+            $ requestHeadersToRecord conf
+
+    semConvStabilityOptIn <- liftIO getSemConvStabilityOptIn
+    case semConvStabilityOptIn of


If you use the set-membership approach I suggested, then here you'd do something like:

when (useStableHttpSemanticConventions semConvStabilityOptIn) addStableAttributes when (useOldHttpSemanticConventions semConvStabilityOptIn) addOldAttributes

iand675 · 2024-05-23T17:57:19Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient/Raw.hs

+              (\h -> (\v -> ("http.request.header." <> T.decodeUtf8 (foldedCase h), toAttribute (T.decodeUtf8 v))) <$> lookup h (requestHeaders req))
+            $ requestHeadersToRecord conf
+
+    semConvStabilityOptIn <- liftIO getSemConvStabilityOptIn


I appreciate trying to avoid breaking the interface. I don't think it's great to have to read the environment variable and parse the functionality on each web request though, so I think we need to memoize the result if we're not going to pass in the opt-ins directly.

I'd recommend adding something like this to the Semantic Convention module:

import Control.Exception import Data.IORef import System.IO.Unsafe -- | Create a new memoized IO action using an 'IORef' under the surface. Note that -- the action may be run in multiple threads simultaneously, so this may not be -- thread safe (depending on the underlying action). For the sake of reading an environment -- variable and parsing some stuff, we don't have to be concerned about thread-safety. memoize :: IO a -> IO (IO a) memoize action = do ref <- newIORef Nothing pure $ do mres <- readIORef ref res <- case mres of Just res -> pure res Nothing -> do res <- tryAny action writeIORef ref $ Just res pure res either throwIO pure res -- This uses the global IORef trick: -- https://www.parsonsmatt.org/2021/04/21/global_ioref_in_template_haskell.html memoizedGetSemanticConventionOptIns :: IO SemanticConventionStabilityOptIns memoizedGetSemanticConventionOptIns = unsafePerformIO $ memoize getSemConvStabilityOptIn {-# NOINLINE memoizedGetSemanticConventionOptIns #-}

Then in places where you're calling getSemConvStabilityOptIn currently, use memoizedGetSemanticConventionOptIns instead.

iand675 · 2024-05-23T17:59:42Z

instrumentation/persistent-mysql/src/OpenTelemetry/Instrumentation/Persistent/MySQL.hs

+  let attrs' =
+        case semConvStabilityOptIn of
+          Stable -> addStableAttributes attrs
+          Both -> addStableAttributes . addOldAttributes $ attrs


I have a (very) minor preference towards using $ unless I'm doing pointfree style. It makes it a little bit easier to move things around in my experience.

That said, you could just write addStableAttributes $ addOldAttributes attrs and do away with the dot entirely.

…instead of booleans

…ion tests.

api/src/OpenTelemetry/SemanticsConfig.hs

Co-authored-by: Ian Duncan <[email protected]>

evanlauer1

Finished re-review

evanlauer1 · 2024-05-24T18:40:43Z

api/src/OpenTelemetry/SemanticsConfig.hs

+  | StableAndOld
+  | Old
+  deriving (Show, Eq)
+


I used this instead of set membership because I did not like that if the pattern was to use when (useStableHttpSemantics options) useStableAttributes it would be easy to forget to check both useStable and useOld because the compiler does not enforce it. This way, the compiler will say "missing cases in pattern match".

evanlauer1 · 2024-05-24T18:42:03Z

api/src/OpenTelemetry/SemanticsConfig.hs

+parseHttpOption envs
+  | "http/dup" `elem` envs = StableAndOld
+  | "http" `elem` envs = Stable
+  | otherwise = httpOption defaultOptions


Using httpOption defaultOptions means that the value if the environment variable is unset will always be the same as if it is set but missing configuration for http.

evanlauer1 · 2024-05-24T18:42:34Z

api/src/OpenTelemetry/SemanticsConfig.hs

+parseSemanticsOptions (Just env) = SemanticsOptions {..}
+  where
+    envs = fmap T.strip $ T.splitOn "," $ T.pack env
+    httpOption = parseHttpOption envs


The compiler will give a warning if one of the fields is unset.

evanlauer1 · 2024-05-24T18:43:54Z

api/src/OpenTelemetry/SemanticsConfig.hs

+unless it is necessary to retrieve the value of OTEL_SEMCONV_STABILITY_OPT_IN every time getSemanticsOptions' is called.
+-}
+getSemanticsOptions' :: IO SemanticsOptions
+getSemanticsOptions' = parseSemanticsOptions <$> lookupEnv "OTEL_SEMCONV_STABILITY_OPT_IN"


I used function' to indicate a different version of function. I'm not sure if that is the best way to indicate the difference. I want function to be the default and function' to only be used in necessary circumstances.

evanlauer1 · 2024-05-24T18:44:32Z

api/test/OpenTelemetry/SemanticsConfigSpec.hs

+    describe "HttpOption" $ do
+      it "defaults to 'Old' when env var has no value" $ do
+        unsetEnv envVarName
+        semanticsOptions <- getSemanticsOptions'


Uses getSemanticsOptions' so that the tests are not affected by memoization. Memoization is tested later.

evanlauer1 · 2024-05-24T18:45:29Z

instrumentation/http-client/src/OpenTelemetry/Instrumentation/HttpClient/Raw.hs

+    semanticsOptions <- liftIO getSemanticsOptions
+    case httpOption semanticsOptions of
+      Stable -> addStableAttributes
+      StableAndOld -> addStableAttributes >> addOldAttributes


StableAndOld seemed like a more specific name than Both.

evanlauer1 added 30 commits May 14, 2024 15:21

Work in Progress: Request and Response Attributes updated to match th…

5330fa4

…e latest stable semantics if the environment variable OTEL_SEMCONV_STABILITY_OPT_IN is "http"

Added Stable attributes to instrumentResponse

5e1bab4

Refactored to fit new httpTracerProvider return type

178e3b8

Fixed missing "do" notation and removed unnecessary imports

b4bff14

Fix typo and update comment

b18b623

Found unchanged http name

e5af40c

Found outdated http attribute names

9309250

Fixed cradle so HLS works

64c1331

Added Settings File for application-wide environment variable configu…

39ff844

…ration

Moved semConvStabilityOptIn and related types to OpenTelemetry.Settings

f9e987c

Changed code using HttpTracerProvider to work with changed return type

3e853aa

Fixed exports

a9520aa

Replaced general Settings.hs with specific SemConvStability.hs

77538ca

Removed comment because the problem it referenced is solved

17d2e22

removed unused language pragma

32c68be

fixed imports

b28866f

Replaced outdated http conventions based on the value of OTEL_SEMCONV…

7d92f10

…_STABILITY_OPT_IN

changed line about generation version

5200465

added unordered-containers to dependencies

1a1aba1

Moved semConvStabilityOptIn so that it is in scope

29bf4d7

Updated dependencies for testing

c2ea53f

WIP testing for http-client

6b1baaa

Generated by newer version

524faea

Removed WIP testing

6da0bef

Updated getSemConvStabilityOptIn so it parses environment variable as…

8a11e96

… a comma-separated list

derives Show and Eq for testing purposes

8b6f9f3

Added tests for SemConvStabilityOptIn to make sure environment variab…

e76b118

…le is read correctly

Updated Haddock to include blurb about the new HTTP semantic conventions

3ca8531

Removed hspec from dependencies

4669ae3

Auto-generated change. Removed hspec from build-depends

25d1cf5

evanlauer1 commented May 22, 2024

View reviewed changes

evanlauer1 marked this pull request as ready for review May 22, 2024 20:18

evanlauer1 requested a review from iand675 May 22, 2024 20:18

andrewsmith reviewed May 22, 2024

View reviewed changes

MaxGabriel reviewed May 22, 2024

View reviewed changes

instrumentation/yesod/src/OpenTelemetry/Instrumentation/Yesod.hs Outdated Show resolved Hide resolved

evanlauer1 added 5 commits May 22, 2024 15:44

Added newline to end of file

d8771ed

Split off parsing logic from getSemConvStabilityOptIn to parseSemConv…

76f062e

…StabilityOptIn

Added blank line for readability

5542954

Changed pure to Just to improve readability and contrast with Nothing

234e43e

Rewrote tests to eliminate boilerplate and increase readability

266b974

iand675 requested changes May 23, 2024

View reviewed changes

evanlauer1 added 4 commits May 23, 2024 16:40

Refactored and Renamed SemanticsConfig.hs to future proof

73c5dca

Refactored SemanticsConfig so options are Enums for pattern matching …

591ac8c

…instead of booleans

Updated SemanticsConfig tests. NOTE: does not currently pass memoizat…

77b4c5f

…ion tests.

Updated to match changes to SemanticsConfig.hs

e6b7fc4

evanlauer1 requested a review from iand675 May 24, 2024 01:03

iand675 reviewed May 24, 2024

View reviewed changes

api/src/OpenTelemetry/SemanticsConfig.hs Outdated Show resolved Hide resolved

evanlauer1 and others added 3 commits May 24, 2024 09:20

Fix infinite loop issue

e0179a4

Co-authored-by: Ian Duncan <[email protected]>

changed from . to $

ac5546f

Added haddocks

9523dad

evanlauer1 commented May 24, 2024

View reviewed changes

Formatting fixes

36fd483

iand675 merged commit 07f3ed5 into main May 28, 2024
6 of 8 checks passed

evanlauer1 deleted the update-http-semantics branch May 28, 2024 21:02

		data SemConvStabilityOptIn = Stable \| Both \| Old deriving (Show, Eq)


		getSemConvStabilityOptIn :: IO SemConvStabilityOptIn

		@@ -0,0 +1,26 @@
		{-# LANGUAGE OverloadedStrings #-}

		module OpenTelemetry.SemConvStabilityOptIn (

		import System.Environment (lookupEnv)


		data SemConvStabilityOptIn = Stable \| Both \| Old deriving (Show, Eq)

		data SemConvStabilityOptIn = Stable \| Both \| Old deriving (Show, Eq)


		parseSemConvStabilityOptIn :: Maybe String -> SemConvStabilityOptIn

Update HTTP Semantic Conventions to Stable Versions #118

Update HTTP Semantic Conventions to Stable Versions #118

Conversation

evanlauer1 commented May 22, 2024 • edited Loading

Context

Testing

evanlauer1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iand675 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iand675 May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanlauer1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanlauer1 commented May 22, 2024 •

edited

Loading

iand675 May 23, 2024 •

edited

Loading