Consider using warnings over logging.warning for important messages
- Dominant language
- Python
- Stars
- 338
- Forks
- 147
- PR merge metrics
- No merged PRs in 30d
Description
Hello X-Ray team!
I've recently discovered that tracing annotations with `-` or spaces in the name are silently dropped at the [SDK level](https://github.com/aws/aws-xray-sdk-python/blob/894b419518e14b5cb2a75293ed4da17f87b2a7c0/aws_xray_sdk/core/models/entity.py#L146). When looking for a possible cause, I've found out X-Ray documentation calls this out in the [API Segment Document schema page](https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-annotations).
It would've been quicker to spot this issue if X-Ray SDK were to use the standard library [Warnings package](https://docs.python.org/3/library/warnings.html). This allows library maintainers to warn their users and give them a chance to suppress certain filters at will.
**What's the difference between `logging.warning` vs `warnings.warn`?**
For Logging, it's a good practice for library owners to use a [NullHandler](https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library). This means any logging message produced by a library will be silently dropped, **unless** the consumer explicitly enables the library logging in question (or sets a Root logger).
In contrast, `warnings.warn` will notify consumers that something it's worth investigating. They also support categories of warnings to allow complex libraries to have multiple warnings, so their consumers can explicitly disable a subset of warnings without having to know every possible message to filter out.
Here's an example in Lambda Powertools for Python where we [warn customers](https://github.com/awslabs/aws-lambda-powertools-python/blob/develop/aws_lambda_powertools/metrics/metrics.py#L189:L189) if they have no metrics to be flushed. This will happen regardless of how they configure their loggers. If they're intentional about having no user-defined metrics, they suppress this warning altogether with `warnings.filterwarnings("ignore", "No metrics to publish*")`.
Hopefully this will help strike a balance between not interrupting customers at runtime while warning them they might be losing important tracing information.
Thank you!
Contributor guide
Assessment
This issue has not been assessed yet.