Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix http.host and net.peer.ip new http semconv mapping #2814

Merged
merged 12 commits into from
Aug 23, 2024
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2792](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2792))
- `opentelemetry-instrumentation-tornado` Handle http client exception and record exception info into span
([#2563](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2563))
- `opentelemetry-instrumentation` fix `http.host` new http semantic convention mapping to depend on `kind` of span
([2814](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2814))
lzchen marked this conversation as resolved.
Show resolved Hide resolved

## Version 1.26.0/0.47b0 (2024-07-23)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
_server_duration_attrs_new,
_server_duration_attrs_old,
_set_http_flavor_version,
_set_http_host,
_set_http_host_server,
_set_http_method,
_set_http_net_host_port,
_set_http_peer_ip,
_set_http_peer_ip_server,
_set_http_peer_port_server,
_set_http_scheme,
_set_http_target,
Expand Down Expand Up @@ -342,7 +342,7 @@ def collect_request_attributes(
if scheme:
_set_http_scheme(result, scheme, sem_conv_opt_in_mode)
if server_host:
_set_http_host(result, server_host, sem_conv_opt_in_mode)
_set_http_host_server(result, server_host, sem_conv_opt_in_mode)
if port:
_set_http_net_host_port(result, port, sem_conv_opt_in_mode)
flavor = scope.get("http_version")
Expand Down Expand Up @@ -380,7 +380,9 @@ def collect_request_attributes(
_set_http_user_agent(result, http_user_agent[0], sem_conv_opt_in_mode)

if "client" in scope and scope["client"] is not None:
_set_http_peer_ip(result, scope.get("client")[0], sem_conv_opt_in_mode)
_set_http_peer_ip_server(
result, scope.get("client")[0], sem_conv_opt_in_mode
)
_set_http_peer_port_server(
result, scope.get("client")[1], sem_conv_opt_in_mode
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@
from opentelemetry.semconv.attributes.network_attributes import (
NETWORK_PROTOCOL_VERSION,
)
from opentelemetry.semconv.attributes.server_attributes import (
SERVER_ADDRESS,
SERVER_PORT,
)
from opentelemetry.semconv.attributes.server_attributes import SERVER_PORT
from opentelemetry.semconv.attributes.url_attributes import (
URL_PATH,
URL_QUERY,
Expand Down Expand Up @@ -406,7 +403,6 @@ def validate_outputs(
HTTP_REQUEST_METHOD: "GET",
URL_SCHEME: "http",
SERVER_PORT: 80,
SERVER_ADDRESS: "127.0.0.1",
NETWORK_PROTOCOL_VERSION: "1.0",
URL_PATH: "/",
CLIENT_ADDRESS: "127.0.0.1",
Expand Down Expand Up @@ -442,7 +438,6 @@ def validate_outputs(
HTTP_REQUEST_METHOD: "GET",
URL_SCHEME: "http",
SERVER_PORT: 80,
SERVER_ADDRESS: "127.0.0.1",
NETWORK_PROTOCOL_VERSION: "1.0",
URL_PATH: "/",
CLIENT_ADDRESS: "127.0.0.1",
Expand Down Expand Up @@ -688,7 +683,7 @@ def test_behavior_with_scope_server_as_none_new_semconv(self):
def update_expected_server(expected):
expected[3]["attributes"].update(
{
SERVER_ADDRESS: "0.0.0.0",
CLIENT_ADDRESS: "0.0.0.0",
SERVER_PORT: 80,
}
)
Expand All @@ -715,7 +710,7 @@ def update_expected_server(expected):
SpanAttributes.HTTP_HOST: "0.0.0.0",
SpanAttributes.NET_HOST_PORT: 80,
SpanAttributes.HTTP_URL: "http://0.0.0.0/",
SERVER_ADDRESS: "0.0.0.0",
CLIENT_ADDRESS: "0.0.0.0",
SERVER_PORT: 80,
}
)
Expand Down Expand Up @@ -1001,7 +996,6 @@ def test_websocket_new_semconv(self):
"attributes": {
URL_SCHEME: self.scope["scheme"],
SERVER_PORT: self.scope["server"][1],
SERVER_ADDRESS: self.scope["server"][0],
NETWORK_PROTOCOL_VERSION: self.scope["http_version"],
URL_PATH: self.scope["path"],
CLIENT_ADDRESS: self.scope["client"][0],
Expand Down Expand Up @@ -1086,7 +1080,6 @@ def test_websocket_both_semconv(self):
SpanAttributes.HTTP_METHOD: self.scope["method"],
URL_SCHEME: self.scope["scheme"],
SERVER_PORT: self.scope["server"][1],
SERVER_ADDRESS: self.scope["server"][0],
NETWORK_PROTOCOL_VERSION: self.scope["http_version"],
URL_PATH: self.scope["path"],
CLIENT_ADDRESS: self.scope["client"][0],
Expand Down Expand Up @@ -1629,7 +1622,6 @@ def test_request_attributes_new_semconv(self):
attrs,
{
HTTP_REQUEST_METHOD: "GET",
SERVER_ADDRESS: "127.0.0.1",
URL_PATH: "/",
URL_QUERY: "foo=bar",
SERVER_PORT: 80,
Expand Down Expand Up @@ -1665,7 +1657,6 @@ def test_request_attributes_both_semconv(self):
SpanAttributes.NET_PEER_IP: "127.0.0.1",
SpanAttributes.NET_PEER_PORT: 32767,
HTTP_REQUEST_METHOD: "GET",
SERVER_ADDRESS: "127.0.0.1",
URL_PATH: "/",
URL_QUERY: "foo=bar",
SERVER_PORT: 80,
Expand All @@ -1690,7 +1681,7 @@ def test_query_string_new_semconv(self):
_HTTPStabilityMode.HTTP,
)
self.assertEqual(attrs[URL_SCHEME], "http")
self.assertEqual(attrs[SERVER_ADDRESS], "127.0.0.1")
self.assertEqual(attrs[CLIENT_ADDRESS], "127.0.0.1")
self.assertEqual(attrs[URL_PATH], "/")
self.assertEqual(attrs[URL_QUERY], "foo=bar")

Expand All @@ -1704,7 +1695,7 @@ def test_query_string_both_semconv(self):
attrs[SpanAttributes.HTTP_URL], "http://127.0.0.1/?foo=bar"
)
self.assertEqual(attrs[URL_SCHEME], "http")
self.assertEqual(attrs[SERVER_ADDRESS], "127.0.0.1")
self.assertEqual(attrs[CLIENT_ADDRESS], "127.0.0.1")
self.assertEqual(attrs[URL_PATH], "/")
self.assertEqual(attrs[URL_QUERY], "foo=bar")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ async def async_response_hook(span, request, response):
_OpenTelemetrySemanticConventionStability,
_OpenTelemetryStabilitySignalType,
_report_new,
_set_http_host,
_set_http_host_client,
_set_http_method,
_set_http_network_protocol_version,
_set_http_peer_port_client,
Expand Down Expand Up @@ -342,7 +342,7 @@ def _apply_request_client_attributes_to_span(
if _report_new(semconv):
if url.host:
# http semconv transition: http.host -> server.address
_set_http_host(span_attributes, url.host, semconv)
_set_http_host_client(span_attributes, url.host, semconv)
# http semconv transition: net.sock.peer.addr -> network.peer.address
span_attributes[NETWORK_PEER_ADDRESS] = url.host
if url.port:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def response_hook(span, request_obj, response)
_OpenTelemetryStabilitySignalType,
_report_new,
_report_old,
_set_http_host,
_set_http_host_client,
_set_http_method,
_set_http_net_peer_name_client,
_set_http_network_protocol_version,
Expand Down Expand Up @@ -212,14 +212,14 @@ def get_or_create_headers():
metric_labels, parsed_url.scheme, sem_conv_opt_in_mode
)
if parsed_url.hostname:
_set_http_host(
_set_http_host_client(
metric_labels, parsed_url.hostname, sem_conv_opt_in_mode
)
_set_http_net_peer_name_client(
metric_labels, parsed_url.hostname, sem_conv_opt_in_mode
)
if _report_new(sem_conv_opt_in_mode):
_set_http_host(
_set_http_host_client(
span_attributes,
parsed_url.hostname,
sem_conv_opt_in_mode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def response_hook(
_OpenTelemetryStabilitySignalType,
_report_new,
_report_old,
_set_http_host,
_set_http_host_client,
_set_http_method,
_set_http_net_peer_name_client,
_set_http_network_protocol_version,
Expand Down Expand Up @@ -491,7 +491,9 @@ def _set_metric_attributes(
sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT,
) -> None:

_set_http_host(metric_attributes, instance.host, sem_conv_opt_in_mode)
_set_http_host_client(
metric_attributes, instance.host, sem_conv_opt_in_mode
)
_set_http_scheme(metric_attributes, instance.scheme, sem_conv_opt_in_mode)
_set_http_method(
metric_attributes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he
_set_http_net_host,
_set_http_net_host_port,
_set_http_net_peer_name_server,
_set_http_peer_ip,
_set_http_peer_ip_server,
_set_http_peer_port_server,
_set_http_scheme,
_set_http_target,
Expand Down Expand Up @@ -360,7 +360,7 @@ def collect_request_attributes(

remote_addr = environ.get("REMOTE_ADDR")
if remote_addr:
_set_http_peer_ip(result, remote_addr, sem_conv_opt_in_mode)
_set_http_peer_ip_server(result, remote_addr, sem_conv_opt_in_mode)

peer_port = environ.get("REMOTE_PORT")
if peer_port:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,16 +252,32 @@ def _set_http_scheme(result, scheme, sem_conv_opt_in_mode):
set_string_attribute(result, URL_SCHEME, scheme)


def _set_http_host(result, host, sem_conv_opt_in_mode):
def _set_http_flavor_version(result, version, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(result, SpanAttributes.HTTP_HOST, host)
set_string_attribute(result, SpanAttributes.HTTP_FLAVOR, version)
if _report_new(sem_conv_opt_in_mode):
set_string_attribute(result, SERVER_ADDRESS, host)
set_string_attribute(result, NETWORK_PROTOCOL_VERSION, version)


def _set_http_user_agent(result, user_agent, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(
result, SpanAttributes.HTTP_USER_AGENT, user_agent
)
if _report_new(sem_conv_opt_in_mode):
set_string_attribute(result, USER_AGENT_ORIGINAL, user_agent)


# Client


def _set_http_host_client(result, host, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(result, SpanAttributes.HTTP_HOST, host)
if _report_new(sem_conv_opt_in_mode):
set_string_attribute(result, SERVER_ADDRESS, host)


def _set_http_net_peer_name_client(result, peer_name, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(result, SpanAttributes.NET_PEER_NAME, peer_name)
Expand Down Expand Up @@ -310,27 +326,32 @@ def _set_http_target(result, target, path, query, sem_conv_opt_in_mode):
set_string_attribute(result, URL_QUERY, query)


def _set_http_peer_ip(result, ip, sem_conv_opt_in_mode):
def _set_http_host_server(result, host, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(result, SpanAttributes.NET_PEER_IP, ip)
set_string_attribute(result, SpanAttributes.HTTP_HOST, host)
if _report_new(sem_conv_opt_in_mode):
set_string_attribute(result, CLIENT_ADDRESS, ip)
set_string_attribute(result, CLIENT_ADDRESS, host)
emdneto marked this conversation as resolved.
Show resolved Hide resolved


def _set_http_peer_port_server(result, port, sem_conv_opt_in_mode):
# net.peer.ip -> net.sock.peer.addr
# https://github.com/open-telemetry/semantic-conventions/blob/40db676ca0e735aa84f242b5a0fb14e49438b69b/schemas/1.15.0#L18
# net.sock.peer.addr -> client.socket.address for server spans (TODO) AND client.address if missing
# https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/CHANGELOG.md#v1210-2023-07-13
# https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/http-migration.md#common-attributes-across-http-client-and-server-spans
def _set_http_peer_ip_server(result, ip, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_int_attribute(result, SpanAttributes.NET_PEER_PORT, port)
set_string_attribute(result, SpanAttributes.NET_PEER_IP, ip)
if _report_new(sem_conv_opt_in_mode):
set_int_attribute(result, CLIENT_PORT, port)
# Only populate if not already populated
emdneto marked this conversation as resolved.
Show resolved Hide resolved
if not result.get(CLIENT_ADDRESS):
set_string_attribute(result, CLIENT_ADDRESS, ip)


def _set_http_user_agent(result, user_agent, sem_conv_opt_in_mode):
def _set_http_peer_port_server(result, port, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(
result, SpanAttributes.HTTP_USER_AGENT, user_agent
)
set_int_attribute(result, SpanAttributes.NET_PEER_PORT, port)
if _report_new(sem_conv_opt_in_mode):
set_string_attribute(result, USER_AGENT_ORIGINAL, user_agent)
set_int_attribute(result, CLIENT_PORT, port)


def _set_http_net_peer_name_server(result, name, sem_conv_opt_in_mode):
Expand All @@ -340,13 +361,6 @@ def _set_http_net_peer_name_server(result, name, sem_conv_opt_in_mode):
set_string_attribute(result, CLIENT_ADDRESS, name)


def _set_http_flavor_version(result, version, sem_conv_opt_in_mode):
if _report_old(sem_conv_opt_in_mode):
set_string_attribute(result, SpanAttributes.HTTP_FLAVOR, version)
if _report_new(sem_conv_opt_in_mode):
set_string_attribute(result, NETWORK_PROTOCOL_VERSION, version)


def _set_status(
span,
metrics_attributes: dict,
Expand Down