feat(logging): add implementation for cloudwatch client#4212
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## cloudwatch-client-2 #4212 +/- ##
======================================================
Coverage ? 66.64%
======================================================
Files ? 1145
Lines ? 43602
Branches ? 0
======================================================
Hits ? 29057
Misses ? 14545
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| public init(from decoder: Decoder) throws { | ||
| let container = try decoder.singleValueContainer() | ||
| if let rawString = try? container.decode(String.self).lowercased() { | ||
| switch rawString { | ||
| case "error": self = .error | ||
| case "warn": self = .warn | ||
| case "info": self = .info | ||
| case "debug": self = .debug | ||
| case "verbose": self = .verbose | ||
| case "none": self = .none | ||
| default: | ||
| let context = DecodingError.Context( | ||
| codingPath: [], | ||
| debugDescription: "No matching LogLevel found" | ||
| ) | ||
| throw DecodingError.valueNotFound(AmplifyFoundation.LogLevel.self, context) | ||
| } | ||
| } else if let rawInt = try? container.decode(Int.self), | ||
| let value = AmplifyFoundation.LogLevel(rawValue: rawInt) { | ||
| self = value | ||
| } else { | ||
| let context = DecodingError.Context( | ||
| codingPath: [], | ||
| debugDescription: "Unable to decode LogLevel" | ||
| ) | ||
| throw DecodingError.dataCorrupted(context) | ||
| } | ||
| } | ||
|
|
||
| public func encode(to encoder: Encoder) throws { | ||
| var container = encoder.singleValueContainer() | ||
| try container.encode(rawValue) | ||
| } |
There was a problem hiding this comment.
Curious to understand why you had to write this custom encoding and decoding logic, shouldn't this be automatically taken care by Swift or we have to follow some custom logic?
There was a problem hiding this comment.
Earlier this was being populated from config file, so this was needed for Codable conformance. Will remove it as it's not needed in the new client.
There was a problem hiding this comment.
Consider adding default values here to ?
| } | ||
|
|
||
| extension CloudWatchLoggingConsumer: LogBatchConsumer { | ||
| func consume(batch: any LogBatch) async throws { |
There was a problem hiding this comment.
this seems to be a long method, can we break it down to logical pieces and test them individually and also combined? Similar thing applies to some other methods in the file too.
| self.logGroupName = logGroupName | ||
| } | ||
|
|
||
| private func safeEncode(_ value: some Encodable) throws -> Data { |
There was a problem hiding this comment.
is safeEncode reallly necessary? because I see encode without locks in other parts of the file.
| func encode(entry: LogEntry) throws -> Data { | ||
| let encoder = JSONEncoder() | ||
| encoder.dateEncodingStrategy = .millisecondsSince1970 | ||
| encoder.outputFormatting = .sortedKeys |
|
|
||
| func decode(from fileURL: URL) throws -> [LogEntry] { | ||
| guard fileURL.isFileURL else { | ||
| throw DecodingError.invalidScheme(log: fileURL) |
There was a problem hiding this comment.
Should this error be more descriptive, like invalidPath or invalidFile, not sure why we chose invalid scheme.
There was a problem hiding this comment.
it's coming from here: isFileURL checks if file:// scheme is present
https://developer.apple.com/documentation/foundation/nsurl/isfileurl
|
|
||
| // MARK: - LogSinkBehavior | ||
|
|
||
| public var id: String { sinkId } |
There was a problem hiding this comment.
should we rename this to a sink id?
There was a problem hiding this comment.
this is coming from LogSinkBehavior conformance.
* feat(logging): add v3 cloudwatch @SPI definitions * add privacyinfo * chore(logging): move shared code to internal module * fix unit tests * feat(logging): add implementation for cloudwatch client --------- Co-authored-by: Harsh <6162866+harsh62@users.noreply.github.com>
2 participants
Issue #
NA
Description
Add implementation for cloudwatch logging client
General Checklist
Given When Theninline code documentation and are named accordinglytestThing_condition_expectation()By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.