-
Notifications
You must be signed in to change notification settings - Fork 523
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
[v2] update endpoints to separate intake from assets #1360
Conversation
beater/route_config.go
Outdated
rootURL = "/" | ||
HealthCheckURL = "/healthcheck" | ||
|
||
// intake v1 |
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.
Those URLs are also deprecated.
beater/route_config.go
Outdated
@@ -36,20 +36,28 @@ import ( | |||
) | |||
|
|||
var ( | |||
rootURL = "/" | |||
rootURL = "/" | |||
HealthCheckURL = "/healthcheck" |
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.
will also be deprecated, as rootUrl
is the new healthcheck URL.
Consider adding deprecation warnings already now for the old sourcemap urls that will be removed in v7 Edit: Maybe it doesn't make sense... anyways, just wanted to mention it |
Wasn't the sourcemap URL supposed to be |
set sourcemap url to |
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.
Didn't you have a deprecation warning somewhere in the old commits? Why did you remove that?
@@ -35,7 +35,7 @@ Example source map request | |||
|
|||
["source","sh",subs="attributes"] | |||
--------------------------------------------------------------------------- | |||
curl -X POST http://127.0.0.1:8200/v1/rum/sourcemaps \ | |||
curl -X POST http://127.0.0.1:8200/assets/v1/sourcemaps \ |
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'd expect to have a note about the deprecated URL somewhere in the docs. It is going to be part of a minor release, we should avoid simply removing deprecated but supported URLS.
I don't think there was a deprecation warning, but I'm happy to add one. |
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 think a deprecation notice in the docs would be nice as well. Besides that I'm 👍
FYI, there's a custom keyword you can use in asciidoc to show deprecation notices: deprecated[6.5.0,The URL for uploading sourcemaps was previously `/v1/sourcemaps`. It still works, but is being deprecated in favor of `/assets/v1/sourcemaps`.] I use it in the Node.js docs here: https://www.elastic.co/guide/en/apm/agent/nodejs/current/agent-api.html#apm-build-span |
But come to think of it, that might look weird in this case. I'm also fine with how it looks now :) |
@elastic/apm-agent-devs heads up: this changes the intake v2 urls |
New one is |
yes @mikker |
Closes #1336.