weissi / swift-server-logging-api-proposal

SSWG Proposal for Logging API
9 stars 8 forks source link

add support for local metadata (option 1) #16

Closed tomerd closed 5 years ago

tomerd commented 5 years ago

motivation: allow users to define metadata at the log API call site

changes: add metadata param to all log APIs, and implement merge at our example implementations

tomerd commented 5 years ago

@weissi not sure about this one, since the merging is delegated to the implementation, but given the current api design where metadata is managed at the logger level, this seems to be the most flexible design

the alternative is to do the merging at the API level and then the implementations will need to know they should ignore the instance state and only use the merged one that came through the LogHandler.log call. see option 2. the main advantage of option 2 is somewhat easier API to reason about, the disadvantage is performance hit on prettyfing the metadata every call

wdyt?

/cc @ktoso

ktoso commented 5 years ago

Looked in depth at both options and I think option 1 (this one) is viable and good™.

It makes sense to leave to implementations to do what they want with the incoming per log statement metadata. The ability to keep the pre-rendered if-no-per-log-statement-metadata is also very valuable (or rather, not being able to do so in option 2 disqualifies it I'd say even).

👍 for this version. And guidance that implementations should optimize for the metadata staying stable, and merge on incoming more metadata, though they MAY choose to do other things.

weissi commented 5 years ago

thanks @tomerd , that lgtm!