Skip to content

Conversation

@TheKevJames
Copy link
Member

Summary

Clean up our general approach to logging:

  • include an exc_message extra wherever an exception is relevant to
    the log, to allow for message-level aggregation (eg. grouping errors
    by raised exception)
  • avoid using string templates in exception messages, to make
    aggregation easier (message is always a constant string for any
    given error case, so it can be alerted on)
  • downgrade application callback exceptions to a warning, since that can
    be a normal part of user flows -- if users want it logged at the error
    level, they can always do that themselves, we shouldn't be enforcing
    it

Clean up our general approach to logging:
* include an `exc_message` extra wherever an exception is relevant to
  the log, to allow for message-level aggregation (eg. grouping errors
  by raised exception)
* avoid using string templates in exception messages, to make
  aggregation easier (`message` is always a constant string for any
  given error case, so it can be alerted on)
* downgrade application callback exceptions to a warning, since that can
  be a normal part of user flows -- if users want it logged at the error
  level, they can always do that themselves, we shouldn't be enforcing
  it
@TheKevJames TheKevJames requested review from a team and cphoward as code owners March 19, 2025 14:18
@TheKevJames TheKevJames requested review from eileensoong and olidp and removed request for a team March 19, 2025 14:18
Copy link
Contributor

@eddiedialpad eddiedialpad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved: logging practices in pubsub

@TheKevJames TheKevJames merged commit 2eb5931 into master Mar 19, 2025
72 checks passed
@TheKevJames TheKevJames deleted the kjames/pubsub-log-cleanup branch March 19, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants