add cloud trace propagator #819
Conversation
...elemetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloudtracepropagator.py
Outdated
Show resolved
Hide resolved
AndrewAXue
force-pushed
the
cloud_propagator
branch
from
be19bc5 to
680cce2
Compare
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
|
I think this header is still required to get automatic tracing in App Engine, so this is great to have! What does the user actually have to do to use |
…ter/cloud_trace/cloud_trace_propagator.py Co-authored-by: Aaron Abbott <[email protected]>
| ) | ||
|
|
||
|
|
||
| def get_dict_value(dict_object: typing.Dict[str, str], key: str) -> str: |
There was a problem hiding this comment.
Do we have any conventions for typing hints? I see typing hints being using in a few places, but I don't see much consistency.
There was a problem hiding this comment.
The tl;dr is that these hints never get evaluated, and don't follow any convention in this repo. They're for the benefit of the person reading the code only and can be safely removed.
Longer answer:
Every public method in opentelemetry-api should have type hints. Tests in that package may have type hints, but the point is to uncover unexpected type changes in the API, not to document the tests.
tox -e mypy runs:
mypy --namespace-packages opentelemetry-api/src/opentelemetry/
mypy --namespace-packages --config-file=mypy-relaxed.ini opentelemetry-api/tests/
We don't run mypy for opentelemetry-sdk, but the public-facing parts of the SDK (including all the exporter classes) should have type hints, and we really should have a tox target that checks this.
We also don't run mypy on ext packages, so the type hints here are just documentation. If we wanted type checking for ext packages similar to the API package we could add a target that ran:
mypy --namespace-packages --config-file=mypy-relaxed.ini ext/opentelemetry-exporter-cloud-trace/tests
...but this fails right now, and we may need to add type stubs for dependencies to get meaningful results from mypy here.
There was a problem hiding this comment.
Issue for tox not running mypy on the sdk: #773
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
| ) | ||
|
|
||
|
|
||
| def get_dict_value(dict_object: typing.Dict[str, str], key: str) -> str: |
There was a problem hiding this comment.
The tl;dr is that these hints never get evaluated, and don't follow any convention in this repo. They're for the benefit of the person reading the code only and can be safely removed.
Longer answer:
Every public method in opentelemetry-api should have type hints. Tests in that package may have type hints, but the point is to uncover unexpected type changes in the API, not to document the tests.
tox -e mypy runs:
mypy --namespace-packages opentelemetry-api/src/opentelemetry/
mypy --namespace-packages --config-file=mypy-relaxed.ini opentelemetry-api/tests/
We don't run mypy for opentelemetry-sdk, but the public-facing parts of the SDK (including all the exporter classes) should have type hints, and we really should have a tox target that checks this.
We also don't run mypy on ext packages, so the type hints here are just documentation. If we wanted type checking for ext packages similar to the API package we could add a target that ran:
mypy --namespace-packages --config-file=mypy-relaxed.ini ext/opentelemetry-exporter-cloud-trace/tests
...but this fails right now, and we may need to add type stubs for dependencies to get meaningful results from mypy here.
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
...emetry-exporter-cloud-trace/src/opentelemetry/exporter/cloud_trace/cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Show resolved
Hide resolved
ext/opentelemetry-exporter-cloud-trace/tests/test_cloud_trace_propagator.py
Outdated
Show resolved
Hide resolved
…ter/cloud_trace/cloud_trace_propagator.py Co-authored-by: Diego Hurtado <[email protected]>
…propagator.py Co-authored-by: Diego Hurtado <[email protected]>
Some old services (e.g. https://github.com/googleapis/cloud-trace-nodejs/blob/master/samples/app.js) still use the http header "X-Cloud-Trace-Context" to pass SpanContext. This propagator allows us both to inject and extract info from this http header so we have backwards compatibility both upstream and downstream.
Addresses issue GoogleCloudPlatform/opentelemetry-operations-python#5