Skip to content

Add OpenTelemetry instrumentation support via enable-otel flag#7

Open
tonyalaribe wants to merge 1 commit intofpringle:mainfrom
tonyalaribe:instrument-pg
Open

Add OpenTelemetry instrumentation support via enable-otel flag#7
tonyalaribe wants to merge 1 commit intofpringle:mainfrom
tonyalaribe:instrument-pg

Conversation

@tonyalaribe
Copy link

When the flag is enabled, database operations automatically create OpenTelemetry spans using hs-opentelemetry-instrumentation-postgresql-simple. No code changes required. The same Effectful.PostgreSQL module transparently uses instrumented functions.

Includes workaround for forEach bug in the upstream instrumentation package.

When the flag is enabled, database operations automatically create
OpenTelemetry spans using hs-opentelemetry-instrumentation-postgresql-simple.
No code changes required - the same Effectful.PostgreSQL module transparently
uses instrumented functions.

Includes workaround for forEach bug in the upstream instrumentation package.
Copy link
Owner

@fpringle fpringle left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Just a few questions.

I'll add some commits afterwards - the CICD should test the build both with and without opentelemetry, and the Nix overlay should include the revision you pinned in cabal.project.

forEach q row forR =
unliftWithConn $ \conn unlift ->
#if OTEL
-- Workaround for bug in hs-opentelemetry-instrumentation-postgresql-simple
Copy link
Owner

Choose a reason for hiding this comment

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

What bug is this? If there's a Github issue on the hs-opentelemetry repo, I can follow it and make sure to update this line when it's patched.

Comment on lines +10 to +18

-- Use latest hs-opentelemetry from GitHub for GHC 9.12 support
-- (Hackage version is outdated)
source-repository-package
type: git
location: https://github.com/iand675/hs-opentelemetry.git
tag: 841a8e191fba3f2c76f6e6fa10d26b644856b7ee
subdir: api
instrumentation/postgresql-simple
Copy link
Owner

Choose a reason for hiding this comment

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

Could you elaborate on why this is needed? Will the Hackage version eventually catch up, so we'll be able to remove this?

I should probably add this to the nix overlay as well.

@fpringle
Copy link
Owner

fpringle commented Mar 4, 2026

Another thought: I'm not sure about controlling the OpenTelemetry integration via a cabal flag (which are a bit fragile, and slightly difficult to control when not building directly with cabal). Before this, there was only one reasonable way to do each of the PostgreSQL operations inside the WithConnection effect: by just delegating to the corresponding postgresql-simple library. But now, there are 2. In effectful, when I hear "multiple ways to perform an effect", I think "dynamic effect". This would also give us the option to decide whether or not to use the OpenTelemetry integration at runtime.

I'm going to have a go at wrapping all the PostgreSQL operations (forEach, insert etc) into a dynamic effect, which can then be interepreted by multiple interpreters using WithConnection. Then I'll see how that feels.

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