open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
705 stars 589 forks source link

requests instrumentation does not provide net.peer.name or net.peer.port to spans #2138

Open thpierce opened 8 months ago

thpierce commented 8 months ago

Describe your environment

Steps to reproduce

requests.get('https://0.0.0.0:8443/some/path')

* `pip install requests opentelemetry-distro`
* `opentelemetry-bootstrap -a install`
* `opentelemetry-instrument --traces_exporter console --metrics_exporter console --service_name my_service python sample.py`

**What is the expected behavior?**
One client span and one `http.client.duration` metric, both with the following attributes (and others, not relevant):

"net.peer.name": "0.0.0.0", "net.peer.name": "8443",


**What is the actual behavior?**

Metric attributes contains `net.peer.*`, but span attributes does not:

{ "resource_metrics": [ { "resource": { "attributes": { "telemetry.sdk.language": "python", "telemetry.sdk.name": "opentelemetry", "telemetry.sdk.version": "1.22.0", "service.name": "my_service", "telemetry.auto.version": "0.43b0" }, "schema_url": "" }, "scope_metrics": [ { "scope": { "name": "opentelemetry.instrumentation.requests", "version": "0.43b0", "schema_url": "https://opentelemetry.io/schemas/1.11.0" }, "metrics": [ { "name": "http.client.duration", "description": "measures the duration of the outbound HTTP request", "unit": "ms", "data": { "data_points": [ { "attributes": { "http.method": "GET", "http.scheme": "https", "http.host": "0.0.0.0", "net.peer.name": "0.0.0.0", "net.peer.port": 8443 }, ... } ], "aggregation_temporality": 2 } } ], "schema_url": "https://opentelemetry.io/schemas/1.11.0" } ], "schema_url": "" } ] } { "name": "GET", "context": { "trace_id": "0x7e5965f4350d2ff717908d358ee8e55a", "span_id": "0x20ef5ac593251efa", "trace_state": "[]" }, "kind": "SpanKind.CLIENT", "parent_id": null, "start_time": "2024-01-26T18:57:59.679256Z", "end_time": "2024-01-26T18:57:59.689908Z", "status": { "status_code": "ERROR", "description": "ConnectionError: HTTPSConnectionPool(host='0.0.0.0', port=8443): Max retries exceeded with url: /some/path (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x105958700>: Failed to establish a new connection: [Errno 61] Connection refused'))" }, "attributes": { "http.method": "GET", "http.url": "https://0.0.0.0:8443/some/path" }, "events": [ ... ], "links": [], "resource": { "attributes": { "telemetry.sdk.language": "python", "telemetry.sdk.name": "opentelemetry", "telemetry.sdk.version": "1.22.0", "service.name": "my_service", "telemetry.auto.version": "0.43b0" }, "schema_url": "" } }



**Additional context**
This is happening because we explicitly only set these values for metric attributes [here](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/f2c536ed823c1cd6df0be5136112ded7384fa105/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py#L181) and [here](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/f2c536ed823c1cd6df0be5136112ded7384fa105/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py#L195). I believe that these span attributes were replaced with `network.peer.*` attributes as a part of the HTTP semconv updates, but these are only set [here](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/f2c536ed823c1cd6df0be5136112ded7384fa105/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py#L191) and [here](https://github.com/open-telemetry/opentelemetry-python-contrib/blob/f2c536ed823c1cd6df0be5136112ded7384fa105/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py#L203) using the new values happens only if `_report_new(sem_conv_opt_in_mode)`, but when `_report_old(sem_conv_opt_in_mode)` we are not setting the old values.
thpierce commented 8 months ago

I think specifically what I am asking is the following - can we something to the effect of:

if _report_old(sem_conv_opt_in_mode)
    _set_http_net_peer_name(span_attributes, parsed_url.hostname, sem_conv_opt_in_mode)
    _set_http_port(span_attributes, parsed_url.port, sem_conv_opt_in_mode)

to instrumented_send?