jaegertracing / jaeger-client-python

🛑 This library is DEPRECATED!
https://jaegertracing.io/
Apache License 2.0
408 stars 155 forks source link

Log a warning when string tags are truncated #277

Closed wyattanderson closed 4 years ago

wyattanderson commented 4 years ago

Mostly putting this up for discussion.

With SQL queries reported as tags in the SQLAlchemy instrumentation as of uber-common/opentracing-python-instrumentation#107, I'm seeing a lot of truncated queries and I want to adjust max_tag_value_length in an informed manner. The intent with this change is to make it easier to tune max_tag_value_length based on knowledge of what tags are being truncated and what the actual length of the value is.

That said, if it's common or expected that tag values will be truncated, maybe this isn't the right approach, as I'd want to avoid spamming the logs with warnings that no one will ever pay attention to. Other thoughts I've had have been adding tags or log entries that indicate that a tag value is being truncated (so it would be visible in the UI).

codecov[bot] commented 4 years ago

Codecov Report

Merging #277 into master will increase coverage by 0.11%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
+ Coverage   94.82%   94.94%   +0.11%     
==========================================
  Files          25       25              
  Lines        1931     1937       +6     
  Branches      259      261       +2     
==========================================
+ Hits         1831     1839       +8     
+ Misses         66       65       -1     
+ Partials       34       33       -1     
Impacted Files Coverage Δ
jaeger_client/span.py 96.63% <100.00%> (+0.05%) :arrow_up:
jaeger_client/throttler.py 94.54% <0.00%> (-1.82%) :arrow_down:
jaeger_client/metrics/prometheus.py 100.00% <0.00%> (ø)
jaeger_client/utils.py 90.32% <0.00%> (+6.45%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8155a31...e06c78d. Read the comment docs.

yurishkuro commented 4 years ago

I'd certainly be concerned with suddenly spamming unsuspecting users with warning-level logs. One alternative is to emit a gauge-style metric for max observed tag length.

wyattanderson commented 4 years ago

Makes sense. I'll look into the metric. Thanks!