Add more details to logs to improve debugging.#43
Conversation
|
If you're happy with the approach I'll extend this to the workflow processor also. |
|
Sample from the examples:
|
antstorm
left a comment
antstorm
left a comment
There was a problem hiding this comment.
I like the general direction of adding more info, but let's do this slightly differently. Unstructured logging is difficult to parse and work with. Instead let's add an optional hash to the log calls with all the additional information we want to pass.
Many companies are already doing structured logging, so when implementing their own adapter everyone should be able to format the log lines as they prefer
Defaults to 25 seconds to fit inside the default grace period for kubernetes pod shutdown which is 30 seconds.
This reverts commit b051b53.
Convert some missed logging calls to metadata versions.
antstorm
left a comment
antstorm
left a comment
There was a problem hiding this comment.
Found a couple of potential issues. Also can you please add a spec for the new logger class?
The user can log from their own code if needed for debugging.
Ensure JSON is in strict mode.
|
|
||
| SEVERITIES.each do |severity| | ||
| define_method severity do |message| | ||
| define_method severity do |*args| |
There was a problem hiding this comment.
Can we keep this interface same as our main logger? E.g.:
define_method severity do |message, details = {}|
...
end
| end | ||
|
|
||
| def log(severity, message) | ||
| def log(severity, *args) |
|
This change meant that a standard logger passed into config will no longer function and all loggers in effect must be of type Temporal::Logger because of the signature change on the standard log methods. This makes things a little awkward if you have standard log practices in your organization. It is probably worth a call out in the README that while logger is exposed in Temporal.configure, you should use caution overriding it. |
|
@tristan-instacart you're right, apologies for this. Hopefully the new interface is very easily adoptable. @robholland would you mind updating the README with the mention of the new interface? |
…ner/workflow-started-metadata Run start time available to running workflow
4 participants
No description provided.