Better support for non-string metadata values
- Dominant language
- Swift
- Stars
- 4k
- Forks
- 344
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 4
Description
The metadata structure of `swift-log` is similar to JSON, but values can only be `String` (or `CustomStringConvertible`). In my opinion, this makes it hard for both users of `swift-log` (those that generate the logs) och implementors of `LogHandler`s to properly handle non-string data, such as booleans and numeric data.
## Current situation
Currently, when adding numeric values as metadata, we can add its string representation (which is suggested in [002-StructuredLogging.md](https://github.com/apple/swift-log/blob/main/Sources/Logging/Docs.docc/BestPractices/002-StructuredLogging.md)) or wrap it in `.stringConvertible()`. However, adding all values as strings comes at a cost.
For example, if a `LogHandler` wants to output JSON that is sent to an ELK stack, the logs should be formatted according to ECS (_Elastic common schema_). ECS expects certain fields to be numerical, e.g. [`http.response.status_code` and `http.response.bytes`](https://www.elastic.co/docs/reference/ecs/ecs-http). The situation is the same in [OTEL](https://opentelemetry.io/docs/specs/semconv/registry/attributes/http/#http-request-body-size), and though I have no experience with the LTGM stack, I suspect that it has the same concept of different data types.
One reason for having different data types in these logging tools is to allow for sensible filtering. Even for custom attributes, if I use proper types I can do filterings like “get all logs where ‘process_time_ms' > 10_000". If the tool is strict, it might even reject log data that uses existing keys but with conflicting types.
All this is to say that I think treating all metadata as strings is sub-optimal, especially since we want to promote structured logging.
## (Probably?) Unrealistic change
One change could be to extend `MetadataValue` to be more like JSON, i.e. add cases for booleans and numerics.
```swift
public enum MetadataValue {
case string(String)
case bool(Bool)
case numeric(any Numeric & CustomStringConvertible & Sendable)
case stringConvertible(any CustomStringConvertible & Sendable)
case dictionary(Metadata)
case array([Metadata.Value])
}
```
This is obviously breaking since the enum is public, and that would require all `LogHandler`s to handle the two added cases. However, it would solve the issues I pointed out, since a `LogHandler` could safely serialize booleans and numeric values, or continue to use their string representations.
## More realistic change
A more realistic approach would be to see what could be changed/added in a non-breaking way. Let’s look at a few different examples of `LogHandler` implementations:
### swift-otel
The `LogHandler` of [swift-otel](https://github.com/swift-otel/) maps metadata to OTEL-proto, and maps all values to strings, (https://github.com/swift-otel/swift-otel/blob/main/Sources/OTel/OTLPCore%2BExtensions/Logging/OTelLogRecord%2BProto.swift#L83-L97). This is unfortunate, since the proto definitions allow for many more types ([bool, int, double](https://github.com/swift-otel/swift-otel/blob/main/Sources/OTel/OTLPCore/Generated/opentelemetry/proto/common/v1/common.pb.swift#L65-L87)).
### StackdriverLogging
The `LogHandler` of [StackdriverLogging](https://github.com/brainfinance/StackdriverLogging/) switches on the metadata value enum case, and for `StringConvertible`, checks if the associated data is valid json data returns that rather than the string representation (https://github.com/brainfinance/StackdriverLogging/blob/master/Sources/StackdriverLogging/StackdriverLogHandler.swift#L192-L200).
### json-logger
The `LogHandler` of [json-logger](https://github.com/xcode-actions/json-logger/) also switches on the metadata value case, and for `StringConvertible` checks if the associated data is `Encodable`, and if so JSON-encodes it (https://github.com/xcode-actions/json-logger/blob/develop/Sources/JSONLogger.swift#L344-L357).
Both `StackdriverLogging` and `json-logger` makes it so that
```swift
logger.info(“Log”, metadata: [“key1”: ”\(15)”, “key2": .stringConvertible(15)])
```
yield a JSON-representation like
```json
{
“key1”: “15”
“key2”: 15
}
```
### Proposed additions
So as we can see, it’s not impossible for a `LogHandler` to pattern match on the values and convert common types (Bool, Int, Float, Date) to proper output types/format, but to get it right takes some effort. Also, it’s not intuitive for the user of `Logger` that there is any benefit of using the `.stringConvertible` case rather than just stringifying the value.
To move in this direction, I wonder if it would be sensible to add conformance to `ExpressibleByIntegerLiteral`, `ExpressibleByBooleanLiteral` and `ExpressibleByFloatLiteral`.
```swift
logger.info(“Log”, metadata: [“key1”: 15, “key2": false, “key3”: -1.23])
```
which would be identical to
```swift
logger.info(“Log”, metadata: [“key1”: .stringConvertible(15), “key2": .stringConvertible(false), “key3”: .stringConvertible(-1.23)])
```
## Discussion
The proposed additions doesn’t really help in the case where the values are stored in variables. For booleans, I can see the benefit of passing literals (e.g. `logger[metadataKey: “success”] = true` in an if-case), but having float literals feels less likely. One argue for adding int/float literals would be consistency with the other types.
I’m curious to hear your thoughts on this, how it could be made more convenient. Or are you not seeing this as an issue?
Contributor guide
Research direction
Start by reading the public MetadataValue and LogHandler APIs, then review 002-StructuredLogging.md and the cited swift-otel, StackdriverLogging, and json-logger implementations. The issue needs an agreed non-breaking approach for typed metadata, with its API compatibility and expected handler behavior clearly defined before implementation can be considered done.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- backend-api-design, observability-sre
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 38/100