opentelemetry-python-contrib icon indicating copy to clipboard operation
opentelemetry-python-contrib copied to clipboard

Fix client address is set to server address in new semconv

Open y-young opened this issue 8 months ago • 5 comments
trafficstars

Description

With OTEL_SEMCONV_STABILITY_OPT_IN=http, the client.address in server span is wrongly set to server address.

It it because the middleware calls _set_http_host_server and the client.address is set for the first time to server address here:

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/3708604bb5df303b890155f7367c2c1b2bbe9f01/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py#L357

When it calls _set_http_peer_ip_server later on, since the client.address is already set, this has no effect:

https://github.com/open-telemetry/opentelemetry-python-contrib/blob/3708604bb5df303b890155f7367c2c1b2bbe9f01/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py#L368-L371

This was changed in #2814.

Fixes #3356.

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Tested with a minimal FastAPI application: https://github.com/y-young/opentelemetry-fastapi-test

Without patch

Without OTEL_SEMCONV_STABILITY_OPT_IN

Scope: {'type': 'http', 'asgi': {'version': '3.0', 'spec_version': '2.3'}, 'http_version': '1.1', 'server': ('127.0.0.1', 8000), 'client': ('127.0.0.1', 54737), 'scheme': 'http', 'root_path': ''}

Span:

{
    "name": "GET /",
    "context": {
        "trace_id": "0x3d94c2edf94a8c68e66a261883939a70",
        "span_id": "0x26a8e38e79afe0ae",
        "trace_state": "[]"
    },
    "kind": "SpanKind.SERVER",
    "parent_id": null,
    "start_time": "2025-03-11T14:42:33.575716Z",
    "end_time": "2025-03-11T14:42:33.582507Z",
    "status": {
        "status_code": "UNSET"
    },
    "attributes": {
        "http.scheme": "http",
        "http.host": "127.0.0.1:8000",
        "net.host.port": 8000,
        "http.flavor": "1.1",
        "http.target": "/",
        "http.url": "http://127.0.0.1:8000/",
        "http.method": "GET",
        "http.server_name": "127.0.0.1:8000",
        "http.user_agent": "",
        "net.peer.ip": "127.0.0.1",
        "net.peer.port": 54737,
        "http.route": "/",
        "http.status_code": 200
    },
    "events": [],
    "links": [],
    "resource": {
        "attributes": {
            "service.name": "test-app"
        },
        "schema_url": ""
    }
}

The net.peer.ip and net.peer.port here is correct.

With OTEL_SEMCONV_STABILITY_OPT_IN

Set os.environ["OTEL_SEMCONV_STABILITY_OPT_IN"] = "http" and restart the app:

Scope: {'type': 'http', 'asgi': {'version': '3.0', 'spec_version': '2.3'}, 'http_version': '1.1', 'server': ('127.0.0.1', 8000), 'client': ('127.0.0.1', 50644), 'scheme': 'http', 'root_path': ''}

Span:

{
    "name": "GET /",
    "context": {
        "trace_id": "0xc37fa0f9c920df31a3f6966771f70ce4",
        "span_id": "0x8799fdefa09e60b1",
        "trace_state": "[]"
    },
    "kind": "SpanKind.SERVER",
    "parent_id": null,
    "start_time": "2025-03-11T14:39:09.180212Z",
    "end_time": "2025-03-11T14:39:09.185639Z",
    "status": {
        "status_code": "UNSET"
    },
    "attributes": {
        "url.scheme": "http",
        "client.address": "127.0.0.1:8000",
        "server.port": 8000,
        "network.protocol.version": "1.1",
        "url.path": "/",
        "http.request.method": "GET",
        "user_agent.original": "",
        "client.port": 50644,
        "http.route": "/",
        "http.response.status_code": 200
    },
    "events": [],
    "links": [],
    "resource": {
        "attributes": {
            "service.name": "test-app"
        },
        "schema_url": ""
    }
}

The client.address is set to 127.0.0.1:8000, which is the server address.

Install patched local version

uv add ../opentelemetry-python-contrib

Scope: {'type': 'http', 'asgi': {'version': '3.0', 'spec_version': '2.3'}, 'http_version': '1.1', 'server': ('127.0.0.1', 8000), 'client': ('127.0.0.1', 62487), 'scheme': 'http', 'root_path': ''}

Span:

{
    "name": "GET /",
    "context": {
        "trace_id": "0xc669651b2543f96d4abc362360ff76da",
        "span_id": "0x79ed73755c5bee36",
        "trace_state": "[]"
    },
    "kind": "SpanKind.SERVER",
    "parent_id": null,
    "start_time": "2025-03-11T14:33:58.230421Z",
    "end_time": "2025-03-11T14:33:58.236676Z",
    "status": {
        "status_code": "UNSET"
    },
    "attributes": {
        "url.scheme": "http",
        "server.address": "127.0.0.1:8000",
        "server.port": 8000,
        "network.protocol.version": "1.1",
        "url.path": "/",
        "http.request.method": "GET",
        "user_agent.original": "",
        "client.address": "127.0.0.1",
        "client.port": 62487,
        "http.route": "/",
        "http.response.status_code": 200
    },
    "events": [],
    "links": [],
    "resource": {
        "attributes": {
            "service.name": "test-app"
        },
        "schema_url": ""
    }
}

The client.address is correct now.

Does This PR Require a Core Repo Change?

  • [ ] Yes. - Link to PR:
  • [x] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • [x] Followed the style guidelines of this project
  • [x] Changelogs have been updated
  • [ ] Unit tests have been added
  • [x] Documentation has been updated

y-young avatar Mar 11 '25 14:03 y-young