Closed dreamorosi closed 3 days ago
@heitorlessa & @am29d would love your opinion on this and especially your point of view on the concern I share at the end of the "Solution/User Experience" section.
Also please let me know if anything is not clear, happy to clarify & expound on any detail. Thanks!
⚠️ COMMENT VISIBILITY WARNING ⚠️
This issue is now closed. Please be mindful that future comments are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.
Use case
The Metrics utility emits some warning logs to notify customers that certain expected conditions are not being met. This is the case for when a namespace is not specified or when the
publishStoredMetrics()
method is called on an empty buffer.Currently customers have no way of suppressing these warnings and some customers have reported wanting to do so (#2036). I think this is a fair ask and whatever implementation we settle for in this issue will also be reused for other utilities that emit either warnings or debug logs (Idempotency, Tracer, and Parameters).
Solution/User Experience
We could define a new type/interface in the
commons
package and expose it:From there, customers can use it as a reference to create their own logger and pass it to the Metrics utility.
In the example below I'm making the warn method a no-op to disable the warnings entirely, but customers can write their own custom implementation to decide whether the logs are emitted or not.
Customers should also be able to pass an instance of Powertools Logger if they wish to do so:
My main concern with this is avoiding confusion and conveying clearly that this is only a logger that will be used for debug and warning logs but not to emit the EMF metrics themselves.
The Metrics utility maintain its own
Console
object that logs the metrics usingconsole.log
(notice that thelog()
method is not part of the suggested interface). This is needed for the Metrics utility to work with the Advanced Logging Configuration feature.Alternative solutions
If my memory serves me right the AWS Lambda Node.js managed runtime treats warning emitted via
process.emitWarning(warning[, options])
as errors rendering this method unviable.As part of this issue however we should still test this option just in case I'm wrong.
Other alternatives that I'm not inclined to consider would go along the lines of adding a
doNotWarnOnEmptyMetrics
option to suppress these warnings.Acknowledgment
Future readers
Please react with 👍 and your use case to help us understand customer demand.