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

Collector that uses EIO #51

Open
emjin opened this issue Jan 30, 2024 · 4 comments
Open

Collector that uses EIO #51

emjin opened this issue Jan 30, 2024 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@emjin
Copy link

emjin commented Jan 30, 2024

Thanks for this API and the backends! It's a great project. In addition to the existing collectors, it would be great to have one that uses EIO, which is nicer than Lwt.

I was also wondering if you had any advice for how best to integrate opentelemetry. In particular, I would rather use the cohttp-lwt collector over the ocurl one since it doesn't require having libcurl, but since our codebase already uses lwt in multiple places, with multiple Lwt_main.run scopes, adding a new toplevel Lwt_main.run scope is fairly annoying.

@c-cube
Copy link
Member

c-cube commented Jan 30, 2024

Hi! To run the lwt collector you don't need Lwt_main.run if it's already running. I think the with_setup is a bit useless actually though, if that's what you are referring to, but Opentelemetry_client_cohttp_lwt.setup(); … inside a running Lwt_main.run should work just fine.

@emjin
Copy link
Author

emjin commented Jan 30, 2024

My problem is that at the top level entry point where I want to instrument our code, it's possible that we call a function that would create a Lwt_main.run scope, and it's also possible we don't. My understanding is that I would have to create a Lwt_main.run scope before calling the function that I want to instrument, and then for the later ones I would pass down the Lwt monad.

@ELLIOTTCABLE
Copy link
Collaborator

Hm. An EIO collector might be on my eventual todo-list (I maintain the Lwt one), but I need to deeply test ambient-context w/ Eio first.

Unfortunately, we aren't really using OCaml 5 internally at Ahrefs yet, so I don't get to spend a lot of work-time on this 😓; it'll depend on my available free-time, tbh …

@emjin
Copy link
Author

emjin commented Jan 31, 2024

Maybe we (Semgrep) will take a stab at it in a few months (we haven't switched to OCaml 5 but we hope to). I'll let you know if we do and ask for advice 🙏

@c-cube c-cube added enhancement New feature or request help wanted Extra attention is needed labels Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants