Skip to content

Commit 3b2c3c9

Browse files
authored
Merge pull request #150 from slashmo/feature/baggage-nio
Use forked NIO with embedded Baggage in ChannelHandlerContext
2 parents 0285cc4 + ee0799b commit 3b2c3c9

14 files changed

+178
-98
lines changed

Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ let package = Package(
1414
url: "https://github.com/slashmo/gsoc-swift-baggage-context.git",
1515
from: "0.5.0"
1616
),
17-
.package(url: "https://github.com/apple/swift-nio.git", from: "2.17.0"),
17+
.package(url: "https://github.com/slashmo/swift-nio.git", .branch("feature/baggage-context")),
1818
],
1919
targets: [
2020
// ==== --------------------------------------------------------------------------------------------------------

Sources/NIOInstrumentation/BaggageContextOutboundHTTPHandler.swift

Lines changed: 0 additions & 47 deletions
This file was deleted.

Sources/NIOInstrumentation/BaggageContextInboundHTTPHandler.swift renamed to Sources/NIOInstrumentation/HTTPHeadersExtractingHandler.swift

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,17 @@ import Instrumentation
1616
import NIO
1717
import NIOHTTP1
1818

19-
public final class BaggageContextInboundHTTPHandler: ChannelInboundHandler {
19+
public final class HTTPHeadersExtractingHandler: ChannelInboundHandler {
2020
public typealias InboundIn = HTTPServerRequestPart
2121
public typealias InboundOut = HTTPServerRequestPart
2222

23-
private let instrument: Instrument
24-
private var onBaggageExtracted: (Baggage) -> Void
25-
26-
public init(instrument: Instrument, onBaggage: @escaping (Baggage) -> Void) {
27-
self.instrument = instrument
28-
self.onBaggageExtracted = onBaggage
29-
}
23+
public init() {}
3024

3125
public func channelRead(context: ChannelHandlerContext, data: NIOAny) {
3226
guard case .head(let head) = unwrapInboundIn(data) else { return }
3327
var baggage = Baggage.topLevel
34-
self.instrument.extract(head.headers, into: &baggage, using: HTTPHeadersExtractor())
35-
self.onBaggageExtracted(baggage)
28+
InstrumentationSystem.instrument.extract(head.headers, into: &baggage, using: HTTPHeadersExtractor())
29+
context.baggage = baggage
30+
context.fireChannelRead(data)
3631
}
3732
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift Distributed Tracing open source project
4+
//
5+
// Copyright (c) 2020 Moritz Lang and the Swift Tracing project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
//
10+
// SPDX-License-Identifier: Apache-2.0
11+
//
12+
//===----------------------------------------------------------------------===//
13+
14+
import Baggage
15+
import Instrumentation
16+
import NIO
17+
import NIOHTTP1
18+
19+
public final class HTTPHeadersInjectingHandler: ChannelOutboundHandler {
20+
public typealias OutboundIn = HTTPClientRequestPart
21+
public typealias OutboundOut = HTTPClientRequestPart
22+
23+
public init() {}
24+
25+
public func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {
26+
let requestPart = unwrapOutboundIn(data)
27+
guard case .head(var head) = requestPart else { return }
28+
InstrumentationSystem.instrument.inject(context.baggage, into: &head.headers, using: HTTPHeadersInjector())
29+
context.write(self.wrapOutboundOut(.head(head)), promise: promise)
30+
}
31+
}

Tests/LinuxMain.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ class LinuxMainRunnerImpl: LinuxMainRunner {
3535
@available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings")
3636
func run() {
3737
XCTMain([
38-
testCase(BaggageContextInboundHTTPHandlerTests.allTests),
39-
testCase(BaggageContextOutboundHTTPHandlerTests.allTests),
4038
testCase(HTTPHeadersCarrierTests.allTests),
39+
testCase(HTTPHeadersExtractInjectTests.allTests),
40+
testCase(HTTPHeadersExtractingHandlerTests.allTests),
41+
testCase(HTTPHeadersInjectingHandlerTests.allTests),
4142
testCase(InstrumentTests.allTests),
4243
testCase(InstrumentationSystemTests.allTests),
4344
testCase(SpanAttributeSemanticsTests.allTests),

Tests/NIOInstrumentationTests/BaggageContextInboundHTTPHandlerTests+XCTest.swift renamed to Tests/NIOInstrumentationTests/HTTPHeadersExtractInjectTests+XCTest.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313
//
14-
// BaggageContextInboundHTTPHandlerTests+XCTest.swift
14+
// HTTPHeadersExtractInjectTests+XCTest.swift
1515
//
1616
import XCTest
1717
///
@@ -20,12 +20,12 @@ import XCTest
2020
/// Do NOT edit this file directly as it will be regenerated automatically when needed.
2121
///
2222

23-
extension BaggageContextInboundHTTPHandlerTests {
23+
extension HTTPHeadersExtractInjectTests {
2424

2525
@available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings")
26-
static var allTests : [(String, (BaggageContextInboundHTTPHandlerTests) -> () throws -> Void)] {
26+
static var allTests : [(String, (HTTPHeadersExtractInjectTests) -> () throws -> Void)] {
2727
return [
28-
("testForwardsHTTPHeadersToInstrumentationMiddleware", testForwardsHTTPHeadersToInstrumentationMiddleware),
28+
("test_extracted_baggage_into_subsequent_request_headers", test_extracted_baggage_into_subsequent_request_headers),
2929
]
3030
}
3131
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift Distributed Tracing open source project
4+
//
5+
// Copyright (c) 2020 Moritz Lang and the Swift Tracing project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
//
10+
// SPDX-License-Identifier: Apache-2.0
11+
//
12+
//===----------------------------------------------------------------------===//
13+
14+
@testable import Instrumentation
15+
import NIO
16+
import NIOHTTP1
17+
import NIOInstrumentation
18+
import XCTest
19+
20+
final class HTTPHeadersExtractInjectTests: XCTestCase {
21+
override class func tearDown() {
22+
super.tearDown()
23+
InstrumentationSystem.bootstrapInternal(nil)
24+
}
25+
26+
func test_extracted_baggage_into_subsequent_request_headers() throws {
27+
InstrumentationSystem.bootstrapInternal(FakeTracer())
28+
29+
let outboundHandler = HTTPHeadersInjectingHandler()
30+
let requestHandler = MockRequestHandler()
31+
let inboundHandler = HTTPHeadersExtractingHandler()
32+
33+
let channel = EmbeddedChannel(loop: EmbeddedEventLoop())
34+
XCTAssertNoThrow(try channel.pipeline.addHandlers([outboundHandler, requestHandler, inboundHandler]).wait())
35+
36+
let requestHead = HTTPRequestHead(version: .init(major: 1, minor: 1), method: .GET, uri: "/", headers: [
37+
FakeTracer.headerName: "abc",
38+
])
39+
try channel.writeInbound(HTTPServerRequestPart.head(requestHead))
40+
41+
guard case .head(let subsequentRequestHead)? = try channel.readOutbound(as: HTTPClientRequestPart.self) else {
42+
XCTFail("Expected HTTPRequestHead to be written outbound")
43+
return
44+
}
45+
XCTAssertEqual(subsequentRequestHead.headers.count, 2)
46+
XCTAssertEqual(subsequentRequestHead.headers.first(name: "Content-Type"), "application/json")
47+
XCTAssertEqual(subsequentRequestHead.headers.first(name: FakeTracer.headerName), "abc")
48+
}
49+
}
50+
51+
private final class MockRequestHandler: ChannelDuplexHandler {
52+
typealias InboundIn = HTTPServerRequestPart
53+
typealias OutboundIn = HTTPClientRequestPart
54+
typealias OutboundOut = HTTPClientRequestPart
55+
56+
func channelReadComplete(context: ChannelHandlerContext) {
57+
let requestHead = HTTPRequestHead(version: .init(major: 1, minor: 1), method: .GET, uri: "/", headers: [
58+
"Content-Type": "application/json",
59+
])
60+
context.writeAndFlush(wrapOutboundOut(.head(requestHead)), promise: nil)
61+
}
62+
}

Tests/NIOInstrumentationTests/BaggageContextOutboundHTTPHandlerTests+XCTest.swift renamed to Tests/NIOInstrumentationTests/HTTPHeadersExtractingHandlerTests+XCTest.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313
//
14-
// BaggageContextOutboundHTTPHandlerTests+XCTest.swift
14+
// HTTPHeadersExtractingHandlerTests+XCTest.swift
1515
//
1616
import XCTest
1717
///
@@ -20,12 +20,12 @@ import XCTest
2020
/// Do NOT edit this file directly as it will be regenerated automatically when needed.
2121
///
2222

23-
extension BaggageContextOutboundHTTPHandlerTests {
23+
extension HTTPHeadersExtractingHandlerTests {
2424

2525
@available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings")
26-
static var allTests : [(String, (BaggageContextOutboundHTTPHandlerTests) -> () throws -> Void)] {
26+
static var allTests : [(String, (HTTPHeadersExtractingHandlerTests) -> () throws -> Void)] {
2727
return [
28-
("testUsesInstrumentationMiddlewareToInjectHTTPHeadersFromContext", testUsesInstrumentationMiddlewareToInjectHTTPHeadersFromContext),
28+
("test_extracts_http_request_headers_into_baggage", test_extracts_http_request_headers_into_baggage),
2929
]
3030
}
3131
}

Tests/NIOInstrumentationTests/BaggageContextInboundHTTPHandlerTests.swift renamed to Tests/NIOInstrumentationTests/HTTPHeadersExtractingHandlerTests.swift

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,33 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
import Baggage
15-
import Instrumentation
15+
@testable import Instrumentation
1616
import NIO
1717
import NIOHTTP1
1818
import NIOInstrumentation
1919
import XCTest
2020

21-
final class BaggageContextInboundHTTPHandlerTests: XCTestCase {
22-
func testForwardsHTTPHeadersToInstrumentationMiddleware() throws {
23-
let traceID = "abc"
24-
let callbackExpectation = expectation(description: "Expected onBaggageExtracted to be called")
21+
final class HTTPHeadersExtractingHandlerTests: XCTestCase {
22+
override class func tearDown() {
23+
super.tearDown()
24+
InstrumentationSystem.bootstrapInternal(nil)
25+
}
2526

26-
var extractedBaggage: Baggage?
27-
let handler = BaggageContextInboundHTTPHandler(instrument: FakeTracer()) { baggage in
28-
extractedBaggage = baggage
29-
callbackExpectation.fulfill()
30-
}
27+
func test_extracts_http_request_headers_into_baggage() throws {
28+
InstrumentationSystem.bootstrapInternal(FakeTracer())
29+
30+
let traceID = "abc"
31+
let handler = HTTPHeadersExtractingHandler()
3132
let loop = EmbeddedEventLoop()
3233
let channel = EmbeddedChannel(handler: handler, loop: loop)
34+
3335
var requestHead = HTTPRequestHead(version: .init(major: 1, minor: 1), method: .GET, uri: "/")
3436
requestHead.headers = [FakeTracer.headerName: traceID]
3537

36-
try channel.writeInbound(HTTPServerRequestPart.head(requestHead))
38+
XCTAssertNil(channel._channelCore.baggage[FakeTracer.TraceIDKey.self])
3739

38-
waitForExpectations(timeout: 0.5)
40+
try channel.writeInbound(HTTPServerRequestPart.head(requestHead))
3941

40-
XCTAssertNotNil(extractedBaggage)
41-
XCTAssertEqual(extractedBaggage![FakeTracer.TraceIDKey.self], traceID)
42+
XCTAssertEqual(channel._channelCore.baggage[FakeTracer.TraceIDKey.self], traceID)
4243
}
4344
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift Distributed Tracing open source project
4+
//
5+
// Copyright (c) 2020 Moritz Lang and the Swift Tracing project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
//
10+
// SPDX-License-Identifier: Apache-2.0
11+
//
12+
//===----------------------------------------------------------------------===//
13+
//
14+
// HTTPHeadersInjectingHandlerTests+XCTest.swift
15+
//
16+
import XCTest
17+
///
18+
/// NOTE: This file was generated by generate_linux_tests.rb
19+
///
20+
/// Do NOT edit this file directly as it will be regenerated automatically when needed.
21+
///
22+
23+
extension HTTPHeadersInjectingHandlerTests {
24+
25+
@available(*, deprecated, message: "not actually deprecated. Just deprecated to allow deprecated tests (which test deprecated functionality) without warnings")
26+
static var allTests : [(String, (HTTPHeadersInjectingHandlerTests) -> () throws -> Void)] {
27+
return [
28+
("test_injects_baggage_into_http_request_headers", test_injects_baggage_into_http_request_headers),
29+
]
30+
}
31+
}
32+

0 commit comments

Comments
 (0)