The package is being proposed under the Incubating maturity level.
Reviews are an important part of the Swift Server Work Group incubation process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager (via email or direct message in the Swift forums).
What goes into a review of a proposal?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, become listed on the Swift Server Ecosystem index page.
When reviewing a proposal, here are some questions to consider:
What is your evaluation of the proposal and its APIs/implementation?
Is the problem being addressed relevant for the wider Swift on Server ecosystem?
Does this proposal fit well with the feel and direction of the Swift Server ecosystem?
If you have used other libraries in Swift or other languages, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I just finished integrating tracing (with otel) in our swift services without any issues and I will push it to production very soon. Context propagation works seamlessly across a network of node.js and swift services (via amqp and http), the logging metadata system just works, and the APIs feel right at home - both for the tracing domain and for being swifty.
To me this a great achievement and a fantastic foundational package for server-side swift. Thank you for your efforts! May it attract many adopters.
Two tiny little things I noticed
As I understand it, when setting span attributes in bulk, it's either via the dictionary APIs (no SpanAttributeConvertible comfort), or individual set operations (potential repeated locking performance overhead). I noticed this hummingbird construct - maybe worth adding to the span API directly?
If I wanted to set the status of a span to error but also throw, it gets a bit weird when using withSpan as it always records the error, but a setStatus would need to be inside the block. So in the block you'd need to catch -> setStatus -> NOT record the error -> throw. Probably not worth adding anything specific for, but I remain confused about about the guidance on setting span statuses (more an otel problem than a swift problem tbh).
Thanks! I'm very glad that things are working well for you! It's great to see the just works™ including logging confirmed from actual adopters, it's something we've been shooting for with this lib
Great nitpicks as well:
or individual set operations (potential repeated locking performance overhead). I noticed this hummingbird construct - maybe worth adding to the span API directly?
yes that's right. Repeated writes potentially will cause many lock/unlock, which under no contention should not be too bad but is wasteful anyway -- I would not be opposed to including the updateAttributes actually Mind opening an issue or sending in a PR with it right away?
Yeah, good catch we've noticed this annoyance too recently. It's true tha this comes somewhat "from" the otel style of setting these... I'm supposed to figure something out about this actually but didn't have the time yet -- I'd love some other ideas and an issue about this as well if you could open one?
I've not figured it out but am wondering if this could be solved by some additional customization / hook that a tracer can implement. Then the library's withSpan would be calling that