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

Adding typed_json log format #5270

Merged
merged 4 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPENDENCIES.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ libraries:

Name Version License(s)
---- ------- ----------
Cython 0.29.36 Apache License 2.0
Cython 0.29.37 Apache License 2.0
Flask 3.0.0 3-clause BSD license
Jinja2 3.1.2 3-clause BSD license
MarkupSafe 2.1.3 3-clause BSD license
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/getambassador.io/v3alpha1/crd_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ type AmbassadorConfigSpec struct {
// run a custom lua script on every request. see below for more details.
LuaScripts string `json:"lua_scripts,omitempty"`

// +kubebuilder:validation:Enum={"text", "json"}
// +kubebuilder:validation:Enum={"text", "json", "typed_json"}
EnvoyLogType string `json:"envoy_log_type,omitempty"`

// envoy_log_path defines the path of log envoy will use. By default this is standard output
Expand Down
79 changes: 48 additions & 31 deletions python/ambassador/envoy/v3/v3listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,40 @@ def add_chain(

return chain

def json_helper(self) -> Any:
log_format = self.config.ir.ambassador_module.get("envoy_log_format", None)
if log_format is None:
log_format = {
"start_time": "%START_TIME%",
"method": "%REQ(:METHOD)%",
"path": "%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%",
"protocol": "%PROTOCOL%",
"response_code": "%RESPONSE_CODE%",
"response_flags": "%RESPONSE_FLAGS%",
"bytes_received": "%BYTES_RECEIVED%",
"bytes_sent": "%BYTES_SENT%",
"duration": "%DURATION%",
"upstream_service_time": "%RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)%",
"x_forwarded_for": "%REQ(X-FORWARDED-FOR)%",
"user_agent": "%REQ(USER-AGENT)%",
"request_id": "%REQ(X-REQUEST-ID)%",
"authority": "%REQ(:AUTHORITY)%",
"upstream_host": "%UPSTREAM_HOST%",
"upstream_cluster": "%UPSTREAM_CLUSTER%",
"upstream_local_address": "%UPSTREAM_LOCAL_ADDRESS%",
"downstream_local_address": "%DOWNSTREAM_LOCAL_ADDRESS%",
"downstream_remote_address": "%DOWNSTREAM_REMOTE_ADDRESS%",
"requested_server_name": "%REQUESTED_SERVER_NAME%",
"istio_policy_status": "%DYNAMIC_METADATA(istio.mixer:status)%",
"upstream_transport_failure_reason": "%UPSTREAM_TRANSPORT_FAILURE_REASON%",
}

tracing_config = self.config.ir.tracing
if tracing_config and tracing_config.driver == "envoy.tracers.datadog":
log_format["dd.trace_id"] = "%REQ(X-DATADOG-TRACE-ID)%"
log_format["dd.span_id"] = "%REQ(X-DATADOG-PARENT-ID)%"
return log_format

# access_log constructs the access_log configuration for this V3Listener
def access_log(self) -> List[dict]:
access_log: List[dict] = []
Expand Down Expand Up @@ -388,45 +422,28 @@ def access_log(self) -> List[dict]:

# Use sane access log spec in JSON
if self.config.ir.ambassador_module.envoy_log_type.lower() == "json":
log_format = self.config.ir.ambassador_module.get("envoy_log_format", None)
if log_format is None:
log_format = {
"start_time": "%START_TIME%",
"method": "%REQ(:METHOD)%",
"path": "%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%",
"protocol": "%PROTOCOL%",
"response_code": "%RESPONSE_CODE%",
"response_flags": "%RESPONSE_FLAGS%",
"bytes_received": "%BYTES_RECEIVED%",
"bytes_sent": "%BYTES_SENT%",
"duration": "%DURATION%",
"upstream_service_time": "%RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)%",
"x_forwarded_for": "%REQ(X-FORWARDED-FOR)%",
"user_agent": "%REQ(USER-AGENT)%",
"request_id": "%REQ(X-REQUEST-ID)%",
"authority": "%REQ(:AUTHORITY)%",
"upstream_host": "%UPSTREAM_HOST%",
"upstream_cluster": "%UPSTREAM_CLUSTER%",
"upstream_local_address": "%UPSTREAM_LOCAL_ADDRESS%",
"downstream_local_address": "%DOWNSTREAM_LOCAL_ADDRESS%",
"downstream_remote_address": "%DOWNSTREAM_REMOTE_ADDRESS%",
"requested_server_name": "%REQUESTED_SERVER_NAME%",
"istio_policy_status": "%DYNAMIC_METADATA(istio.mixer:status)%",
"upstream_transport_failure_reason": "%UPSTREAM_TRANSPORT_FAILURE_REASON%",
log_format = V3Listener.json_helper(self)
access_log.append(
{
"name": "envoy.access_loggers.file",
"typed_config": {
"@type": "type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog",
"path": self.config.ir.ambassador_module.envoy_log_path,
"json_format": log_format,
},
}
)

tracing_config = self.config.ir.tracing
if tracing_config and tracing_config.driver == "envoy.tracers.datadog":
log_format["dd.trace_id"] = "%REQ(X-DATADOG-TRACE-ID)%"
log_format["dd.span_id"] = "%REQ(X-DATADOG-PARENT-ID)%"

# Use sane access log spec in Typed JSON
elif self.config.ir.ambassador_module.envoy_log_type.lower() == "typed_json":
tenshinhigashi marked this conversation as resolved.
Show resolved Hide resolved
log_format = V3Listener.json_helper(self)
access_log.append(
{
"name": "envoy.access_loggers.file",
"typed_config": {
"@type": "type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog",
"path": self.config.ir.ambassador_module.envoy_log_path,
"json_format": log_format,
"typed_json_format": log_format,
},
}
)
Expand Down
45 changes: 44 additions & 1 deletion python/ambassador/envoy/v3/v3ready.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ def access_log(cls, config: "V3Config") -> List[dict]:
if tracing_config and tracing_config.driver == "envoy.tracers.datadog":
log_format["dd.trace_id"] = "%REQ(X-DATADOG-TRACE-ID)%"
log_format["dd.span_id"] = "%REQ(X-DATADOG-PARENT-ID)%"

access_log.append(
{
"name": "envoy.access_loggers.file",
Expand All @@ -152,6 +151,50 @@ def access_log(cls, config: "V3Config") -> List[dict]:
},
}
)
# Use sane access log spec in Typed JSON
elif config.ir.ambassador_module.envoy_log_type.lower() == "typed_json":
log_format = config.ir.ambassador_module.get("envoy_log_format", None)
if log_format is None:
log_format = {
"start_time": "%START_TIME%",
"method": "%REQ(:METHOD)%",
"path": "%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%",
"protocol": "%PROTOCOL%",
"response_code": "%RESPONSE_CODE%",
"response_flags": "%RESPONSE_FLAGS%",
"bytes_received": "%BYTES_RECEIVED%",
"bytes_sent": "%BYTES_SENT%",
"duration": "%DURATION%",
"upstream_service_time": "%RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)%",
"x_forwarded_for": "%REQ(X-FORWARDED-FOR)%",
"user_agent": "%REQ(USER-AGENT)%",
"request_id": "%REQ(X-REQUEST-ID)%",
"authority": "%REQ(:AUTHORITY)%",
"upstream_host": "%UPSTREAM_HOST%",
"upstream_cluster": "%UPSTREAM_CLUSTER%",
"upstream_local_address": "%UPSTREAM_LOCAL_ADDRESS%",
"downstream_local_address": "%DOWNSTREAM_LOCAL_ADDRESS%",
"downstream_remote_address": "%DOWNSTREAM_REMOTE_ADDRESS%",
"requested_server_name": "%REQUESTED_SERVER_NAME%",
"istio_policy_status": "%DYNAMIC_METADATA(istio.mixer:status)%",
"upstream_transport_failure_reason": "%UPSTREAM_TRANSPORT_FAILURE_REASON%",
}

tracing_config = config.ir.tracing
if tracing_config and tracing_config.driver == "envoy.tracers.datadog":
log_format["dd.trace_id"] = "%REQ(X-DATADOG-TRACE-ID)%"
log_format["dd.span_id"] = "%REQ(X-DATADOG-PARENT-ID)%"

access_log.append(
{
"name": "envoy.access_loggers.file",
"typed_config": {
"@type": "type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog",
"path": config.ir.ambassador_module.envoy_log_path,
"typed_json_format": log_format,
},
}
)
else:
# Use a sane access log spec
log_format = config.ir.ambassador_module.get("envoy_log_format", None)
Expand Down
13 changes: 12 additions & 1 deletion python/ambassador/ir/irambassador.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,20 @@ def finalize(self, ir: "IR", aconf: Config) -> bool:
)
self["envoy_log_format"] = {}
return False
elif self.get("envoy_log_type") == "typed_json":
if self.get("envoy_log_format", None) is not None and not isinstance(
self.get("envoy_log_format"), dict
):
self.post_error(
"envoy_log_type 'typed_json' requires a dictionary in envoy_log_format: {}, invalidating...".format(
self.get("envoy_log_format")
)
)
self["envoy_log_format"] = {}
return False
else:
self.post_error(
"Invalid log_type specified: {}. Supported: json, text".format(
"Invalid log_type specified: {}. Supported: json, text, typed_json".format(
self.get("envoy_log_type")
)
)
Expand Down
38 changes: 38 additions & 0 deletions python/tests/kat/t_envoy_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,41 @@ def check(self):
assert access_log_entry_regex.match(
line
), f"{line} does not match {access_log_entry_regex}"


class EnvoyLogTypeJSONTest(AmbassadorTest):
target: ServiceType
log_path: str

def init(self):
self.target = HTTP()
self.log_path = "/tmp/ambassador/ambassador.log"

def config(self) -> Generator[Union[str, Tuple[Node, str]], None, None]:
yield self, self.format(
"""
---
apiVersion: getambassador.io/v3alpha1
kind: Module
name: ambassador
ambassador_id: [{self.ambassador_id}]
config:
envoy_log_path: {self.log_path}
envoy_log_format:
protocol: "%PROTOCOL%"
duration: "%DURATION%"
envoy_log_type: typed_json
"""
)

def check(self):
access_log_entry_regex = re.compile('^({"duration":|{"protocol":)')

cmd = ShellCommand("tools/bin/kubectl", "exec", self.path.k8s, "cat", self.log_path)
if not cmd.check("check envoy access log"):
pytest.exit("envoy access log does not exist")

for line in cmd.stdout.splitlines():
assert access_log_entry_regex.match(
line
), f"{line} does not match {access_log_entry_regex}"
Loading