Skip to content

Commit

Permalink
Update Tracing package 📦
Browse files Browse the repository at this point in the history
  • Loading branch information
slashmo committed Oct 1, 2020
1 parent 0946749 commit 1a98075
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 54 deletions.
6 changes: 2 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,15 @@ let package = Package(
.package(url: "https://github.com/apple/swift-nio-transport-services.git", from: "1.5.1"),
.package(url: "https://github.com/apple/swift-log.git", from: "1.4.0"),
.package(url: "https://github.com/slashmo/gsoc-swift-tracing.git", .branch("main")),
.package(url: "https://github.com/slashmo/gsoc-swift-baggage-context.git", from: "0.2.0"),
],
targets: [
.target(
name: "AsyncHTTPClient",
dependencies: ["NIO", "NIOHTTP1", "NIOSSL", "NIOConcurrencyHelpers", "NIOHTTPCompression",
"NIOFoundationCompat", "NIOTransportServices", "Logging",
.product(name: "TracingInstrumentation", package: "gsoc-swift-tracing"),
.product(name: "Tracing", package: "gsoc-swift-tracing"),
.product(name: "OpenTelemetryInstrumentationSupport", package: "gsoc-swift-tracing"),
.product(name: "NIOInstrumentation", package: "gsoc-swift-tracing"),
.product(name: "BaggageLogging", package: "swift-baggage-context")]
.product(name: "NIOInstrumentation", package: "gsoc-swift-tracing")]
),
.testTarget(
name: "AsyncHTTPClientTests",
Expand Down
33 changes: 16 additions & 17 deletions Sources/AsyncHTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
//
//===----------------------------------------------------------------------===//

import Baggage
import BaggageLogging
import BaggageContext
import Foundation
import Instrumentation
import TracingInstrumentation
import Tracing
import Logging
import NIO
import NIOConcurrencyHelpers
Expand Down Expand Up @@ -233,7 +232,7 @@ public class HTTPClient {
/// - url: Remote URL.
/// - context: Baggage context associated with this request
/// - deadline: Point in time by which the request must complete.
public func get(url: String, context: LoggingBaggageContextCarrier, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func get(url: String, context: BaggageContext, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
return self.execute(.GET, url: url, context: context, deadline: deadline)
}

Expand All @@ -244,7 +243,7 @@ public class HTTPClient {
/// - context: Baggage context associated with this request
/// - body: Request body.
/// - deadline: Point in time by which the request must complete.
public func post(url: String, context: LoggingBaggageContextCarrier, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func post(url: String, context: BaggageContext, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
return self.execute(.POST, url: url, context: context, body: body, deadline: deadline)
}

Expand All @@ -255,7 +254,7 @@ public class HTTPClient {
/// - context: Baggage context associated with this request
/// - body: Request body.
/// - deadline: Point in time by which the request must complete.
public func patch(url: String, context: LoggingBaggageContextCarrier, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func patch(url: String, context: BaggageContext, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
return self.execute(.PATCH, url: url, context: context, body: body, deadline: deadline)
}

Expand All @@ -266,7 +265,7 @@ public class HTTPClient {
/// - context: Baggage context associated with this request
/// - body: Request body.
/// - deadline: Point in time by which the request must complete.
public func put(url: String, context: LoggingBaggageContextCarrier, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func put(url: String, context: BaggageContext, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
return self.execute(.PUT, url: url, context: context, body: body, deadline: deadline)
}

Expand All @@ -276,7 +275,7 @@ public class HTTPClient {
/// - url: Remote URL.
/// - context: Baggage context associated with this request
/// - deadline: The time when the request must have been completed by.
public func delete(url: String, context: LoggingBaggageContextCarrier, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func delete(url: String, context: BaggageContext, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
return self.execute(.DELETE, url: url, context: context, deadline: deadline)
}

Expand All @@ -288,7 +287,7 @@ public class HTTPClient {
/// - context: Baggage context associated with this request
/// - body: Request body.
/// - deadline: Point in time by which the request must complete.
public func execute(_ method: HTTPMethod = .GET, url: String, context: LoggingBaggageContextCarrier, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func execute(_ method: HTTPMethod = .GET, url: String, context: BaggageContext, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
do {
let request = try Request(url: url, method: method, body: body)
return self.execute(request: request, context: context, deadline: deadline)
Expand All @@ -306,7 +305,7 @@ public class HTTPClient {
/// - context: Baggage context associated with this request
/// - body: Request body.
/// - deadline: Point in time by which the request must complete.
public func execute(_ method: HTTPMethod = .GET, socketPath: String, urlPath: String, context: LoggingBaggageContextCarrier, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func execute(_ method: HTTPMethod = .GET, socketPath: String, urlPath: String, context: BaggageContext, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
do {
guard let url = URL(httpURLWithSocketPath: socketPath, uri: urlPath) else {
throw HTTPClientError.invalidURL
Expand All @@ -328,7 +327,7 @@ public class HTTPClient {
/// - body: Request body.
/// - deadline: Point in time by which the request must complete.
/// - logger: The logger to use for this request.
public func execute(_ method: HTTPMethod = .GET, secureSocketPath: String, urlPath: String, context: LoggingBaggageContextCarrier, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func execute(_ method: HTTPMethod = .GET, secureSocketPath: String, urlPath: String, context: BaggageContext, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
do {
guard let url = URL(httpsURLWithSocketPath: secureSocketPath, uri: urlPath) else {
throw HTTPClientError.invalidURL
Expand All @@ -346,7 +345,7 @@ public class HTTPClient {
/// - request: HTTP request to execute.
/// - context: Baggage context associated with this request
/// - deadline: Point in time by which the request must complete.
public func execute(request: Request, context: LoggingBaggageContextCarrier, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func execute(request: Request, context: BaggageContext, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
let accumulator = ResponseAccumulator(request: request)
return self.execute(request: request, delegate: accumulator, context: context, deadline: deadline).futureResult
}
Expand All @@ -358,7 +357,7 @@ public class HTTPClient {
/// - eventLoop: NIO Event Loop preference.
/// - context: Baggage context associated with this request
/// - deadline: Point in time by which the request must complete.
public func execute(request: Request, eventLoop: EventLoopPreference, context: LoggingBaggageContextCarrier, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
public func execute(request: Request, eventLoop: EventLoopPreference, context: BaggageContext, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
let accumulator = ResponseAccumulator(request: request)
return self.execute(request: request, delegate: accumulator, eventLoop: eventLoop, context: context, deadline: deadline).futureResult
}
Expand All @@ -372,7 +371,7 @@ public class HTTPClient {
/// - deadline: Point in time by which the request must complete.
public func execute<Delegate: HTTPClientResponseDelegate>(request: Request,
delegate: Delegate,
context: LoggingBaggageContextCarrier,
context: BaggageContext,
deadline: NIODeadline? = nil) -> Task<Delegate.Response> {
return self.execute(request: request, delegate: delegate, eventLoop: .indifferent, context: context, deadline: deadline)
}
Expand All @@ -388,9 +387,9 @@ public class HTTPClient {
public func execute<Delegate: HTTPClientResponseDelegate>(request: Request,
delegate: Delegate,
eventLoop eventLoopPreference: EventLoopPreference,
context: LoggingBaggageContextCarrier,
context: BaggageContext,
deadline: NIODeadline? = nil) -> Task<Delegate.Response> {
var span = InstrumentationSystem.tracingInstrument.startSpan(named: request.method.rawValue, context: context, ofKind: .client)
let span = InstrumentationSystem.tracer.startSpan(named: request.method.rawValue, baggage: context.baggage, ofKind: .client)
span.attributes.http.method = request.method.rawValue
span.attributes.http.scheme = request.scheme
span.attributes.http.target = request.uri
Expand All @@ -402,7 +401,7 @@ public class HTTPClient {
// TODO: net.peer.ip / Not required, but recommended

var request = request
InstrumentationSystem.instrument.inject(span.context.baggage, into: &request.headers, using: HTTPHeadersInjector())
InstrumentationSystem.instrument.inject(span.baggage, into: &request.headers, using: HTTPHeadersInjector())

let logger = context.logger.attachingRequestInformation(request, requestID: globalRequestID.add(1))

Expand Down
2 changes: 1 addition & 1 deletion Sources/AsyncHTTPClient/Utils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import NIOHTTP1
import NIOHTTPCompression
import NIOSSL
import NIOTransportServices
import TracingInstrumentation
import Tracing

internal extension String {
var isIPAddress: Bool {
Expand Down
15 changes: 3 additions & 12 deletions Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
//===----------------------------------------------------------------------===//

@testable import AsyncHTTPClient
import Baggage
import BaggageLogging
import BaggageContext
import Logging
import NIO
import NIOConcurrencyHelpers
Expand Down Expand Up @@ -1177,14 +1176,6 @@ extension TaskHandler.State {
}
}

private struct TestContext: LoggingBaggageContextCarrier {
var logger: Logger
var baggage: BaggageContext
}

func testContext(
_ context: BaggageContext = BaggageContext(),
logger: Logger = Logger(label: "test")
) -> LoggingBaggageContextCarrier {
TestContext(logger: logger.with(context: context), baggage: context)
func testContext(_ baggage: Baggage = .topLevel, logger: Logger = Logger(label: "test")) -> BaggageContext {
DefaultContext(baggage: baggage, logger: logger)
}
35 changes: 15 additions & 20 deletions Tests/AsyncHTTPClientTests/HTTPClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#endif
import Baggage
import Instrumentation
import TracingInstrumentation
import Tracing
import Logging
import NIO
import NIOConcurrencyHelpers
Expand Down Expand Up @@ -501,7 +501,8 @@ class HTTPClientTests: XCTestCase {

let progress = try self.defaultClient.execute(
request: request,
delegate: delegate
delegate: delegate,
context: testContext()
)
.wait()

Expand All @@ -526,7 +527,8 @@ class HTTPClientTests: XCTestCase {

let progress = try self.defaultClient.execute(
request: request,
delegate: delegate
delegate: delegate,
context: testContext()
)
.wait()

Expand Down Expand Up @@ -2664,7 +2666,7 @@ class HTTPClientTests: XCTestCase {
func testDoubleError() throws {
// This is needed to that connection pool will not get into closed state when we release
// second connection.
_ = self.defaultClient.get(url: "http://localhost:\(self.defaultHTTPBin.port)/events/10/1")
_ = self.defaultClient.get(url: "http://localhost:\(self.defaultHTTPBin.port)/events/10/1", context: testContext())

var request = try HTTPClient.Request(url: "http://localhost:\(self.defaultHTTPBin.port)/wait", method: .POST)
request.body = .stream { writer in
Expand All @@ -2683,7 +2685,7 @@ class HTTPClientTests: XCTestCase {

// We specify a deadline of 2 ms co that request will be timed out before all chunks are writtent,
// we need to verify that second error on write after timeout does not lead to double-release.
XCTAssertThrowsError(try self.defaultClient.execute(request: request, deadline: .now() + .milliseconds(2)).wait())
XCTAssertThrowsError(try self.defaultClient.execute(request: request, context: testContext(), deadline: .now() + .milliseconds(2)).wait())
}

// MARK: - Tracing -
Expand All @@ -2708,19 +2710,16 @@ class HTTPClientTests: XCTestCase {
}
}

private final class TestTracer: TracingInstrument {
private final class TestTracer: Tracer {
private(set) var recordedSpans = [TestSpan]()

func startSpan(
named operationName: String,
context: BaggageContextCarrier,
baggage: Baggage,
ofKind kind: SpanKind,
at timestamp: Timestamp
) -> Span {
let span = TestSpan(operationName: operationName,
kind: kind,
startTimestamp: timestamp,
context: context.baggage)
let span = TestSpan(operationName: operationName, kind: kind, startTimestamp: timestamp, baggage: baggage)
recordedSpans.append(span)
return span
}
Expand All @@ -2729,26 +2728,22 @@ private final class TestTracer: TracingInstrument {

func extract<Carrier, Extractor>(
_ carrier: Carrier,
into context: inout BaggageContext,
into baggage: inout Baggage,
using extractor: Extractor
)
where
Carrier == Extractor.Carrier,
Extractor: ExtractorProtocol {}

func inject<Carrier, Injector>(
_ context: BaggageContext,
into carrier: inout Carrier,
using injector: Injector
)
func inject<Carrier, Injector>(_ baggage: Baggage, into carrier: inout Carrier, using injector: Injector)
where
Carrier == Injector.Carrier,
Injector: InjectorProtocol {}

final class TestSpan: Span {
private let operationName: String
private let kind: SpanKind
var context: BaggageContext
let baggage: Baggage

private(set) var status: SpanStatus?
private(set) var isRecording = false
Expand All @@ -2772,11 +2767,11 @@ private final class TestTracer: TracingInstrument {
self.status = status
}

init(operationName: String, kind: SpanKind, startTimestamp: Timestamp, context: BaggageContext) {
init(operationName: String, kind: SpanKind, startTimestamp: Timestamp, baggage: Baggage) {
self.operationName = operationName
self.kind = kind
self.startTimestamp = startTimestamp
self.context = context
self.baggage = baggage
}
}
}

0 comments on commit 1a98075

Please sign in to comment.