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

Consider flattening headers and cookies #13023

Open
axw opened this issue Apr 22, 2024 · 8 comments
Open

Consider flattening headers and cookies #13023

axw opened this issue Apr 22, 2024 · 8 comments

Comments

@axw
Copy link
Member

axw commented Apr 22, 2024

See elastic/ecs#2333 (comment)

@felixbarny
Copy link
Member

IIRC, we didn't always index all headers. When and why did we change that? Or did we always do that?

@axw
Copy link
Member Author

axw commented Apr 23, 2024

The change was made in #12102. The idea was to align with how we would deal with OpenTelemetry semantic conventions in the future, which would involve dynamically mapping headers.

We could revert to disabling indexing for headers, or switch to flattening them. Either way it's a special case, and not a general solution to avoid hitting field limits.

@felixbarny
Copy link
Member

Interestingly, that change used the flattened field type for HTTP headers

http.{request,response}.headers have been changed from object to flattened.

Did this change to dynamically indexing as keywords in the context of the apm-data plugin in Elasticsearch?

@axw
Copy link
Member Author

axw commented Apr 23, 2024

@felixbarny sorry, yes, I should have mentioned earlier: I ended up removing that particular use of flattened and switched it to being dynamically mapped to keywords, to match what we would end up doing with OTel.

@felixbarny
Copy link
Member

Both for OTel and for non-OTel, I think we should use flattened for HTTP headers.

@axw
Copy link
Member Author

axw commented May 3, 2024

I think that should be fine. OTOH we've found that using flattened for stack traces is causing problems for ingesting exceptions with huge amounts of source context or deeply nested vars, so we'll probably revert to disabling there.

@axw
Copy link
Member Author

axw commented May 3, 2024

@axw
Copy link
Member Author

axw commented May 3, 2024

We'll also need to bring those changes across to the integration package for 8.15.0. We should be pretty close to switching to apm-data, but I think it may be safer to wait until 8.16.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants