Skip to content

Commit

Permalink
Merge branch 'main' into mdalmamun/add-metrics-to-StableOtlpHTTPMetri…
Browse files Browse the repository at this point in the history
…cExporter
  • Loading branch information
nachoBonafonte authored Nov 22, 2024
2 parents 8f1c189 + 9332fec commit 13fa8c8
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 35 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/BuildAndTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ on: [push, pull_request, workflow_dispatch]

jobs:
macOS:
runs-on: macos-14
runs-on: macos-15
steps:
- uses: actions/checkout@v2
- name: Build and Test for macOS
Expand All @@ -16,7 +16,7 @@ jobs:
xcrun llvm-cov export -ignore-filename-regex="pb\.swift|grpc\.swift" -format="lcov" .build/debug/opentelemetry-swiftPackageTests.xctest/Contents/MacOS/opentelemetry-swiftPackageTests -instr-profile .build/debug/codecov/default.profdata > .build/debug/codecov/coverage_report.lcov
./codecov -f .build/debug/codecov/coverage_report.lcov
iOS:
runs-on: macos-14
runs-on: macos-15
steps:
- uses: actions/checkout@v2
- name: Install Homebrew kegs
Expand All @@ -26,7 +26,7 @@ jobs:
- name: Test for iOS
run: make test-without-building-ios
tvOS:
runs-on: macos-14
runs-on: macos-15
steps:
- uses: actions/checkout@v2
- name: Install Homebrew kegs
Expand All @@ -36,7 +36,7 @@ jobs:
- name: Test for tvOS
run: make test-without-building-tvos
watchOS:
runs-on: macos-14
runs-on: macos-15
steps:
- uses: actions/checkout@v2
- name: Install Homebrew kegs
Expand All @@ -53,4 +53,4 @@ jobs:
- name: Build tests for Linux
run: swift build --build-tests
- name: Run tests for Linux
run: swift test
run: swift test
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ PROJECT_NAME="opentelemetry-swift-Package"

XCODEBUILD_OPTIONS_IOS=\
-configuration Debug \
-destination platform='iOS Simulator,name=iPhone 15,OS=latest' \
-destination platform='iOS Simulator,name=iPhone 16,OS=18.2' \
-scheme $(PROJECT_NAME) \
-test-iterations 5 \
-retry-tests-on-failure \
Expand All @@ -18,7 +18,7 @@ XCODEBUILD_OPTIONS_TVOS=\

XCODEBUILD_OPTIONS_WATCHOS=\
-configuration Debug \
-destination platform='watchOS Simulator,name=Apple Watch Series 8 (45mm),OS=latest' \
-destination platform='watchOS Simulator,name=Apple Watch Series 10 (46mm),OS=11.2' \
-scheme $(PROJECT_NAME) \
-test-iterations 5 \
-retry-tests-on-failure \
Expand Down Expand Up @@ -50,7 +50,7 @@ build-for-testing-tvos:

.PHONY: build-for-testing-watchos
build-for-testing-watchos:
set -o pipefail && xcodebuild $(XCODEBUILD_OPTIONS_WATCHOS) build-for-testing | xcbeautify
set -o pipefail && xcodebuild OTHER_LDFLAGS="$(OTHER_LDFLAGS) -fprofile-instr-generate" $(XCODEBUILD_OPTIONS_WATCHOS) build-for-testing | xcbeautify

