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

MonadMask Support #20

Closed
timbess opened this issue Jun 22, 2022 · 5 comments
Closed

MonadMask Support #20

timbess opened this issue Jun 22, 2022 · 5 comments

Comments

@timbess
Copy link

timbess commented Jun 22, 2022

My MTL stack includes ExceptT and I want to leave the door open for StateT too, but UnliftIO doesn't support it. I poked around a bit and got it working by making versions of inSpan and bracketError that rely only on MonadMask instead

Would you accept a PR that adds a new version of inSpan that doesn't require UnliftIO?

Great project btw! Glad to see someone finally tackling this issue in Haskell.

@iand675
Copy link
Owner

iand675 commented Jun 22, 2022

I'd definitely accept a PR for this!

Ideally I think it'd be nice to have it as a separate package within the repo (maybe a util directory?) because I want to try to avoid adding new direct dependencies to hs-opentelemetry-api or hs-opentelemetry-sdk.

Also, for the reasons outlined here– I'd like to ensure that people opting in to tracing mtl stacks understand that there can be some subtle issues and maybe consider alternative approaches.

@timbess
Copy link
Author

timbess commented Jun 23, 2022

Awesome, I'll submit one soon then 👍

Sure, maybe something like hs-opentelemetry-masked?

Thanks for a link to some interesting discussion! I've actually read his FPComplete article before and I respectfully disagree with his argument (with regards to ExceptT at least). I'm glad you're open to at least supporting this style even if you don't prefer it though! I'm not a fan of libraries that don't support some behavior purely to stop the consumer from doing something they subjectively deem an anti-pattern. If it takes extra work to maintain or whatever, then sure, but people refusing to provide basic instances to control the way I choose to code really rubs me the wrong way.

Also, I'll also try to flesh out the B3 propagator if I have extra time. Again, great work on this library, so good to finally have some visibility into my Haskell services!

@iand675
Copy link
Owner

iand675 commented Jun 23, 2022

IIRC the instance comes from the exceptions library, so maybe hs-opentelemetry-util-exceptions?

I'm not a fan of libraries that don't support some behavior purely to stop the consumer from doing something they subjectively deem an anti-pattern. If it takes extra work to maintain or whatever, then sure, but people refusing to provide basic instances to control the way I choose to code really rubs me the wrong way.

Yeah, I understand. Ideally the hs-opentelemetry set of packages (especially the underlying hs-opentelemetry-api library) should be usable in any Haskell codebase due to the significant network effects that come from thorough instrumentation, so it'd be self-defeating to make it harder for people to use 😄. That being said, I don't want to cause subtle new issues crop up in people's instrumented code, so I just want to be conservative with the base offerings.

@timbess
Copy link
Author

timbess commented Jun 23, 2022

Sure, that makes sense to me 👍

@lf-
Copy link
Collaborator

lf- commented Nov 22, 2022

I believe this was fixed by #21.

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

No branches or pull requests

3 participants