Add ECS log files to the intake log endpoint #9349
Conversation
|
This pull request does not have a backport label. Could you fix it @lahsivjar? 🙏
NOTE: |
mergify
bot
added
the
backport-skip
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
There was a problem hiding this comment.
Looks good, just left a couple of suggestions.
Handling maybe-flattened-maybe-not JSON will be interesting. I haven't looked into using it myself before, but json-iterator has an extension API which might be useful? https://pkg.go.dev/github.com/json-iterator/go#Extension
@axw Thanks for this. I was not aware of this and my searches also came up empty, it sounds way better than what I am doing right now which is introducing a map to decode the nested structure into and finally merging them with the flat decoded values via generated code. Something like this: type ecsLogService struct {
NestedStruct map[string]interface{} `json:"service" flattenSource:"true"`
ServiceName nullable.String `json:"service.name" validate:"maxLength=1024"`
ServiceVersion nullable.String `json:"service.version" validate:"maxLength=1024"`
ServiceEnvironment nullable.String `json:"service.environment" validate:"maxLength=1024"`
ServiceNodeName nullable.String `json:"service.node.name" validate:"maxLength=1024"`
}Finally, the generator will scan the |
📚 Go benchmark reportDiff with the report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat |
|
Another edge case that's worth discussing is how to deal with duplicate keys. As ECS loggers allow users to add arbitrary keys at the top level, the resulting JSON may contain duplicate keys, such as |
|
When testing this PR with Java agent draft PR, I get a parsing error on the It's quite easy to reproduce with the following payload: |
|
Great point. Yeah, the Thanks for testing this @SylvainJuge 👏 |
|
Thanks, @SylvainJuge, and @felixbarny for the proactive feedback and testing. Currently, APM-Server only supports UPDATE: I have added a PR #9376 to add support for string timestamp. For now, I have only added format |
|
Yes, I think |
|
@felixbarny Thanks for the clarification. Based on this doc, IIUC, |
|
Yes, that's correct. |
|
This pull request is now in conflicts. Could you fix it @lahsivjar? 🙏 |
docs/spec/v2/log.json
Outdated
| ], | ||
| "maxLength": 1024 | ||
| }, | ||
| "ecslogerrorfields": { |
There was a problem hiding this comment.
I wonder if we should just not produce JSON Schema for logs for now? IIRC that's what @felixbarny suggested earlier. Otherwise we should probably ensure the embedded structs don't affect the schema.
There was a problem hiding this comment.
I wonder if we should just not produce JSON Schema for logs for now?
How are the generated specs utilized? FWIW, I was planning on fixing the embedded struct handling in the schema generation code but if we don't have a use case for the schema or if it is not required for the logs then removing it sounds good to me.
There was a problem hiding this comment.
At least some of the agents use the schema for payload validation in tests. We used to use it in APM Server, before we generated the validation code. Let's drop it for now and come back to it if agents want to start validating logs payloads.
There was a problem hiding this comment.
Thanks @axw, I have removed the JSON spec generation for now.
|
@SylvainJuge Thanks for the earlier testing. Whenever you have time can you test again with the JAVA agent as all the required PRs are now merged. |
I just did a quick test and it seems that not all "string-based" timestamps formats are supported, see #9376 (comment) for an example. |
Motivation/summary
Add ECS logging fields to intake API's log endpoint.
Checklist
- [ ] Update package changelog.yml (only if changes toapmpackagehave been made)How to test these changes
logevent type:{"metadata":{"process":{"pid":1234,"title":"/usr/lib/jvm/java-10-openjdk-amd64/bin/java","ppid":1,"argv":["-v"]},"system":{"architecture":"amd64","detected_hostname":"8ec7ceb99074","configured_hostname":"host1","platform":"Linux","container":{"id":"8ec7ceb990749e79b37f6dc6cd3628633618d6ce412553a552a0fa6b69419ad4"},"kubernetes":{"namespace":"default","pod":{"uid":"b17f231da0ad128dc6c6c0b2e82f6f303d3893e3","name":"instrumented-java-service"},"node":{"name":"node-name"}}},"service":{"name":"1234_service-12a3","version":"4.3.0","node":{"configured_name":"8ec7ceb990749e79b37f6dc6cd3628633618d6ce412553a552a0fa6b69419ad4"},"environment":"production","language":{"name":"Java","version":"10.0.2"},"agent":{"version":"1.10.0","name":"java","ephemeral_id":"e71be9ac-93b0-44b9-a997-5638f6ccfc36"},"framework":{"name":"spring","version":"5.0.0"},"runtime":{"name":"Java","version":"10.0.2"}},"labels":{"group":"experimental","ab_testing":true,"segment":5}}} {"log": {"message": "test log message without timestamp"}} {"log": {"message": "test log message with timestamp", "@timestamp": 1662616049000000}} {"log": {"message": "test log message with string timestamp", "@timestamp": "2022-09-08T13:47:29.111-0800"}} {"log": {"message": "test log message with faas", "@timestamp": 1662616049000000,"faas":{"coldstart":true,"execution":"6f7f0961f83442118a7af6fe80b88d56","id":"arn:aws:lambda:us-east-2:123456789012:function:custom-runtime"}}} {"log": {"trace.id": "trace-id", "transaction.id": "txn-id", "log.level": "warn", "log.logger": "testLogger", "log.origin.file.name": "testFile", "log.origin.file.line": 10, "log.origin.function": "testFunc", "service.name": "testSvc", "service.version": "v1.0.0", "service.environment": "prod", "process.thread.name": "testThread", "dataset": "accesslog", "labels": {"str": "str", "int": 1, "float": 1.1, "bool": true}, "message": "test log message with faas and ecs fields", "@timestamp": 1662616049000000,"faas":{"coldstart":true,"execution":"6f7f0961f83442118a7af6fe80b88d56","id":"arn:aws:lambda:us-east-2:123456789012:function:custom-runtime"}}}Related issues
Closes #9100