.PHONY: test-ios
test-ios:
Expand Down
3 changes: 2 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ let package = Package(
path: "Sources/Exporters/OpenTelemetryProtocolCommon"),
.target(name: "OpenTelemetryProtocolExporterHttp",
dependencies: ["OpenTelemetrySdk",
"OpenTelemetryProtocolExporterCommon"],
"OpenTelemetryProtocolExporterCommon",
"DataCompression"],
path: "Sources/Exporters/OpenTelemetryProtocolHttp"),
.target(name: "OpenTelemetryProtocolExporterGrpc",
dependencies: ["OpenTelemetrySdk",
Expand Down
3 changes: 2 additions & 1 deletion [email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ let package = Package(
path: "Sources/Exporters/OpenTelemetryProtocolCommon"),
.target(name: "OpenTelemetryProtocolExporterHttp",
dependencies: ["OpenTelemetrySdk",
"OpenTelemetryProtocolExporterCommon"],
"OpenTelemetryProtocolExporterCommon",
"DataCompression"],
path: "Sources/Exporters/OpenTelemetryProtocolHttp"),
.target(name: "OpenTelemetryProtocolExporterGrpc",
dependencies: ["OpenTelemetrySdk",
Expand Down
3 changes: 2 additions & 1 deletion [email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ let package = Package(
path: "Sources/Exporters/OpenTelemetryProtocolCommon"),
.target(name: "OpenTelemetryProtocolExporterHttp",
dependencies: ["OpenTelemetrySdk",
"OpenTelemetryProtocolExporterCommon"],
"OpenTelemetryProtocolExporterCommon",
"DataCompression"],
path: "Sources/Exporters/OpenTelemetryProtocolHttp"),
.target(name: "OpenTelemetryProtocolExporterGrpc",
dependencies: ["OpenTelemetrySdk",
Expand Down
30 changes: 16 additions & 14 deletions Sources/Instrumentation/URLSession/URLSessionInstrumentation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,21 @@ struct NetworkRequestState {
private var idKey: Void?

public class URLSessionInstrumentation {
private var requestMap = [String: NetworkRequestState]()

var configuration: URLSessionInstrumentationConfiguration

private let queue = DispatchQueue(label: "io.opentelemetry.ddnetworkinstrumentation")

static var instrumentedKey = "io.opentelemetry.instrumentedCall"

static let avAssetDownloadTask: AnyClass? = NSClassFromString("__NSCFBackgroundAVAssetDownloadTask")

private var requestMap = [String: NetworkRequestState]()

var configuration: URLSessionInstrumentationConfiguration

private let queue = DispatchQueue(label: "io.opentelemetry.ddnetworkinstrumentation")

static var instrumentedKey = "io.opentelemetry.instrumentedCall"

static let AVTaskClassList : [AnyClass] = {
["__NSCFBackgroundAVAggregateAssetDownloadTask",
"__NSCFBackgroundAVAssetDownloadTask",
"__NSCFBackgroundAVAggregateAssetDownloadTaskNoChildTask" ]
.compactMap { NSClassFromString($0) }
}()

public private(set) var tracer: Tracer

public var startedRequestSpans: [Span] {
Expand Down Expand Up @@ -592,10 +597,7 @@ public class URLSessionInstrumentation {

private func urlSessionTaskWillResume(_ task: URLSessionTask) {
// AV Asset Tasks cannot be auto instrumented, they dont include request attributes, skip them
if let avAssetTaskClass = Self.avAssetDownloadTask,
task.isKind(of: avAssetTaskClass) {
return
}
guard !Self.AVTaskClassList.contains(where: {task.isKind(of:$0)}) else { return }

// We cannot instrument async background tasks because they crash if you assign a delegate
if #available(OSX 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public struct URLSessionInstrumentationConfiguration {
createdRequest: ((URLRequest, Span) -> Void)? = nil,
receivedResponse: ((URLResponse, DataOrFile?, Span) -> Void)? = nil,
receivedError: ((Error, DataOrFile?, HTTPStatus, Span) -> Void)? = nil,
delegateClassesToInstrument: [AnyClass]? = nil)
delegateClassesToInstrument: [AnyClass]? = nil,
defaultBaggageProvider: (() -> (Baggage)?)? = nil)
{
self.shouldRecordPayload = shouldRecordPayload
self.shouldInstrument = shouldInstrument
Expand All @@ -36,6 +37,7 @@ public struct URLSessionInstrumentationConfiguration {
self.receivedResponse = receivedResponse
self.receivedError = receivedError
self.delegateClassesToInstrument = delegateClassesToInstrument
self.defaultBaggageProvider = defaultBaggageProvider
}

// Instrumentation Callbacks
Expand Down Expand Up @@ -73,4 +75,14 @@ public struct URLSessionInstrumentationConfiguration {

/// The array of URLSession delegate classes that will be instrumented by the library, will autodetect if nil is passed.
public var delegateClassesToInstrument: [AnyClass]?

/// Implement this callback to provide a default baggage instance for all instrumented requests.
/// The provided baggage is merged with active baggage (if any) to create a combined baggage.
/// The combined baggage is then injected into request headers using the configured `TextMapBaggagePropagator`.
/// This allows consistent propagation across requests, regardless of the active context.
///
/// Note: The injected baggage depends on the propagator in use (e.g., W3C or custom).
/// Returns: A `Baggage` instance or `nil` if no default baggage is needed.
public let defaultBaggageProvider: (() -> (Baggage)?)?

}
25 changes: 21 additions & 4 deletions Sources/Instrumentation/URLSession/URLSessionLogger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,22 @@ class URLSessionLogger {
var instrumentedRequest = request
objc_setAssociatedObject(instrumentedRequest, URLSessionInstrumentation.instrumentedKey, true, .OBJC_ASSOCIATION_COPY_NONATOMIC)
let propagators = OpenTelemetry.instance.propagators
var traceHeaders = tracePropagationHTTPHeaders(span: span, textMapPropagator: propagators.textMapPropagator, textMapBaggagePropagator: propagators.textMapBaggagePropagator)

let defaultBaggage = instrumentation.configuration.defaultBaggageProvider?()

var traceHeaders = tracePropagationHTTPHeaders(span: span,
defaultBaggage: defaultBaggage,
textMapPropagator: propagators.textMapPropagator,
textMapBaggagePropagator: propagators.textMapBaggagePropagator)

if let originalHeaders = request.allHTTPHeaderFields {
traceHeaders.merge(originalHeaders) { _, new in new }
}
instrumentedRequest.allHTTPHeaderFields = traceHeaders
return instrumentedRequest
}

private static func tracePropagationHTTPHeaders(span: Span?, textMapPropagator: TextMapPropagator, textMapBaggagePropagator: TextMapBaggagePropagator) -> [String: String] {
private static func tracePropagationHTTPHeaders(span: Span?, defaultBaggage: Baggage?, textMapPropagator: TextMapPropagator, textMapBaggagePropagator: TextMapBaggagePropagator) -> [String: String] {
var headers = [String: String]()

struct HeaderSetter: Setter {
Expand All @@ -182,9 +189,19 @@ class URLSessionLogger {
}
textMapPropagator.inject(spanContext: currentSpan.context, carrier: &headers, setter: HeaderSetter())

if let baggage = OpenTelemetry.instance.contextProvider.activeBaggage {
textMapBaggagePropagator.inject(baggage: baggage, carrier: &headers, setter: HeaderSetter())
let baggageBuilder = OpenTelemetry.instance.baggageManager.baggageBuilder()

if let activeBaggage = OpenTelemetry.instance.contextProvider.activeBaggage {
activeBaggage.getEntries().forEach { baggageBuilder.put(key: $0.key, value: $0.value, metadata: $0.metadata) }
}

if let defaultBaggage {
defaultBaggage.getEntries().forEach { baggageBuilder.put(key: $0.key, value: $0.value, metadata: $0.metadata) }
}

let combinedBaggage = baggageBuilder.build()
textMapBaggagePropagator.inject(baggage: combinedBaggage, carrier: &headers, setter: HeaderSetter())

return headers
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ class URLSessionInstrumentationTests: XCTestCase {
static var requestCopy: URLRequest!
static var responseCopy: HTTPURLResponse!

static var activeBaggage: Baggage!
static var defaultBaggage: Baggage!

static var config = URLSessionInstrumentationConfiguration(shouldRecordPayload: nil,
shouldInstrument: { req in
checker.shouldInstrumentCalled = true
Expand Down Expand Up @@ -76,25 +79,31 @@ class URLSessionInstrumentationTests: XCTestCase {
},
receivedError: { _, _, _, _ in
URLSessionInstrumentationTests.checker.receivedErrorCalled = true
},
defaultBaggageProvider: {
defaultBaggage
})

static var checker = Check()
static var semaphore: DispatchSemaphore!
var sessionDelegate: SessionDelegate!
static var instrumentation: URLSessionInstrumentation!
static var baggage: Baggage!

static let server = HttpTestServer(url: URL(string: "http://localhost:33333"), config: nil)

override class func setUp() {
OpenTelemetry.registerPropagators(textPropagators: [W3CTraceContextPropagator()], baggagePropagator: W3CBaggagePropagator())
OpenTelemetry.registerTracerProvider(tracerProvider: TracerProviderSdk())

baggage = DefaultBaggageManager.instance.baggageBuilder()
defaultBaggage = DefaultBaggageManager.instance.baggageBuilder()
.put(key: EntryKey(name: "bar")!, value: EntryValue(string: "baz")!, metadata: nil)
.build()

activeBaggage = DefaultBaggageManager.instance.baggageBuilder()
.put(key: EntryKey(name: "foo")!, value: EntryValue(string: "bar")!, metadata: nil)
.build()

OpenTelemetry.instance.contextProvider.setActiveBaggage(baggage)
OpenTelemetry.instance.contextProvider.setActiveBaggage(activeBaggage)

let sem = DispatchSemaphore(value: 0)
DispatchQueue.global(qos: .default).async {
Expand All @@ -111,7 +120,8 @@ class URLSessionInstrumentationTests: XCTestCase {

override class func tearDown() {
server.stop()
OpenTelemetry.instance.contextProvider.removeContextForBaggage(baggage)
defaultBaggage = nil
OpenTelemetry.instance.contextProvider.removeContextForBaggage(activeBaggage)
}

override func setUp() {
Expand Down Expand Up @@ -262,13 +272,63 @@ class URLSessionInstrumentationTests: XCTestCase {

XCTAssertTrue(URLSessionInstrumentationTests.checker.shouldInstrumentCalled)

XCTAssertEqual(1, URLSessionLogger.runningSpans.count)

let span = try XCTUnwrap(URLSessionLogger.runningSpans["111"])
XCTAssertEqual("HTTP GET", span.name)
}

public func testShouldInstrumentRequest_PropagateCombinedActiveAndDefaultBaggages() throws {
let request1 = URLRequest(url: URL(string: "http://defaultName.com")!)
let request2 = URLRequest(url: URL(string: "http://dontinstrument.com")!)

let processedRequest1 = try XCTUnwrap(URLSessionLogger.processAndLogRequest(request1, sessionTaskId: "111", instrumentation: URLSessionInstrumentationTests.instrumentation, shouldInjectHeaders: true))
let processedRequest2 = URLSessionLogger.processAndLogRequest(request2, sessionTaskId: "222", instrumentation: URLSessionInstrumentationTests.instrumentation, shouldInjectHeaders: true)

// `processedRequest2` is expected to be nil, because its URL was marked as not to be instrumented.
XCTAssertNil(processedRequest2)

XCTAssertTrue(URLSessionInstrumentationTests.checker.shouldInstrumentCalled)

let processedHeaders1 = try XCTUnwrap(processedRequest1.allHTTPHeaderFields)

// headers injected from `TextMapPropagator` implementation
XCTAssertTrue(processedHeaders1.contains(where: { $0.key == W3CTraceContextPropagator.traceparent }))

// headers injected from `TextMapBaggagePropagator` implementation
let baggageHeaderValue = try XCTUnwrap(processedHeaders1[W3CBaggagePropagator.headerBaggage])

// foo=bar propagated through active baggage defined in `setUp`
XCTAssertTrue(baggageHeaderValue.contains("foo=bar"))

// bar=baz propagated through default baggage provided in `URLSessionInstrumentationConfiguration`
XCTAssertTrue(baggageHeaderValue.contains("bar=baz"))

XCTAssertEqual(1, URLSessionLogger.runningSpans.count)

let span = try XCTUnwrap(URLSessionLogger.runningSpans["111"])
XCTAssertEqual("HTTP GET", span.name)
}

public func testShouldInstrumentRequest_PropagateOnlyActiveBaggage() throws {
Self.defaultBaggage = nil

let request1 = URLRequest(url: URL(string: "http://defaultName.com")!)

let processedRequest1 = try XCTUnwrap(URLSessionLogger.processAndLogRequest(request1, sessionTaskId: "111", instrumentation: URLSessionInstrumentationTests.instrumentation, shouldInjectHeaders: true))

XCTAssertTrue(URLSessionInstrumentationTests.checker.shouldInstrumentCalled)

let processedHeaders1 = try XCTUnwrap(processedRequest1.allHTTPHeaderFields)

// headers injected from `TextMapPropagator` implementation
XCTAssertTrue(processedHeaders1.contains(where: { $0.key == W3CTraceContextPropagator.traceparent }))

// headers injected from `TextMapBaggagePropagator` implementation
XCTAssertTrue(processedHeaders1.contains(where: { $0.key == W3CBaggagePropagator.headerBaggage && $0.value == "foo=bar" }))
let baggageHeaderValue = try XCTUnwrap(processedHeaders1[W3CBaggagePropagator.headerBaggage])

// bar=baz propagated through default baggage provided in `URLSessionInstrumentationConfiguration`
XCTAssertEqual(baggageHeaderValue, "foo=bar")

XCTAssertEqual(1, URLSessionLogger.runningSpans.count)

Expand Down

0 comments on commit 13fa8c8

Please sign in to comment.