From 09b6529c9dfd271e08fc3f6856a62fafa7fe0d6b Mon Sep 17 00:00:00 2001 From: Bryce Buchanan Date: Thu, 14 Sep 2023 12:03:27 -0700 Subject: [PATCH 1/3] ensure that call option timeout is included in export calls --- Package.swift | 16 ++++++++-------- .../logs/OtlpLogExporter.swift | 8 +++++++- .../trace/OtlpTraceExporter.swift | 4 ++-- .../SpanProcessors/BatchSpanProcessor.swift | 5 ++++- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Package.swift b/Package.swift index b914dac6..1f10a928 100644 --- 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 index c73e53b3..4c1fa01a 100644 --- 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 index 6fbcfb60..eb8c57dd 100644 --- 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 index c0fa1705..5abfb7c7 100644 --- a/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift +++ b/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift @@ -134,7 +134,10 @@ 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() From a1e64a125c7151bf13c3b5ef92c34d8d25ce031e Mon Sep 17 00:00:00 2001 From: Bryce Buchanan <75274611+bryce-b@users.noreply.github.com> Date: Thu, 14 Sep 2023 12:48:42 -0700 Subject: [PATCH 2/3] Update BatchSpanProcessor.swift --- .../Trace/SpanProcessors/BatchSpanProcessor.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift b/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift index 5abfb7c7..3e7aa027 100644 --- a/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift +++ b/Sources/OpenTelemetrySdk/Trace/SpanProcessors/BatchSpanProcessor.swift @@ -136,7 +136,6 @@ private class BatchWorker: Thread { let timeoutTimer = DispatchSource.makeTimerSource(queue: DispatchQueue.global()) timeoutTimer.setEventHandler { exportOperation.cancel() - } let maxTimeOut = min(explicitTimeout ?? TimeInterval.greatestFiniteMagnitude, exportTimeout) timeoutTimer.schedule(deadline: .now() + .milliseconds(Int(maxTimeOut.toMilliseconds)), leeway: .milliseconds(1)) From bf5f4ec1a0ade38b35a6d9b391c6fdb6330bf410 Mon Sep 17 00:00:00 2001 From: Bryce Buchanan Date: Mon, 18 Sep 2023 11:37:16 -0700 Subject: [PATCH 3/3] fixed tests --- .../OtlpLogRecordExporterTests.swift | 12 +++------ .../OtlpTraceExporterTests.swift | 25 ++++++++----------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/Tests/ExportersTests/OpenTelemetryProtocol/OtlpLogRecordExporterTests.swift b/Tests/ExportersTests/OpenTelemetryProtocol/OtlpLogRecordExporterTests.swift index 7947ac28..7d35708a 100644 --- 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 index 58c41f89..ee7befb6 100644 --- 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,7 +83,7 @@ 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) } @@ -96,7 +91,7 @@ class OtlpTraceExporterTests: XCTestCase { 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) }