Skip to content
This repository has been archived by the owner on Apr 23, 2021. It is now read-only.

Record HTTP requests made by AsyncHTTPClient #95

Open
pokryfka opened this issue Jul 24, 2020 · 2 comments · May be fixed by swift-server/async-http-client#289
Open

Record HTTP requests made by AsyncHTTPClient #95

pokryfka opened this issue Jul 24, 2020 · 2 comments · May be fixed by swift-server/async-http-client#289

Comments

@pokryfka
Copy link
Contributor

pokryfka commented Jul 24, 2020

This is related with #46 but goes one step forward in order to address #72 (comment),
not only we pass the context but also use TracingInstrument to record the request and response.

My worry is that even if/when AsyncHTTPClientAPI is extended to pass the context,
dependency on Instrumentation and NIOInstrumentation maybe to much to ask them (?)

I made a quick proof of concept implementation in pokryfka/aws-xray-sdk-swift#16

class BetterHTTPClient {
    private let client: HTTPClient

    init(eventLoopGroup: EventLoopGroup? = nil) {
        if let eventLoopGroup = eventLoopGroup {
            client = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
        } else {
            client = HTTPClient(eventLoopGroupProvider: .createNew)
        }
    }

    deinit {
        try? client.syncShutdown()
    }

    func execute(request: HTTPClient.Request, baggage: BaggageContext) -> NIO.EventLoopFuture<AsyncHTTPClient.HTTPClient.Response> {
        let tracer = InstrumentationSystem.tracer
        var span = tracer.startHTTPSpan(request: request, context: baggage)
        var request = request
        tracer.inject(span.baggage, into: &request.headers, using: HTTPHeadersInjector())
        return client.execute(request: request)
            .always { result in
                switch result {
                case .success(let response):
                    span.setHTTPAttributes(response: response)
                case .failure(let error):
                    // TODO: span does not expose a way to recoer an error at the moment, see https://github.com/slashmo/gsoc-swift-tracing/issues/90
                    span.addEvent(.init(name: "Error \(error)"))
                }
                span.end()
            }
    }
}

private extension TracingInstrument {
    func startHTTPSpan(request: HTTPClient.Request, context: BaggageContext) -> Span {
        // TODO: create name per https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md
        var span = startSpan(named: "HTTP \(request.url)", context: context)
        // TODO: preferably the attributes would be passed in ctor
        span.setHTTPAttributes(request: request)
        return span
    }
}

private extension Span {
    mutating func setHTTPAttributes(request: HTTPClient.Request) {
        setAttribute(request.method.rawValue, forKey: OpenTelemetry.SpanAttributes.HTTP.method)
        setAttribute(request.url.absoluteString, forKey: OpenTelemetry.SpanAttributes.HTTP.url)
    }

    mutating func setHTTPAttributes(response: HTTPClient.Response) {
        setAttribute(response.status.code, forKey: OpenTelemetry.SpanAttributes.HTTP.statusCode)
        setAttribute(response.body?.readableBytes ?? -1, forKey: OpenTelemetry.SpanAttributes.HTTP.responseContentLength)
    }
}

enum OpenTelemetry {
    enum SpanAttributes {
        /// [Semantic conventions for HTTP spans](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md)
        enum HTTP {
            /// HTTP request method. E.g. "GET". Required.
            static let method = "http.method"

            /// Full HTTP request URL in the form `scheme://host[:port]/path?query[#fragment]`.
            /// Usually the fragment is not transmitted over HTTP, but if it is known, it should be included nevertheless.
            static let url = "http.url"

            /// HTTP response status code. E.g. `200` (integer)
            static let statusCode = "http.status_code"

            /// The size of the response payload body in bytes.
            /// This is the number of bytes transferred excluding headers and is often, but not always, present as the `Content-Length` header.
            /// For requests using transport encoding, this should be the compressed size.
            static let responseContentLength = "http.response_content_length"
        }
    }
}
@ktoso
Copy link
Collaborator

ktoso commented Jul 24, 2020

It's not "too much to ask" wrt. instrumentation, we work closely with that team and we (the entire SSWG) want tracing to just work across all the core projects – when we say we're going to instrument the client you can trust us that it will happen 😉

Yup, that's the rough shape how an instrumentation will look like more or less 👍

@pokryfka
Copy link
Contributor Author

below output of the BetterHTTPClient (using XRayRecorder and XRayLogEmitter):

{
  "service" : {
    "version" : "aws-xray-sdk-swift"
  },
  "end_time" : 1595597689.3613839,
  "id" : "d0fae7e50034a757",
  "annotations" : {
    "http.method" : "GET",
    "http.url" : "https:\/\/aws.amazon.com",
    "http.response_content_length" : 212445,
    "http.status_code" : "200"
  },
  "start_time" : 1595597687.7261341,
  "trace_id" : "1-5759e988-bd862e3fe1be46a994272793",
  "name" : "HTTP https:\/\/aws.amazon.com"
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants