Add logs support to events intake API #9068
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lahsivjar
force-pushed
the
8757_logs_intake_tk2
branch
from
9d858b3 to
fd8fdfc
Compare
|
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 commentsTo re-run your PR in the CI, just comment with:
|
3 tasks
📚 Go benchmark reportDiff with the report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat |
felixbarny
reviewed
Sep 8, 2022
docs/spec/v2/log.json
Outdated
| "integer" | ||
| ] | ||
| }, | ||
| "span_id": { |
There was a problem hiding this comment.
Note that unlike traces and spans, logs are expected to be already in ECS format, as we'll use our ECS logging libraries to convert log events to JSON. So it shouldn't be span_id but span.id. This also applies to timestamp -> @timestamp, trace_id -> trace.id and others.
I think it will end up being much easier to not even have a dedicated spec for log. Just forward everything agents send to Elasticserach.
There was a problem hiding this comment.
Having said that, the global metadata ofc needs to be converted to ECS.
What I had in mind is that APM Server would just send a doc like
{
... global metadata
"message": "<whatever the agent has sent>"
}
And then use an ingest node pipeline to do a JSON decoding of the message field that merges the fields into the root of the doc.
Dynamic fields become an issue then. If we want to disallow dynamic fields in the first iteration, we can add dynamic: false to the mappings. However, we could also just add dynamic: runtime. This makes the fields searchable but avoids (some) mapping issues related to dynamic fields and can save space and index time for fields that are not commonly used in searches.
Other mechanisms to avoid issues with dynamic fields are to enable ignore_malformed for the whole data stream and to enable subobjects: true on the root level once elastic/elasticsearch#88934 has been resolved.
There was a problem hiding this comment.
I think it will end up being much easier to not even have a dedicated spec for log. Just forward everything agents send to Elasticserach.
If I understand correctly, for this we will need to have the ingest pipeline setup. We also have a requirement to keep the OTEL logs pipeline stable so I am a bit reluctant to introduce a lot of changes to the existing mapping. I am thinking that instead of using message field to put agent data we can use another named field that doesn't collide with any ECS schema field, maybe something like raw. This will allow us to incrementally migrate to the ingest node pipeline in the future. WDYT?
@axw Please correct me if I have misunderstood something here.
UPDATE: Thinking about this again, we can just let the message field be as it is sent to us and pass it on to ES. For some of the other fields, like trace_id and span_id, maybe we can keep the current structure and it can be overridden by the ingest node pipeline if message has the fields duplicated?
There was a problem hiding this comment.
maybe something like raw.
Or we can put everything else under labels as suggested by Andrew in the slack conversation.
There was a problem hiding this comment.
IIANM, for Lambda all we get is a timestamp and a string for function logs. Is that right? If so then I agree with the suggestions above to just send @timestamp and message, along with any fields that come from metadata.
For the first cut, I would suggest we just store message as it is received. Then we can follow up with what Felix suggests, and attempt to decode the message as JSON into the root, and use dynamic: runtime.
axw
reviewed
Sep 8, 2022
docs/spec/v2/log.json
Outdated
| "integer" | ||
| ] | ||
| }, | ||
| "span_id": { |
There was a problem hiding this comment.
IIANM, for Lambda all we get is a timestamp and a string for function logs. Is that right? If so then I agree with the suggestions above to just send @timestamp and message, along with any fields that come from metadata.
For the first cut, I would suggest we just store message as it is received. Then we can follow up with what Felix suggests, and attempt to decode the message as JSON into the root, and use dynamic: runtime.
axw
reviewed
Sep 8, 2022
axw
reviewed
Sep 9, 2022
This is done. |
simitt
approved these changes
Sep 9, 2022
|
@axw I missed adding |
axw
approved these changes
Sep 9, 2022
This was referenced Sep 9, 2022
6 tasks
|
What do you think about also adding the fields from the ECS logging spec? This way, we could use this as the foundation for https://github.com/elastic/apm-dev/issues/833 and merge elastic/apm-agent-java#2694 as an experimental feature. Dynamic mappings can come later. |
|
@felixbarny Thanks for the message. I have created #9100 to handle this, feel free to add any more information to it if required. I will try to push the required changes for 8.5. |
|
Verified with 8.5.0 BC1: $ curl -i -H "Authorization: Bearer 123" -H "Content-type: application/x-ndjson" --data-binary @testdata/intake-v2/logs.ndjson https://5fa0c22c644145439c4c4f9daa176b05.apm.us-west2.gcp.elastic-cloud.com:443/intake/v2/events
HTTP/2 202
date: Tue, 27 Sep 2022 07:21:56 GMT
x-cloud-request-id: AprFqdBaR7akPHcDaGQT-Q
x-content-type-options: nosniff
x-found-handling-cluster: 5fa0c22c644145439c4c4f9daa176b05
x-found-handling-instance: instance-0000000000
content-length: 0
I couldn't verify the log message with
|
|
Verified with the latest
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation/summary
Adds support for
logevent type to the intake API.Checklist
- [ ] Update package changelog.yml (only if changes toapmpackagehave been made)- [ ] Documentation has been updatedHow to test these changes
logevent type:{"metadata": {"user": {"domain": "ldap://abc", "id": "123", "email": "[email protected]", "username": "john"}, "process": {"ppid": 6789, "pid": 1234,"argv": ["node", "server.js"], "title": "node"}, "system": {"platform": "darwin", "hostname": "prod1.example.com", "architecture": "x64", "container": {"id": "container-id"}, "kubernetes": {"namespace": "namespace1", "pod": {"uid": "pod-uid", "name": "pod-name"}, "node": {"name": "node-name"}}}, "labels": {"tag1": "label1"}, "service": {"name": "testsvc", "language": {"version": "8", "name": "ecmascript"}, "agent": {"version": "3.14.0", "name": "elastic-node"}, "environment": "staging", "framework": {"version": "1.2.3", "name": "Express"}, "version": "5.1.3", "runtime": {"version": "8.0.0", "name": "node"}},"cloud":{"account":{"id":"account_id","name":"account_name"},"availability_zone":"cloud_availability_zone","instance":{"id":"instance_id","name":"instance_name"},"machine":{"type":"machine_type"},"project":{"id":"project_id","name":"project_name"},"provider":"cloud_provider","region":"cloud_region","service":{"name":"lambda"}}}} {"log": {"message": "test message"}} {"log": {"message": "test message 2", "timestamp": 1662616049000000}} {"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"}}}Related issues
Closes #8757