Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Add instrumentation hook for wrapping statements #1219

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

iand675
Copy link
Contributor

@iand675 iand675 commented Mar 29, 2021

This PR is intended to enable more sophisticated instrumentation of SQL query/execute calls. The way that the statement cache currently works makes it difficult to associate metadata to instrumentation on per-call basis, so this PR adds facilities for that to use with things like OpenTelemetry or Honeycomb:

wrapConnection :: Span {- ^ the thing that we annotate with tracing info -} -> SqlBackend -> SqlBackend
wrapConnection span conn = conn
    { connStatementMiddleware = \t stmt -> do
        pure $ stmt
          { stmtExecute = recordAndTimeExecute span t $ stmtExecute stmt
          , stmtQuery = recordAndTimeQuery span t $ stmtQuery stmt ps
          }
    }

image

I wanted to get a little feedback on the particular way that this is implemented to see if there were any better ways to do this, but it seems like a minimally invasive change for the most part.

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Bumped the version number if there isn't an (unreleased) on the Changelog
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

@@ -221,6 +224,8 @@ deriving instance (Eq (Key record), Eq record) => Eq (Entity record)
deriving instance (Ord (Key record), Ord record) => Ord (Entity record)
deriving instance (Show (Key record), Show record) => Show (Entity record)
deriving instance (Read (Key record), Read record) => Read (Entity record)
deriving instance (Data (Key record), Data record) => Data (Entity record)
deriving instance (Typeable (Key record), Typeable record) => Typeable (Entity record)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary - Typeable is automagically derived for all types as of relatively recent GHCs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants