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

Nodejs agent sending undefined values in context #1873

Closed
simitt opened this issue Feb 4, 2019 · 5 comments
Closed

Nodejs agent sending undefined values in context #1873

simitt opened this issue Feb 4, 2019 · 5 comments
Milestone

Comments

@simitt
Copy link
Contributor

simitt commented Feb 4, 2019

So far I found that the nodejs agent sends following fields within context that have been stored for 6.x documents, but will be removed in 7.0 as of #411:

  • context.request.headers.accept,
  • context.request.headers.host

According to previous discussions no additional information should have been sent up by the nodejs agent (outside of context.custom). @watson and @Qard right now those fields will not be stored any more in 7.0, and they would also be removed from 6.x docs once they are reindexed. If they should be stored, we need to define them on API level.

@simitt simitt added this to the 7.0 milestone Feb 4, 2019
@roncohen
Copy link
Contributor

roncohen commented Feb 4, 2019

Users will probably expect all headers to show up. I suggest we move to change the JSON and APM Server handling of context.request.headers to accept and store arbitrary headers. We should probably still have a max length on the values.

@Qard
Copy link
Contributor

Qard commented Feb 4, 2019

Yeah, the current behaviour is to send everything. 👍 to not restricting the headers object.

@simitt
Copy link
Contributor Author

simitt commented Feb 4, 2019

@roncohen the headers are not going to be indexed. Usually we limit the values when they are indexed as keywords. What would be the motivation for limiting the length? When adding checks we would also need to define how to handle other value types (int, objects, ..).
Since we haven't had any restrictions on headers, this would be a breaking change and could only be introduced with a new API version.
The handling that would be closest to what is currently released would be to simply store whatever is sent without any restrictions. I suggest we move forward with this now and consider limitations when working on a new API version.

@roncohen
Copy link
Contributor

roncohen commented Feb 4, 2019

Ok. Sounds good @Silvia

simitt added a commit to simitt/apm-server that referenced this issue Feb 4, 2019
Remove checks on Intake API and do not move or parse header fields.

fixes elastic#1873
@simitt
Copy link
Contributor Author

simitt commented Feb 5, 2019

resolved in #1870

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

4 participants