Merge pull request open-telemetry#459 from open-telemetry/batch-timeo…

…ut-fix grpc call options were not using a timeout.
nachoBonafonte · Sep 19, 2023 · 621d039 · 621d039
2 parents ec42fcd + bf5f4ec
commit 621d039
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 35 deletions.
diff --git a/Package.swift b/Package.swift
@@ -34,14 +34,14 @@ let package = Package(
         .executable(name: "loggingTracer", targets: ["LoggingTracer"]),
     ],
     dependencies: [
-        .package(name: "Opentracing", url: "https://github.com/undefinedlabs/opentracing-objc", from: "0.5.2"),
-        .package(name: "Thrift", url: "https://github.com/undefinedlabs/Thrift-Swift", from: "1.1.1"),
-        .package(name: "swift-nio", url: "https://github.com/apple/swift-nio.git", from: "2.0.0"),
-        .package(name: "grpc-swift", url: "https://github.com/grpc/grpc-swift.git", from: "1.0.0"),
-        .package(name: "swift-protobuf", url: "https://github.com/apple/swift-protobuf.git", from: "1.20.2"),
-        .package(name: "swift-log", url: "https://github.com/apple/swift-log.git", from: "1.4.4"),
-        .package(name: "swift-metrics", url: "https://github.com/apple/swift-metrics.git", from: "2.1.1"),
-        .package(name: "Reachability.swift", url: "https://github.com/ashleymills/Reachability.swift", from: "5.1.0")
+        .package(name: "Opentracing", url: "https://github.com/undefinedlabs/opentracing-objc", exact: "0.5.2"),
+        .package(name: "Thrift", url: "https://github.com/undefinedlabs/Thrift-Swift", exact: "1.1.1"),
+        .package(name: "swift-nio", url: "https://github.com/apple/swift-nio.git", exact: "2.0.0"),
+        .package(name: "grpc-swift", url: "https://github.com/grpc/grpc-swift.git", exact: "1.0.0"),
+        .package(name: "swift-protobuf", url: "https://github.com/apple/swift-protobuf.git", exact: "1.20.2"),
+        .package(name: "swift-log", url: "https://github.com/apple/swift-log.git", exact: "1.4.4"),
+        .package(name: "swift-metrics", url: "https://github.com/apple/swift-metrics.git", exact: "2.1.1"),
+        .package(name: "Reachability.swift", url: "https://github.com/ashleymills/Reachability.swift", exact: "5.1.0")
     ],
     targets: [
         .target(name: "OpenTelemetryApi",

diff --git a/Sources/Exporters/OpenTelemetryProtocolGrpc/logs/OtlpLogExporter.swift b/Sources/Exporters/OpenTelemetryProtocolGrpc/logs/OtlpLogExporter.swift
@@ -16,7 +16,7 @@ public class OtlpLogExporter : LogRecordExporter {
     let channel : GRPCChannel
     var logClient : Opentelemetry_Proto_Collector_Logs_V1_LogsServiceNIOClient
     let config : OtlpConfiguration
-    var callOptions : CallOptions? = nil
+    var callOptions : CallOptions
 
     public init(channel: GRPCChannel,
                 config: OtlpConfiguration = OtlpConfiguration(),
@@ -46,6 +46,12 @@ public class OtlpLogExporter : LogRecordExporter {
         let logRequest = Opentelemetry_Proto_Collector_Logs_V1_ExportLogsServiceRequest.with { request in
             request.resourceLogs = LogRecordAdapter.toProtoResourceRecordLog(logRecordList: logRecords)
         }
+
+        if config.timeout > 0 {
+          callOptions.timeLimit = TimeLimit.timeout(TimeAmount.nanoseconds(Int64(config.timeout.toNanoseconds)))
+        }
+
+
         let export = logClient.export(logRequest, callOptions: callOptions)
         do {
             _ = try export.response.wait()

diff --git a/Sources/Exporters/OpenTelemetryProtocolGrpc/trace/OtlpTraceExporter.swift b/Sources/Exporters/OpenTelemetryProtocolGrpc/trace/OtlpTraceExporter.swift
@@ -16,7 +16,7 @@ public class OtlpTraceExporter: SpanExporter {
     let channel: GRPCChannel
     var traceClient: Opentelemetry_Proto_Collector_Trace_V1_TraceServiceNIOClient
     let config : OtlpConfiguration
-    var callOptions : CallOptions? = nil
+    var callOptions : CallOptions
 
     public init(channel: GRPCChannel, config: OtlpConfiguration = OtlpConfiguration(), logger: Logging.Logger = Logging.Logger(label: "io.grpc", factory: { _ in SwiftLogNoOpLogHandler() }), envVarHeaders: [(String,String)]? = EnvVarHeaders.attributes) {
         self.channel = channel
@@ -44,7 +44,7 @@ public class OtlpTraceExporter: SpanExporter {
         }
 
         if config.timeout > 0 {
-            traceClient.defaultCallOptions.timeLimit = TimeLimit.timeout(TimeAmount.nanoseconds(Int64(config.timeout.toNanoseconds)))
+           callOptions.timeLimit = TimeLimit.timeout(TimeAmount.nanoseconds(Int64(config.timeout.toNanoseconds)))
         }
 
         let export = traceClient.export(exportRequest, callOptions: callOptions)

diff --git a/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift b/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift
@@ -134,7 +134,9 @@ private class BatchWorker: Thread {
       self?.exportAction(spanList: spanList)
     }
     let timeoutTimer = DispatchSource.makeTimerSource(queue: DispatchQueue.global())
-    timeoutTimer.setEventHandler { exportOperation.cancel() }
+    timeoutTimer.setEventHandler {
+      exportOperation.cancel()
+    }
     let maxTimeOut = min(explicitTimeout ?? TimeInterval.greatestFiniteMagnitude, exportTimeout)
     timeoutTimer.schedule(deadline: .now() + .milliseconds(Int(maxTimeOut.toMilliseconds)), leeway: .milliseconds(1))
     timeoutTimer.activate()

diff --git a/Tests/ExportersTests/OpenTelemetryProtocol/OtlpLogRecordExporterTests.swift b/Tests/ExportersTests/OpenTelemetryProtocol/OtlpLogRecordExporterTests.swift
@@ -86,16 +86,12 @@ class OtlpLogRecordExporterTests: XCTestCase {
 
     func testImplicitGrpcLoggingConfig() throws {
         let exporter = OtlpLogExporter(channel: channel)
-        guard let logger = exporter.callOptions?.logger else {
-            throw "Missing logger"
-        }
+        let logger = exporter.callOptions.logger
         XCTAssertEqual(logger.label, "io.grpc")
     }
     func testExplicitGrpcLoggingConfig() throws {
         let exporter = OtlpLogExporter(channel: channel, logger: Logger(label: "my.grpc.logger"))
-        guard let logger = exporter.callOptions?.logger else {
-            throw "Missing logger"
-        }
+        let logger = exporter.callOptions.logger  
         XCTAssertEqual(logger.label, "my.grpc.logger")
     }
 
@@ -110,13 +106,13 @@ class OtlpLogRecordExporterTests: XCTestCase {
         XCTAssertNotNil(exporter.config.headers)
         XCTAssertEqual(exporter.config.headers?[0].0, "FOO")
         XCTAssertEqual(exporter.config.headers?[0].1, "BAR")
-        XCTAssertEqual("BAR", exporter.callOptions?.customMetadata.first(name: "FOO"))
+        XCTAssertEqual("BAR", exporter.callOptions.customMetadata.first(name: "FOO"))
     }
 
     func testConfigHeadersAreSet_whenInitCalledWithExplicitHeaders() throws {
         let exporter = OtlpLogExporter(channel: channel, envVarHeaders: [("FOO", "BAR")])
         XCTAssertNil(exporter.config.headers)
-        XCTAssertEqual("BAR", exporter.callOptions?.customMetadata.first(name: "FOO"))
+        XCTAssertEqual("BAR", exporter.callOptions.customMetadata.first(name: "FOO"))
     }
 
     func testExportAfterShutdown() {

diff --git a/Tests/ExportersTests/OpenTelemetryProtocol/OtlpTraceExporterTests.swift b/Tests/ExportersTests/OpenTelemetryProtocol/OtlpTraceExporterTests.swift
@@ -50,28 +50,23 @@ class OtlpTraceExporterTests: XCTestCase {
 
     func testImplicitGrpcLoggingConfig() throws {
         let exporter = OtlpTraceExporter(channel: channel)
-        guard let logger = exporter.callOptions?.logger else {
-            throw "Missing logger"
-        }
-        XCTAssertEqual(logger.label, "io.grpc")
+        let logger = exporter.callOptions.logger
     }
 
     func testExplicitGrpcLoggingConfig() throws {
         let exporter = OtlpTraceExporter(channel: channel, logger: Logger(label: "my.grpc.logger"))
-        guard let logger = exporter.callOptions?.logger else {
-            throw "Missing logger"
-        }
+        let logger = exporter.callOptions.logger
         XCTAssertEqual(logger.label, "my.grpc.logger")
     }
 
     func verifyUserAgentIsSet(exporter: OtlpTraceExporter) {
-        if let callOptions = exporter.callOptions {
-            let customMetadata = callOptions.customMetadata
-            let userAgent = Headers.getUserAgentHeader()
-            if customMetadata.contains(name: Constants.HTTP.userAgent) && customMetadata.first(name: Constants.HTTP.userAgent) == userAgent {
-                return
-            }
+        let callOptions = exporter.callOptions
+        let customMetadata = callOptions.customMetadata
+        let userAgent = Headers.getUserAgentHeader()
+        if customMetadata.contains(name: Constants.HTTP.userAgent) && customMetadata.first(name: Constants.HTTP.userAgent) == userAgent {
+            return
         }
+
         XCTFail("User-Agent header was not set correctly")
     }
 
@@ -88,15 +83,15 @@ class OtlpTraceExporterTests: XCTestCase {
         XCTAssertNotNil(exporter.config.headers)
         XCTAssertEqual(exporter.config.headers?[0].0, "FOO")
         XCTAssertEqual(exporter.config.headers?[0].1, "BAR")
-        XCTAssertEqual("BAR", exporter.callOptions?.customMetadata.first(name: "FOO"))
+        XCTAssertEqual("BAR", exporter.callOptions.customMetadata.first(name: "FOO"))
 
         verifyUserAgentIsSet(exporter: exporter)
     }
 
     func testConfigHeadersAreSet_whenInitCalledWithExplicitHeaders() throws {
         let exporter = OtlpTraceExporter(channel: channel, envVarHeaders: [("FOO", "BAR")])
         XCTAssertNil(exporter.config.headers)
-        XCTAssertEqual("BAR", exporter.callOptions?.customMetadata.first(name: "FOO"))
+        XCTAssertEqual("BAR", exporter.callOptions.customMetadata.first(name: "FOO"))
 
         verifyUserAgentIsSet(exporter: exporter)
     }