aws / aws/aws-xray-sdk-python

Consider using warnings over logging.warning for important messages

オープン
#347 コメント 4 件 リアクション 0 件 担当者 1 名 @NathanielRN が担当を希望しています GitHub で見る
主要言語
Python
スター
338
フォーク
147
PR マージ指標
30日以内にマージされた PR はありません

説明

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!

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。