envoy
envoy copied to clipboard
json log with sort_properties prints decimals for all numbers
Title: json log with sort_properties prints decimals for all numbers
Description:
The json logs when using FileAccessLog are serialized as floating numbers with decimals rather than integers.
An example is response codes which are logged as "HTTP" 200.0.
We see this when json_format_options.sort_properties: true is set (introduced in https://github.com/envoyproxy/envoy/commit/fc33e5d94f6919f6748e468b75df607bdd667e24).
{"bytes_received":0.0,"bytes_sent":11576.0,"duration":6.0,"method":"GET","response_code":200.0,"upstream_service_time":"6"}...
Repro steps:
To reproduce, we use istio 1.22 (envoy b2dd15dc498b84431c063042e7821e179d5a3c41/1.30.2-dev) with the following configuration. The change in the format is json_format_options.sort_properties: true.
- name: envoy.filters.network.http_connection_manager
typed_config:
'@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
access_log:
- name: envoy.access_loggers.file
typed_config:
'@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog
log_format:
json_format:
authority: '%REQ(:AUTHORITY)%'
bytes_received: '%BYTES_RECEIVED%'
bytes_sent: '%BYTES_SENT%'
connection_termination_details: '%CONNECTION_TERMINATION_DETAILS%'
downstream_local_address: '%DOWNSTREAM_LOCAL_ADDRESS%'
downstream_remote_address: '%DOWNSTREAM_REMOTE_ADDRESS%'
duration: '%DURATION%'
method: '%REQ(:METHOD)%'
path: '%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%'
protocol: '%PROTOCOL%'
request_id: '%REQ(X-REQUEST-ID)%'
requested_server_name: '%REQUESTED_SERVER_NAME%'
response_code: '%RESPONSE_CODE%'
response_code_details: '%RESPONSE_CODE_DETAILS%'
response_flags: '%RESPONSE_FLAGS%'
route_name: '%ROUTE_NAME%'
start_time: '%START_TIME%'
upstream_cluster: '%UPSTREAM_CLUSTER%'
upstream_host: '%UPSTREAM_HOST%'
upstream_local_address: '%UPSTREAM_LOCAL_ADDRESS%'
upstream_service_time: '%RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)%'
upstream_transport_failure_reason: '%UPSTREAM_TRANSPORT_FAILURE_REASON%'
user_agent: '%REQ(USER-AGENT)%'
x_forwarded_for: '%REQ(X-FORWARDED-FOR)%'
json_format_options:
sort_properties: true
path: /dev/stdout
Note: The Envoy_collect tool gathers a tarball with debug logs, config and the following admin endpoints: /stats, /clusters and /server_info. Please note if there are privacy concerns, sanitize the data prior to sharing the tarball/pasting.
Admin and Stats Output:
Include the admin output for the following endpoints: /stats, /clusters, /routes, /server_info. For more information, refer to the admin endpoint documentation.
"version": "b2dd15dc498b84431c063042e7821e179d5a3c41/1.30.2-dev/Clean/RELEASE/BoringSSL",
"user_agent_name": "envoy",
"user_agent_build_version": {
"version": {
"major_number": 1,
"minor_number": 30,
"patch": 2
},
"metadata": {
"revision.sha": "b2dd15dc498b84431c063042e7821e179d5a3c41",
"ssl.version": "BoringSSL",
"build.label": "dev",
"revision.status": "Clean",
"build.type": "RELEASE"
}
},
Note: If there are privacy concerns, sanitize the data prior to sharing.
Config:
Include the config used to configure Envoy.
Logs:
Include the access logs and the Envoy logs.
Note: If there are privacy concerns, sanitize the data prior to sharing.
Call Stack:
If the Envoy binary is crashing, a call stack is required. Please refer to the Bazel Stack trace documentation.
This might be interesting for a beginner to take on. For json stats generation I wrote a fast json serializer that I think will not have this problem (and possibly be faster for logs): https://github.com/envoyproxy/envoy/blob/main/source/common/json/json_streamer.h
Not sure how easy it would be to incorporate into the access logger. But I'll call this a beginner project for any aspiring Envoy devs.
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.
This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.
Can we reopen this?
The issue seems to be this
JSON values can be one of 5 types namely, string, double, uint64_t, int64_t and bool as described over here
But we have defined formatter bases only for number(double) and string over here
I would like to work on it, please assign this to me.
Oh this might be solved by https://github.com/envoyproxy/envoy/pull/35545 -- @wbpcode WDYT?
All numbers is float now in the log formatter (and in the Protobuf::Value)
The key problem is the Nlohmann json lib. In the previous implemention of json formatter, if sort is required, the Nlohmann json will be used to generate the output. That lib will serialize the number according it's type strictly. So, the response code will be 200.0.
Oh this might be solved by https://github.com/envoyproxy/envoy/pull/35545 -- @wbpcode WDYT?
Yeah, I think so.
Just gave this a shot, and I still see the decimals using the JSON logger. Can I get this assigned to me?
sure but are you certain you are trying with #35545 in your build? Or are you using a release binary that might not include it?
I think maybe an earlier version of #35545 would have fixed this by inheriting the behavior from JsonStreamer:
https://github.com/envoyproxy/envoy/blob/520442a7e2b676ee7c710fcea26ae904be6abc65/source/common/json/json_streamer.h#L389
See the overrides for addNumber with integer args.
However I think we wound up diverging the json access logger from the json streamer in the final version, so you might need to duplicate the behavior of that overload.
It should be there, it was a checkout from this morning. I'll double check anyways.
Remember you still need to set a runtime flag for now. Our unit tests tell me it should be fixed.
Oh OK, I totally missed that. Will try with the flag when I have a chance.
I just verified with the flag (envoy.reloadable_fatures.logging_with_fast_json_formatter) and yes, it does fix the issue. From the comments in the code if I understand correctly this is supposed to launch on 1.32. Meanwhile, can we remove the "beginner" and "help wanted" tags maybe? Not sure if there's a more appropriate issue state (other than "open") for this. Thanks for all the help everyone.
1.32 on the way, shall we close this @wbpcode?
should be closed by #36629