ppkarwasz commented on code in PR #3586:
URL: https://github.com/apache/logging-log4j2/pull/3586#discussion_r2024738244
##########
log4j-layout-template-json/src/main/resources/GcpLayout.json:
##########
@@ -37,8 +42,9 @@
}
},
"logging.googleapis.com/insertId": {
- "$resolver": "counter",
- "stringified": true
+ "$resolver": "pattern",
+ "pattern": "%uuid{TIME}",
+ "stackTraceEnabled": false
Review Comment:
As discussed in #3585, omitting the field entirely or keeping `counter` is
probably the right way to go.
##########
log4j-layout-template-json/src/main/resources/GcpLayout.json:
##########
@@ -1,13 +1,18 @@
{
- "timestamp": {
+ "timestampSeconds": {
"$resolver": "timestamp",
- "pattern": {
- "format": "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'",
- "timeZone": "UTC",
- "locale": "en_US"
+ "epoch": {
+ "unit": "secs",
+ "rounded": true
}
},
- "severity": {
+ "timestampNanos": {
+ "$resolver": "timestamp",
+ "epoch": {
+ "unit": "secs.nanos"
+ }
+ },
Review Comment:
Looking at the [timestamp processing
documentation](https://cloud.google.com/logging/docs/agent/logging/configuration#timestamp-processing),
it is probably better to use a `timestamp` field, which is checked first:
```json
"timestamp": {
"seconds": {
"$resolver": "timestamp",
"epoch": {
"unit": "secs",
"rounded": true
}
},
"nanos": {
"$resolver": "timestamp",
"epoch": {
"unit": "secs.nanos"
}
}
}
```
##########
log4j-layout-template-json/src/main/resources/GcpLayout.json:
##########
@@ -49,25 +55,15 @@
"key": "span_id"
},
"logging.googleapis.com/trace_sampled": true,
- "_exception": {
- "class": {
- "$resolver": "exception",
- "field": "className"
- },
- "message": {
- "$resolver": "exception",
- "field": "message"
- },
- "stackTrace": {
- "$resolver": "pattern",
- "pattern": "%xEx"
- }
+ "exception": {
+ "$resolver": "pattern",
+ "pattern": "%xEx"
},
Review Comment:
Do you have a reference for the `exception` field? I only found [Format
errors in
logs](https://cloud.google.com/error-reporting/docs/formatting-error-messages#format-log-entry),
which checks for `stack_trace` first.
```json
"stack_trace": {
"$resolver": "exception",
"field": "stackTrace",
"stackTrace": {
"stringified": true
}
}
```
seems like a better choice.
##########
log4j-layout-template-json/src/main/resources/GcpLayout.json:
##########
@@ -1,13 +1,18 @@
{
- "timestamp": {
+ "timestampSeconds": {
"$resolver": "timestamp",
- "pattern": {
- "format": "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'",
- "timeZone": "UTC",
- "locale": "en_US"
+ "epoch": {
+ "unit": "secs",
+ "rounded": true
}
},
- "severity": {
+ "timestampNanos": {
+ "$resolver": "timestamp",
+ "epoch": {
+ "unit": "secs.nanos"
+ }
+ },
+ "logging.googleapis.com/severity": {
Review Comment:
Google's documentation is not coherent here:
- [Structured
logging](https://cloud.google.com/logging/docs/structured-logging#structured_logging_special_fields)
calls the field `severity`.
- [Configure the Ops
Agent](https://cloud.google.com/logging/docs/agent/ops-agent/configuration#special-fields)
calls the field `logging.googleapis.com/severity`.
Which one is the correct one?
##########
log4j-layout-template-json/src/main/resources/GcpLayout.json:
##########
@@ -49,25 +55,15 @@
"key": "span_id"
},
"logging.googleapis.com/trace_sampled": true,
- "_exception": {
- "class": {
- "$resolver": "exception",
- "field": "className"
- },
- "message": {
- "$resolver": "exception",
- "field": "message"
- },
- "stackTrace": {
- "$resolver": "pattern",
- "pattern": "%xEx"
- }
+ "exception": {
+ "$resolver": "pattern",
+ "pattern": "%xEx"
},
- "_thread": {
+ "thread": {
"$resolver": "thread",
"field": "name"
},
- "_logger": {
+ "logger": {
Review Comment:
Those field start intentionally with an underscore `_`, so they don't
collide with payload field that Google might introduce in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
