Add typing to tracer.py

[kasium]

This change was done by using pyannotate and running the test_tracer tests.
When in doubt, the type is now Any.

Part of #319

Signed-off-by: Kai Mueller 15907922+kasium@users.noreply.github.com

[codecov]

Codecov Report

Merging #328 (b75eac1) into master (c79ef27) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   95.51%   95.52%   +0.01%     
==========================================
  Files          25       25              
  Lines        2030     2035       +5     
  Branches      274      274              
==========================================
+ Hits         1939     1944       +5     
  Misses         57       57              
  Partials       34       34              

Impacted Files	Coverage Δ
jaeger_client/tracer.py	99.36% <100.00%> (+0.02%)	⬆️

---

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 c79ef27...b75eac1. Read the comment docs.

[kasium]

Please note, that for most types I checked the tests and the docu, but I'm no expert in your code. Feel free to check and to provide better/correct types.

[yurishkuro]

few nits, otherwise looks great

[yurishkuro]

when I run make lint it works fine without this change. How are you testing typing?

[kasium]

Did you also remove "bool(rpc_server)" from the other line. I see then

jaeger_client/tracer.py:223: error: Argument "join" to "_emit_span_metrics" of "Tracer" has incompatible type "Union[Dict[Any, Any], None, Any]"; expected "Optional[bool]"

[yurishkuro]

$ make lint
flake8 jaeger_client crossdock tests
mypy jaeger_client
Success: no issues found in 36 source files
./scripts/check-license.sh

$ gd | cat
diff --git a/jaeger_client/tracer.py b/jaeger_client/tracer.py
index e4e8e00..e553f6e 100644
--- a/jaeger_client/tracer.py
+++ b/jaeger_client/tracer.py
@@ -22,6 +22,8 @@ import random
 import sys
 import time
 import opentracing
+from typing import Optional
+
 from opentracing import Format, UnsupportedFormatException
 from opentracing.ext import tags as ext_tags
 from opentracing.scope_managers import ThreadLocalScopeManager
@@ -282,7 +284,7 @@ class Tracer(opentracing.Tracer):
         self.sampler.close()
         return self.reporter.close()

-    def _emit_span_metrics(self, span, join=False):
+    def _emit_span_metrics(self, span: Span, join: Optional[bool] = False):
         if span.is_sampled():
             self.metrics.spans_started_sampled(1)
         else:

[yurishkuro]

I did pip install mypy

mypy-0.910
mypy_extensions-0.4.3

[kasium]

So, of course if you remove the type annotation of the argument, mypy will not complain 😄

[yurishkuro]

I do have the type declared in the function. I also tried with my original suggestion, still successful:

        rpc_server: bool = tags and \
            tags.get(ext_tags.SPAN_KIND) == ext_tags.SPAN_KIND_RPC_SERVER

[kasium]

This one should fail, because a cast is needed. Not sure, why it's successful on your side. On mine it fails as expected.
Maybe a weird mypy cache issue or something else in your setup. Might also be related to the python version which you running.

However, making a cast is fine, so I see no risk here

[yurishkuro]

Yes, but the cast is ugly code. What errors are you getting with rpc_server: bool = ?

I'm running with Python 3.8.2

[kasium]

jaeger_client/tracer.py:182: error: Incompatible types in assignment (expression has type "Union[Dict[Any, Any], None, Any]", variable has type "bool")

Even a clean doesn't help

[kasium]

Ok, found a way to write it easier