Commit 96cd202
authored
Sampler to default to parentbased_always_on if OTEL_TRACES_SAMPLER is Unset (#140)
*Issue #, if available:*
When `OTEL_TRACES_SAMPLER` is unset, the following error occurs when
applying auto-instrumentation:
```
File "/Users/test/Documents/my_test_venv/lib/python3.12/site-packages/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py", line 136, in _init_tracing
sampler = _customize_sampler(sampler)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/test/Documents/my_test_venv/lib/python3.12/site-packages/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py", line 210, in _customize_sampler
return AlwaysRecordSampler(sampler)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/test/Documents/my_test_venv/lib/python3.12/site-packages/amazon/opentelemetry/distro/always_record_sampler.py", line 37, in __init__
raise ValueError("root_sampler must not be None")
```
If `OTEL_TRACES_SAMPLER` is not set, this error will occur because we
don’t fallback to a default sampler
In upstream OTel Python, this is fine, because the following events
occur:
```
OTEL_TRACES_SAMPLER is unset -> _get_sampler returns None
TracerProvider receives NONE sampler -> TracerProvider defaults to `parentbased_always_on`
```
In this repository, this is not fine because the following events occur:
```
OTEL_TRACES_SAMPLER is unset -> _get_sampler returns None
_get_sampler returns None -> _custom_import_sampler returns None
_customize_sampler receives None sampler -> AlwaysRecordSampler receives NONE sampler and raises error
```
*Description of changes:*
Currently, `_get_sampler()` method returns
`environ.get(OTEL_TRACES_SAMPLER, None)`.
However, OTel Docs states that `OTEL_TRACES_SAMPLER` should [default to
parentbased_always_on](https://opentelemetry.io/docs/languages/sdk-configuration/general/#otel_traces_sampler).
Thus the change is to default to `parentbased_always_on` if
_get_sampler() returns None.
This solution was chosen as it requires the least amount of changes from
upstream.
- An alternative solution is to replace `_import_sampler()` method with
`_get_from_env_or_default()`, which would read from
`OTEL_TRACES_SAMPLER` env var a second time.
Ideally, upstream's `_get_sampler()` should default to
`parentbased_always_on`
*Testing:*
1. Before change, `unset OTEL_TRACES_SAMPLER`, see ValueError when
instrumenting `server_automatic_s3client.py`
2. After change, `unset OTEL_TRACES_SAMPLER`, no more error occurred.
Traces from `server_automatic_s3client.py` were recorded in XRay
console. Also tested with `OTEL_TRACES_SAMPLER=xray` to ensure xray
sampler still works.
By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.1 parent d0ab51a commit 96cd202
File tree
2 files changed
+17
-1
lines changed- aws-opentelemetry-distro
- src/amazon/opentelemetry/distro
- tests/amazon/opentelemetry/distro
2 files changed
+17
-1
lines changedLines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
169 | 169 | | |
170 | 170 | | |
171 | 171 | | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
172 | 180 | | |
173 | 181 | | |
174 | 182 | | |
| |||
aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_opentelementry_configurator.py
Lines changed: 9 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| |||
153 | 153 | | |
154 | 154 | | |
155 | 155 | | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
156 | 164 | | |
157 | 165 | | |
158 | 166 | | |
| |||
0 commit comments