-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[APM] Version public APIs #158233
[APM] Version public APIs #158233
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
Pinging @elastic/apm-ui (Team:APM) |
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.
LGTM
@elasticmachine merge upstream |
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.
LGTM!
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
> | ||
)({ | ||
path: pathname, | ||
access: pathname.includes('/internal/apm') ? 'internal' : 'public', |
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.
Is seems that we rely more and more to the naming scheme to distinguish internal / public APIs, wondering if we can improve this. It's also mentioned here #151940
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.
What do you think needs improvement? You'd rather have an explicit access
prop on all routes?
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.
This might be an idea but not too precious about it
Closes elastic#155245 Adds a version to the `endpoint` name for public APIs. It's required for public APIs and enforced via type checks. Example: `GET /api/apm/settings/agent-configuration/environments 2023-05-22`. The reason why it's part of the endpoint is because it's the simplest change to make, requires very little runtime changes and doesn't dramatically change the way we handle type-safety for our server routes. --------- Co-authored-by: kibanamachine <[email protected]>
Closes #155245
Adds a version to the
endpoint
name for public APIs. It's required for public APIs and enforced via type checks. Example:GET /api/apm/settings/agent-configuration/environments 2023-05-22
. The reason why it's part of the endpoint is because it's the simplest change to make, requires very little runtime changes and doesn't dramatically change the way we handle type-safety for our server routes.