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

Upload sourcemaps synchronously #2862

Closed
simitt opened this issue Oct 25, 2019 · 3 comments
Closed

Upload sourcemaps synchronously #2862

simitt opened this issue Oct 25, 2019 · 3 comments
Assignees

Comments

@simitt
Copy link
Contributor

simitt commented Oct 25, 2019

Change sourcemap upload from asyncronous to syncronous. This can be achieved by using the new go-elasticsearch client.

@simitt simitt assigned simitt and unassigned simitt Oct 25, 2019
@simitt simitt self-assigned this Oct 25, 2019
@graphaelli graphaelli changed the title Upload sourcemaps syncronously Upload sourcemaps synchronously Oct 25, 2019
@simitt
Copy link
Contributor Author

simitt commented Oct 29, 2019

Working on this I realized we have some issues here:

  • Currently sourcemaps are uploaded as beats events, by using libbeat outputs. Therefore there is no direct requirement having ES configured when uploading sourcemaps. Switching to the new go-elasticsearch-client for uploading could be considered a breaking change, as it requires a configuration for Elasticsearch. The counter argument is for sourcemaps to be usable during ingestion one needs to configure either an ES output or a dedicated sourcemap ES instance. If none is configured the uploaded sourcemaps are of no value as they cannot be used. So technically the requirements change a bit, but for using sourcemaps overall requirements stay the same.
  • At the moment the sourcemap index is defined in the output.elasticsearch section. Users can can make use of index build selectors, e.g. use a different sourcemap index for different service names. The build selectors used by the ES output are not accessible from the APM Server, so they cannot easily be reused to figure out the proper index for the sourcemap. A kind of hackish way would be to initialize a new BuildSelector with the given ES output and use that for sourcemap upload. This could be error prone to future changes, as it needs to be implemented independent of the libbeat logic.

@simitt
Copy link
Contributor Author

simitt commented Oct 29, 2019

Even when sourcemaps can only be used when an ES endpoint is configured, this change would force users to permit APM Server write access to ES, even when using a different output for ingestion. Thanks @graphaelli for pointing this out.
Based on above mentioned issues, we consider this a breaking change and revisit for 8.0.0.

Another possibility is to remove functionality from APM Server and instead add it to the Kibana endpoint in 8.0.0 (cc @sqren ).

@axw
Copy link
Member

axw commented Dec 1, 2020

The current rough plan is to move source mapping out of APM Server altogether, and rely on ingest node (#3606) and uploading through Kibana. Closing this, we can reopen as needed.

@axw axw closed this as completed Dec 1, 2020
@axw axw removed the [zube]: Done label Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants