-
Notifications
You must be signed in to change notification settings - Fork 525
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
Disable sourcemap upload endpoint when data streams enabled #4735
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
Just one thing with the error message. Generally I'd like to minimise cognitive overhead for users by tailoring the error message to the running configuration.
beater/api/mux.go
Outdated
"If you are not using the RUM agent, you can safely ignore this error." | ||
enabled := cfg.RumConfig.IsEnabled() && cfg.RumConfig.SourceMapping.IsEnabled() | ||
"If you are not using the RUM agent, you can safely ignore this error. " + | ||
"If you are running APM Server managed by Fleet, you need to upload Sourcemaps directly to Elasticsearch." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We know whether or not APM Server is managed by Fleet - can we change the error message based on that knowledge? Saves the user trying to figure out whether source maps are explicitly disabled, or implicitly because data streams are enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just being lazy... you got me
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Steps errorsExpand to view the steps failures
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks :)
…chemas-to-agents * upstream/master: (111 commits) Introduce metricset.name (elastic#4857) processor/otel: test service.version handling (elastic#4853) docs: Add PHP agent information to shared docs (elastic#4740) Script for faster development workflow (elastic#4731) Update to elastic/beats@1b31c26 (elastic#4763) backport: add 7.12 to .backportrc.json (elastic#4807) backport: enable auto-merge on backport PRs (elastic#4777) Support for Node.js profiles (elastic#4728) docs: readds .NET link (elastic#4764) [DOCS] Fixes URLs on Secure communication with APM Agents page (elastic#4746) ci: set proper parameters for the tar step (elastic#4696) docs: add 7.11.1 release notes (elastic#4727) Disable sourcemap upload endpoint when data streams enabled (elastic#4735) Add service name to dataset field (elastic#4674) Update to elastic/beats@ba423212a660 (elastic#4733) sampling: require a default policy (elastic#4729) processor/otel: add unit test for span status (elastic#4734) Add support for consuming OTLP/gRPC metrics (elastic#4722) [apmpackage] Add config options supported in ESS (elastic#4690) Use the apm-server version everywhere* (elastic#4725) ...
…te-schema-json-1 * upstream/master: (111 commits) Introduce metricset.name (elastic#4857) processor/otel: test service.version handling (elastic#4853) docs: Add PHP agent information to shared docs (elastic#4740) Script for faster development workflow (elastic#4731) Update to elastic/beats@1b31c26 (elastic#4763) backport: add 7.12 to .backportrc.json (elastic#4807) backport: enable auto-merge on backport PRs (elastic#4777) Support for Node.js profiles (elastic#4728) docs: readds .NET link (elastic#4764) [DOCS] Fixes URLs on Secure communication with APM Agents page (elastic#4746) ci: set proper parameters for the tar step (elastic#4696) docs: add 7.11.1 release notes (elastic#4727) Disable sourcemap upload endpoint when data streams enabled (elastic#4735) Add service name to dataset field (elastic#4674) Update to elastic/beats@ba423212a660 (elastic#4733) sampling: require a default policy (elastic#4729) processor/otel: add unit test for span status (elastic#4734) Add support for consuming OTLP/gRPC metrics (elastic#4722) [apmpackage] Add config options supported in ESS (elastic#4690) Use the apm-server version everywhere* (elastic#4725) ...
Confirmed with BC2:
|
Also confirmed that the old message is returned from a standalone APM Server when RUM is disabled:
|
Motivation/summary
If we don't do this, users will see a
202 - Accepted
response, but apm-server will fail to index sourcemaps due to lack of permissions.Checklist
How to test these changes
Run apm-server with data streams enabled and send a POST request to
http://localhost:8200/assets/v1/sourcemaps
.The request should fail.