steffan-westcott / clj-otel

An idiomatic Clojure API for adding telemetry to your libraries and applications using OpenTelemetry.
https://cljdoc.org/d/com.github.steffan-westcott/clj-otel-api
Apache License 2.0
183 stars 12 forks source link

Type hint return value of otel/get-text-map-propagator #14

Closed devurandom closed 7 months ago

devurandom commented 7 months ago

The return value of steffan-westcott.clj-otel.api.otel/get-text-map-propagator is most likely used in a call to (.inject propagator ,,,), where the type hint is helpful.

steffan-westcott commented 7 months ago

Thanks for the suggestion. This improvement does help prevent runtime reflection for uses of this function. The pull request lacked a needed import, so I've fixed this in the linked commit instead.

devurandom commented 7 months ago

Another type hint I needed: ^SpanContext (span/get-span-context) I assume there are more, but apart from using the function and running https://github.com/jonase/eastwood on the code I found no reliable way to determine where Clojure needs the hint. For example, (context/current) does not seem to need them, at least not in (.inject propagator (context/current) _ setter). What is your guideline for this project? Add only where proven to be needed, or add everywhere that Java objects are returned (which I assume means a lot of places)?

steffan-westcott commented 7 months ago

@devurandom Thank you for highlighting the missing typehints. Originally, I relied solely on (set! *warn-on-reflection* true) and then loading clj-otel namespaces. Your observations have shown this approach does not cater for all uses of clj-otel. I have added a commit which should cover all public clj-otel functions.