grafana / beyla

eBPF-based autoinstrumentation of web applications and network metrics
https://grafana.com/oss/beyla-ebpf/
Apache License 2.0
1.45k stars 102 forks source link

Wrong client_address/server_address in application client/server metrics(http_server_request_duration_seconds_bucket) #921

Open sudhi1991-vi opened 5 months ago

sudhi1991-vi commented 5 months ago

We are getting deployment name in client_address in server metrics in latest beyla image. http_server_request_duration_seconds_bucket{client_address="opscruise-dev1-promgw", instance="10.10.101.117:8090", job="oc-kubernetes-pods", k8s_namespace_name="dev3", k8s_node_name="w04-md-w04-85c7bbd88dx8cf5x-d9hs5", k8s_pod_name="chronix-consumer-hour-5f856676b-tgvlf", k8s_replicaset_name="chronix-consumer-hour-5f856676b", neinstip="10.10.101.117", service_name="chronix-consumer-hour", service_namespace="dev3"}

wrong server_address in client metrics too "a7de0457831fd11f7.awsglobalaccelerator.com" http_client_request_duration_seconds_count{http_request_method="GET", http_response_status_code="408", http_route="/tests", instance="10.10.101.123:8090", job="oc-kubernetes-pods", k8s_daemonset_name="beyla", k8s_namespace_name="beyla", k8s_node_name="w04-md-w04-85c7bbd88dx8cf5x-jw5wn", k8s_pod_name="beyla-fd8bp", k8s_pod_start_time="2024-06-04 13:01:19 +0000 UTC", k8s_pod_uid="89befede-d858-4419-88a4-be8ebadd22a0", neinstip="10.10.101.123", server_address="a7de0457831fd11f7.awsglobalaccelerator.com", server_port="80", service_name="beyla", service_namespace="beyla", url_path="/tests"}

expecting pod/service ips instead.

grcevski commented 5 months ago

Thanks for reporting this @sudhi1991, I think we need to split those attributes, address and name.

angu2015 commented 5 months ago

It look like a breakage of existing behaviour. in 1.5.2 the value was IP address for client_address label in httpserver. metrics and for Server_address in httpclient. metrics.

grcevski commented 5 months ago

According to the OpenTelemetry semantic convention the name is preferred, which is why this change was made. We weren't able to determine the name until recently and this is simply to comply better. https://opentelemetry.io/docs/specs/semconv/http/http-metrics/

server.address string Name of the local HTTP server that received the request.

In absence of a name we are to provide the IP address.

The best we can do here is to provide a way to control the behaviour, e.g. let you choose if you prefer the IP address instead of the name, because for a lot of folks the name is the preferred choice.

angu2015 commented 5 months ago

Thanks for the clarification @grcevski and also giving an option based on preference for IP address or name. it would be good if we split the attributes, address and name as you mentioned earlier. Since name is the preferred one, address can be included/excluded based on user preference for both client(http/rpc) and server(http/rpc) metrics.

sudhi1991-vi commented 5 months ago

@grcevski Agreed that there should be option to either give the IP address or name of the server that receives the request. This is acceptable in the 'http_client/rpc_client...' metrics for the label ''server_address". Even though there exists a cardinality risk in using the 'Host: ' header for this purpose.

Other part of the issue is, why should you mention the 'deployment' name in 'client_address' label of http_server../rpc_server.. metric ? It should be the ip address of the client from which the request was received by the server. This is a standard practice to show client ip address in http server access logs. Can you fix this issue on http_server... metric ?"