haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.6k stars 351 forks source link

Resurrect OpenTelemetry #4306

Open JBetz opened 1 week ago

JBetz commented 1 week ago

In the effort of improving observability, we'd like to 1) see whether the opentelemetry integrations still work, 2) try turning them on by default, and 3) provide some documentation for both developers and users to actually use them. At very least, event logs could be attached to bug reports, and we may also be able to get realtime diagnostic information via Tracy.

We also might try switching from https://hackage.haskell.org/package/opentelemetry to https://hackage.haskell.org/package/hs-opentelemetry-sdk, which is more active.

JBetz commented 1 week ago

Existing documentation:

HLS records opentelemetry eventlog traces via opentelemetry. To generate the traces, build with -eventlog and run with +RTS -l. To visualize the traces, install Tracy and use eventlog-to-tracy to open the generated eventlog.

https://haskell-language-server.readthedocs.io/en/latest/contributing/contributing.html#tracing

JBetz commented 1 week ago

Okay, here's some good news:

on the commandline: warning: [-Wdeprecated-flags] -eventlog is deprecated: the eventlog is now enabled in all runtime system ways

So GHC is now turning it on for us by default, at least for 9.6.3+.

There is also a flag for the ghcide executable for memory profiling via OpenTelemetry, but AFACT isn't being used. I tried removing it and ghcide still compiled.

https://github.com/haskell/haskell-language-server/blob/e9c2f5520137e2e71c20fab39c4f4abe2cfd83ea/ghcide/exe/Arguments.hs#L37

JBetz commented 1 week ago

hs-opentelemetry-sdk does not yet support metrics (https://github.com/iand675/hs-opentelemetry/issues/51), so we'd only be able to port the tracing for now. Metrics that HLS are recording are:

https://github.com/haskell/haskell-language-server/blob/e9c2f5520137e2e71c20fab39c4f4abe2cfd83ea/ghcide/src/Development/IDE/Core/Shake.hs#L744-L749

These were also being recorded by ekg which was recently removed. I don't know how many people are using the OpenTelemetry metrics but they definitely seem worth preserving.

michaelpj commented 1 week ago

Yeah, that sounds nice. I didn't know opentelemetry supported metrics, that sounds like a good way to report these things.

michaelpj commented 1 week ago

Oh I see, we'd need to get the metrics API implemented, that's a bunch more work, boo.

mpickering commented 5 days ago

@JBetz Is there an issue with the current opentelemetry library which is motivating the migration?

JBetz commented 5 days ago

@JBetz Is there an issue with the current opentelemetry library which is motivating the migration?

@mpickering Nope, or at least not that I'm aware of. I think the idea was that it probably would have less maintenance costs, and at very least it was worth investigating if it had any features that HLS could benefit from. Also just my own curiosity in how the API compares to the current library. Turns out it's missing features we need, though, so I think the answer to whether it's worth the migration is a clear no.

JBetz commented 5 days ago

I don't have much time to work on this, but I would like to see if there's a more lightweight way to expose the metrics we already have. My original idea was to create a sort of system monitor-esque widget. Any sort of GUI would almost certainly be beyond LSP, though. Maybe there's a tooltip we could add them to?

michaelpj commented 5 days ago

I think the new opentelemetry lib seems to be more actively maintained, that was my reasoning.

@JBetz we used to use the ekg library to expose metrics, but it's not really maintained and was a pain when upgrading. It was also disabled by default and not really surfaced to users, so it's not clear how useful it was. I don't know how to do better!