Auto extract IDs from headers on the server #1510
Conversation
Sources/GRPC/Server.swift
Outdated
| @@ -451,6 +454,22 @@ extension Server { | |||
| } | |||
| } | |||
|
|
|||
| extension Server.Configuration { | |||
| /// Configuration for extracting IDs from metadata and inserting them into a logger. | |||
| public struct TraceIDExtractor { | |||
There was a problem hiding this comment.
Is this sufficiently flexible? Do we need to allow the user to define a type that can do arbitrary searches?
There was a problem hiding this comment.
Good question. I suspect not, I can imagine a situation where we could fallback to a different header field for example.
There was a problem hiding this comment.
Whoa wait a moment guys -- this is reinventing tracing infrastructure and types. Such extract/inject things are exactly what a Tracer (or other Instrument (a Tracer is-a Instrument)) would be doing. So rather, grpc should instrument itself using the tracing instruments, and make use of those to populate baggage, which then is fed into logging.
Here's the basic API: https://github.com/apple/swift-distributed-tracing/blob/main/Sources/Instrumentation/Instrument.swift
We can set aside a moment to talk this through this week if you want @glbrntt? It'd be really valuable to use the tracing types here; The only holdoff from 1.0 them all is projects like this having a serious look at it, as well as the swift-log discussion we're having right now about propagation, so it'd be really a waste to invent your own "extractor" types right now.
Motivation: It's common for service authors to extract a tracing ID from a request header and attach that value to the logger for an RPC. This only requires a few lines of code but must be done in each RPC. gRPC should provide configuration to do this automatically. Modifications: - Add server configuration which extracts a value from headers and attaches it to a logger at the start of each RPC. - Wire up the config. Add tests. Result: Trace IDs can be automatically extracted from headers on the server and attached to loggers.
glbrntt
force-pushed
the
gb-uuid-extraction
branch
from
5cf4d9c to
c0209df
Compare
Sources/GRPC/Server.swift
Outdated
| /// - Parameters: | ||
| /// - loggerKey: The key in the logger to set the trace ID on. | ||
| /// - extractor: A function to extract a trace ID from | ||
| public init(loggerKey: String, extractor: @escaping (HPACKHeaders) -> String?) { |
There was a problem hiding this comment.
In general I love this! I have been doing the exact same thing but needed to extract in each individual method which was kinda annoying.
I am just wondering if we should extend this a bit further to allow more things to get extracted to metadata. Something like this (HPACKHeaders) -> [String] or even (HPACKHeaders, Logger) -> Logger?
My thinking behind this is that there might be more that needs to get logged than just a trace id. Sometimes you want to log the parent id, span id, trace flags etc. https://www.w3.org/TR/trace-context/#traceparent-header
Maybe @ktoso could chime in here as well.
|
Chimed in in-line - this is exactly what swift-distributed-tracing types are for, and yeah it can be very generic -- that's what the
Precisely. And you can never know the actual set of things, which is why it must be just a blank "instrument" that people can fill in with logic -- and you can provide a simple one if you wanted to :) Shall we catch up tomorrow a bit? I'll schedule something. |
|
Cool, so I spent an hour or so and here's the same functionality implemented with the tracing APIs (as PR to this PR branch): glbrntt#2 Let's figure out how to quickly stabilize them so we don't have to do the ad-hoc implementation in here 👍 We also have the 2 year old PoC over here which even did spans: #941 so between those two branches I'm sure we can find a way to land the proper solution nicely – and get the tracing 1.0 out there ASAP so this lib can depend on it 👍 |
|
CLA signed and all other approvals done. Let's get collaboratin~ 🥳 |
| // url: "https://github.com/apple/swift-log.git", | ||
| // from: "1.4.4" | ||
| url: "https://github.com/slashmo/swift-log.git", | ||
| branch: "feature/baggage" |
There was a problem hiding this comment.
This is now merged and available in 1.5.x
|
We won't adopt tracing out-of-the-box in v1, instead this will come in v2 via interceptors. |
Motivation:
It's common for service authors to extract a tracing ID from a request header and attach that value to the logger for an RPC. This only requires a few lines of code but must be done in each RPC. gRPC should provide configuration to do this automatically.
Modifications:
Result:
Trace IDs can be automatically extracted from headers on the server and attached to loggers.