From 6f086ad3a4fa8ea9d8f63aa27bbbbee5ea67523c Mon Sep 17 00:00:00 2001 From: Timo <38291523+lovetodream@users.noreply.github.com> Date: Sun, 21 Apr 2024 20:27:03 +0200 Subject: [PATCH] test: more tests and cleanup --- .github/workflows/tests.yml | 2 +- Sources/LoggingLoki/LokiLogProcessor.swift | 1 + Sources/LoggingLoki/Utility/NIOLock.swift | 178 ------------------ .../Utility/NIOLockedValueBox.swift | 46 ----- Tests/LoggingLokiTests/IntegrationTests.swift | 58 +++++- .../LokiLogProcessorTests.swift | 18 ++ 6 files changed, 76 insertions(+), 227 deletions(-) delete mode 100644 Sources/LoggingLoki/Utility/NIOLock.swift delete mode 100644 Sources/LoggingLoki/Utility/NIOLockedValueBox.swift diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a706911..b0cfa86 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -26,7 +26,7 @@ jobs: env: XCT_LOKI_URL: http://loki:3100 - name: Prepare Code Coverage - run: llvm-cov export -format="lcov" .build/debug/swift-log-lokiPackageTests.xctest -instr-profile .build/debug/codecov/default.profdata > info.lcov + run: llvm-cov export -format="lcov" .build/debug/swift-log-lokiPackageTests.xctest -instr-profile .build/debug/codecov/default.profdata -ignore-filename-regex="\/.pb.swift\/" > info.lcov - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v3 with: diff --git a/Sources/LoggingLoki/LokiLogProcessor.swift b/Sources/LoggingLoki/LokiLogProcessor.swift index 5f92bd8..cd1265a 100644 --- a/Sources/LoggingLoki/LokiLogProcessor.swift +++ b/Sources/LoggingLoki/LokiLogProcessor.swift @@ -3,6 +3,7 @@ import NIOHTTP1 import AsyncHTTPClient import ServiceLifecycle import AsyncAlgorithms +import NIOConcurrencyHelpers public struct LokiLogProcessorConfiguration: Sendable { /// The loki server URL, eg. `http://localhost:3100`. diff --git a/Sources/LoggingLoki/Utility/NIOLock.swift b/Sources/LoggingLoki/Utility/NIOLock.swift deleted file mode 100644 index 7b3daa8..0000000 --- a/Sources/LoggingLoki/Utility/NIOLock.swift +++ /dev/null @@ -1,178 +0,0 @@ -// Implementation vendored from SwiftNIO: -// https://github.com/apple/swift-nio - -//===----------------------------------------------------------------------===// -// -// This source file is part of the SwiftNIO open source project -// -// Copyright (c) 2017-2022 Apple Inc. and the SwiftNIO project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// See CONTRIBUTORS.txt for the list of SwiftNIO project authors -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -#if canImport(Darwin) -import Darwin -#elseif os(Windows) -import ucrt -import WinSDK -#elseif canImport(Glibc) -import Glibc -#elseif canImport(Musl) -import Musl -#else -#error("The concurrency NIOLock module was unable to identify your C library.") -#endif - -#if os(Windows) -@usableFromInline -typealias LockPrimitive = SRWLOCK -#else -@usableFromInline -typealias LockPrimitive = pthread_mutex_t -#endif - -@usableFromInline -enum LockOperations { } - -extension LockOperations { - @inlinable - static func create(_ mutex: UnsafeMutablePointer) { - mutex.assertValidAlignment() - -#if os(Windows) - InitializeSRWLock(mutex) -#else - var attr = pthread_mutexattr_t() - pthread_mutexattr_init(&attr) - debugOnly { - pthread_mutexattr_settype(&attr, .init(PTHREAD_MUTEX_ERRORCHECK)) - } - - let err = pthread_mutex_init(mutex, &attr) - precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)") -#endif - } - - @inlinable - static func destroy(_ mutex: UnsafeMutablePointer) { - mutex.assertValidAlignment() - -#if os(Windows) - // SRWLOCK does not need to be free'd -#else - let err = pthread_mutex_destroy(mutex) - precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)") -#endif - } - - @inlinable - static func lock(_ mutex: UnsafeMutablePointer) { - mutex.assertValidAlignment() - -#if os(Windows) - AcquireSRWLockExclusive(mutex) -#else - let err = pthread_mutex_lock(mutex) - precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)") -#endif - } - - @inlinable - static func unlock(_ mutex: UnsafeMutablePointer) { - mutex.assertValidAlignment() - -#if os(Windows) - ReleaseSRWLockExclusive(mutex) -#else - let err = pthread_mutex_unlock(mutex) - precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)") -#endif - } -} - -// Tail allocate both the mutex and a generic value using ManagedBuffer. -// Both the header pointer and the elements pointer are stable for -// the class's entire lifetime. -// -// However, for safety reasons, we elect to place the lock in the "elements" -// section of the buffer instead of the head. The reasoning here is subtle, -// so buckle in. -// -// _As a practical matter_, the implementation of ManagedBuffer ensures that -// the pointer to the header is stable across the lifetime of the class, and so -// each time you call `withUnsafeMutablePointers` or `withUnsafeMutablePointerToHeader` -// the value of the header pointer will be the same. This is because ManagedBuffer uses -// `Builtin.addressOf` to load the value of the header, and that does ~magic~ to ensure -// that it does not invoke any weird Swift accessors that might copy the value. -// -// _However_, the header is also available via the `.header` field on the ManagedBuffer. -// This presents a problem! The reason there's an issue is that `Builtin.addressOf` and friends -// do not interact with Swift's exclusivity model. That is, the various `with` functions do not -// conceptually trigger a mutating access to `.header`. For elements this isn't a concern because -// there's literally no other way to perform the access, but for `.header` it's entirely possible -// to accidentally recursively read it. -// -// Our implementation is free from these issues, so we don't _really_ need to worry about it. -// However, out of an abundance of caution, we store the Value in the header, and the LockPrimitive -// in the trailing elements. We still don't use `.header`, but it's better to be safe than sorry, -// and future maintainers will be happier that we were cautious. -// -// See also: https://github.com/apple/swift/pull/40000 -@usableFromInline -final class LockStorage: ManagedBuffer { - - @inlinable - static func create(value: Value) -> Self { - let buffer = Self.create(minimumCapacity: 1) { _ in - return value - } - let storage = unsafeDowncast(buffer, to: Self.self) - - storage.withUnsafeMutablePointers { _, lockPtr in - LockOperations.create(lockPtr) - } - - return storage - } - - @inlinable - deinit { - self.withUnsafeMutablePointerToElements { lockPtr in - LockOperations.destroy(lockPtr) - } - } - - @inlinable - func withLockedValue(_ mutate: (inout Value) throws -> T) rethrows -> T { - try self.withUnsafeMutablePointers { valuePtr, lockPtr in - LockOperations.lock(lockPtr) - defer { LockOperations.unlock(lockPtr) } - return try mutate(&valuePtr.pointee) - } - } -} - -extension LockStorage: @unchecked Sendable { } - -extension UnsafeMutablePointer { - @inlinable - func assertValidAlignment() { - assert(UInt(bitPattern: self) % UInt(MemoryLayout.alignment) == 0) - } -} - -/// A utility function that runs the body code only in debug builds, without -/// emitting compiler warnings. -/// -/// This is currently the only way to do this in Swift: see -/// https://forums.swift.org/t/support-debug-only-code/11037 for a discussion. -@inlinable -internal func debugOnly(_ body: () -> Void) { - // FIXME: duplicated with NIO. - assert({ body(); return true }()) -} diff --git a/Sources/LoggingLoki/Utility/NIOLockedValueBox.swift b/Sources/LoggingLoki/Utility/NIOLockedValueBox.swift deleted file mode 100644 index e5a3e6a..0000000 --- a/Sources/LoggingLoki/Utility/NIOLockedValueBox.swift +++ /dev/null @@ -1,46 +0,0 @@ -// Implementation vendored from SwiftNIO: -// https://github.com/apple/swift-nio - -//===----------------------------------------------------------------------===// -// -// This source file is part of the SwiftNIO open source project -// -// Copyright (c) 2022 Apple Inc. and the SwiftNIO project authors -// Licensed under Apache License v2.0 -// -// See LICENSE.txt for license information -// See CONTRIBUTORS.txt for the list of SwiftNIO project authors -// -// SPDX-License-Identifier: Apache-2.0 -// -//===----------------------------------------------------------------------===// - -/// Provides locked access to `Value`. -/// -/// - note: ``NIOLockedValueBox`` has reference semantics and holds the `Value` -/// alongside a lock behind a reference. -/// -/// This is no different than creating a ``Lock`` and protecting all -/// accesses to a value using the lock. But it's easy to forget to actually -/// acquire/release the lock in the correct place. ``NIOLockedValueBox`` makes -/// that much easier. -@usableFromInline -struct NIOLockedValueBox { - - @usableFromInline - internal let _storage: LockStorage - - /// Initialize the `Value`. - @inlinable - init(_ value: Value) { - self._storage = .create(value: value) - } - - /// Access the `Value`, allowing mutation of it. - @inlinable - func withLockedValue(_ mutate: (inout Value) throws -> T) rethrows -> T { - return try self._storage.withLockedValue(mutate) - } -} - -extension NIOLockedValueBox: Sendable where Value: Sendable {} diff --git a/Tests/LoggingLokiTests/IntegrationTests.swift b/Tests/LoggingLokiTests/IntegrationTests.swift index 4123877..182f4bc 100644 --- a/Tests/LoggingLokiTests/IntegrationTests.swift +++ b/Tests/LoggingLokiTests/IntegrationTests.swift @@ -3,6 +3,7 @@ import NIOCore import NIOHTTP1 import Atomics import AsyncHTTPClient +import NIOConcurrencyHelpers @testable import LoggingLoki final class InspectableTransport: LokiTransport { @@ -10,9 +11,27 @@ final class InspectableTransport: LokiTransport { let transported = ManagedAtomic(0) + let errored = ManagedAtomic(0) + let errors: NIOLockedValueBox<[Error]> = NIOLockedValueBox([]) + func transport(_ data: ByteBuffer, url: String, headers: HTTPHeaders) async throws { - try await actual.transport(data, url: url, headers: headers) - transported.wrappingIncrement(ordering: .relaxed) + do { + try await actual.transport(data, url: url, headers: headers) + transported.wrappingIncrement(ordering: .relaxed) + } catch { + errored.wrappingIncrement(ordering: .relaxed) + errors.withLockedValue { $0.append(error) } + } + } +} + +final class BadRequestTransformer: LokiTransformer { + func transform(_ entries: [BatchEntry], headers: inout HTTPHeaders) throws -> ByteBuffer { + headers.add(name: "Content-Type", value: "application/json") + var buffer = ByteBuffer() + buffer.writeString("bad_request :(") + try buffer.writeJSONEncodable(LokiRequest.from(entries: entries)) + return buffer } } @@ -25,6 +44,41 @@ final class IntegrationTests: XCTestCase { try await runHappyPath(LokiJSONTransformer()) } + func testBadRequest() async throws { + try await withThrowingDiscardingTaskGroup { group in + let clock = TestClock() + let transport = InspectableTransport() + let processor = LokiLogProcessor( + configuration: .init(lokiURL: env("XCT_LOKI_URL") ?? "http://localhost:3100", maxBatchTimeInterval: .seconds(10)), + transport: transport, + transformer: BadRequestTransformer(), + clock: clock + ) + var sleepCalls = clock.sleepCalls.makeAsyncIterator() + group.addTask { + try await processor.run() + } + let handler = LokiLogHandler(label: "com.timozacherl.swift-log-loki-tests", processor: processor) + logLine(handler: handler) + await sleepCalls.next() + + // move forward in time until max batch time interval is exceeded + clock.advance(by: .seconds(5)) // tick + await sleepCalls.next() + clock.advance(by: .seconds(5)) // tick + await sleepCalls.next() + + await sleepCalls.next() // export + XCTAssertEqual(transport.transported.load(ordering: .relaxed), 0) + XCTAssertEqual(transport.errored.load(ordering: .relaxed), 1) + let errors = transport.errors.withLockedValue { $0 } + let error = try XCTUnwrap(errors.first as? LokiResponseError) + XCTAssertEqual(error.response.status, .badRequest) + + group.cancelAll() + } + } + func runHappyPath(_ transformer: LokiTransformer, file: StaticString = #filePath, line: UInt = #line) async throws { try await withThrowingDiscardingTaskGroup { group in let clock = TestClock() diff --git a/Tests/LoggingLokiTests/LokiLogProcessorTests.swift b/Tests/LoggingLokiTests/LokiLogProcessorTests.swift index 118040b..105737f 100644 --- a/Tests/LoggingLokiTests/LokiLogProcessorTests.swift +++ b/Tests/LoggingLokiTests/LokiLogProcessorTests.swift @@ -40,4 +40,22 @@ final class LokiLogProcessorTests: XCTestCase { XCTAssertNil(formatted.metadata) XCTAssertEqual(formatted.line, #"INFO: My log message [additional_key: value with whitespace, basic_key: basic_value]"#) } + + func testLogFmtFormatEmptyMetadata() { + var configuration = LokiLogProcessorConfiguration( + lokiURL: "http://localhost:3100", + metadataFormat: .logfmt + ) + configuration.encoding = .json + let processor = LokiLogProcessor(configuration: configuration) + let raw = LokiLog( + timestamp: .init(), + level: .info, + message: "My log message", + metadata: [:] + ) + let formatted = processor.makeLog(raw) + XCTAssertNil(formatted.metadata) + XCTAssertEqual(formatted.line, #"[INFO] message="My log message""#) + } }