Server Distributed Tracing

Server Development
,
22

Ah! I though it was about type name clashing?

23

You're right that type name we would not clash for those (I have another example where you extend a thing and there's a typealias Context that depends on the enclosing type... that would conflict, but that's a minor issue perhaps), it is more about the passing-around sites and parameter naming.

This discussion is pretty helpful, thanks for challenging it they way you do -- that's why we're sharing the WorkInProgress of the GSoC after all :slight_smile:

I think this may have a quite interesting outcome actually, see: Reconsider composability and context naming #30 (copied below):

Goals:

  • easily be used "in" frameworks without them having to "be" a (Baggage)Context
    • e.g. frameworkContext.theMetadataContext -- is nested, not "is
  • avoid context.context
  • avoid name clashes with other contexts

What if we did

// in module Baggage
// so you'd `import Baggage`

protocol ContextCarrier { 
  var baggage: Context { get set }

  subscript<Key: ContextKey>(_ key: Key.Type) -> Key.Value? { get set }
}

struct Context { ... }

extension Context: ContextCarrier { 
  var baggage = self ... 
  ... 
}

This way other libs can:

  • carry Context without being one
  • the module name and type name don't clash:
    • module Baggage
    • and type name Context
  • implement the "accessors" (var zipkinTrace: ...) on the BaggageContextCarrier protocol
    • and it become available on Context as well as "my-framework-context" (any of them for that matter :tada: )

We do have disambiguation that it's the Baggage.Context is any other lib has a Context type and they'd clash.

For passing around:

  • libs unaware of frameworks (say HTTPClient is unaware about Vapor Request) could use func hello(who: String, context: BaggageContextCarrier)
    • this way any library can call hello(who: "me", context: myFrameworkSpecificContext)
    • this can be optional though if they dont want to
    • if they lib does not do this, users would have to pass hello(who: "me", context: myFrameworkSpecificContext.baggage) which again reads quite well
      • or if they have the "raw" context they can pass context :+1:

Let's explore this idea a bit :eyes:

2 Likes
24

Thanks for the detailed replies! Very helpful + :100:

I rescind this nit. After the explanations and related articles I agree BaggageContext is the way to go.

I agree with your reasoning here. :+1:

It does make me curious about why BaggageContext is in its own module though. If BaggageContext is more specifically for instrumentation, why not put it in the Instrumentation module?

25

@ktoso: Correct me if I’m wrong, but the reason we want the BaggageContext library as small as possible is that e.g. NIO could directly depend on it, without getting the whole Instrumentation lib. That way NIO could gain a built in way of passing BaggageContext through the channel pipeline, but instrumentation would still be done in separate handlers/libraries.

26

Yeah that was the idea so far, to keep the module as minimal as possible so there's less reason for projects to "oh but we don't want that X bit...!" complain and not adopt it.

It's a good question though, if they're so close together that it could be fine or not... Let's ticketify and see how things look and feel when we have some end to end things to look at :)

2 Likes
27

Hey everyone, here’s a small progress update from my site.

End-to-end example

We’ve now added a new example that showcases how instrumentation could possibly look like spread across multiple services. This now also lead us to start talking about how to integrate with NIO, so that the same BaggageContext can be accessed from all handlers on the same channel.

Context: Bag of random stuff or meta-data only?

In #37, we also started a discussion around whether to use the BaggageContext to store something like a shared Logger. We’re very much looking forward to everyone’s input on this.

Happy #wwdc20 :wave:

1 Like
28

I agree with this having been involved in a project that had exactly this model. Convenient magic but breaks down easily in a complex system, particularly if that system is expected to be extensible.

Typically for Java you need to enforce that that everyone uses the same Executor and that has obvious issues where a third-party library is creating asynchronous work. This might be fine for a relatively small eco-system like server-side Swift currently where we are all using NIO's event loops but as the eco-system gets more complex (hopefully) this will be less likely.

Also I can confirm that such issues are a nightmare as @ktoso mentioned and have definitely taken up more of my time that I would have liked them to have.

4 Likes
29

Couldn't agree more. Also with SwiftNIO Transport Services, SwiftNIO can actually run on top of Network.framework. In this case you'll use a different EventLoop type (NIOTSEventLoopGroup instead of MultiThreadedEventLoopGroup for NIO on BSD sockets) which executes on DispatchGroups. NIOTSEventLoopGroup gives you all the same synchronisation guarantees as MultiThreadedEventLoopGroup but the underlying thread may change (as DispatchGroups aren't bound to a thread). Long story short: Even considering only SwiftNIO you cannot universally assume that thread locals always work.

5 Likes