-
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
Api Deprecations #193668
Api Deprecations #193668
Conversation
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
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.
@Bamieh some small changes I think could be worth adding
diff
diff --git a/packages/core/http/core-http-server-internal/src/http_service.ts b/packages/core/http/core-http-server-internal/src/http_service.ts
index 2861652976f..b0674c1e546 100644
--- a/packages/core/http/core-http-server-internal/src/http_service.ts
+++ b/packages/core/http/core-http-server-internal/src/http_service.ts
@@ -182,7 +182,7 @@ export class HttpService
this.internalSetup = {
...serverContract,
- registerOnPostValidation: (cb) => {
+ registerOnPostValidationListener: (cb) => {
Router.on('onPostValidate', cb);
},
diff --git a/packages/core/http/core-http-server-internal/src/types.ts b/packages/core/http/core-http-server-internal/src/types.ts
index bd4160d4d4b..cabf60b622b 100644
--- a/packages/core/http/core-http-server-internal/src/types.ts
+++ b/packages/core/http/core-http-server-internal/src/types.ts
@@ -66,7 +66,7 @@ export interface InternalHttpServiceSetup
contextName: ContextName,
provider: IContextProvider<Context, ContextName>
) => IContextContainer;
- registerOnPostValidation(cb: (req: CoreKibanaRequest) => void): void;
+ registerOnPostValidationListener(cb: (req: CoreKibanaRequest) => void): void;
getRegisteredDeprecatedApis: () => any[];
}
diff --git a/packages/core/http/core-http-server/src/router/route.ts b/packages/core/http/core-http-server/src/router/route.ts
index 5de6d590880..c3d48c81309 100644
--- a/packages/core/http/core-http-server/src/router/route.ts
+++ b/packages/core/http/core-http-server/src/router/route.ts
@@ -130,9 +130,10 @@ export interface RouteInputDeprecationRemovedDescription {
}
/** @public */
-export type RouteInputDeprecationDescription =
+export type RouteInputDeprecationDescription = { documentationLink?: string } & (
| RouteInputDeprecationRenamedDescription
- | RouteInputDeprecationRemovedDescription;
+ | RouteInputDeprecationRemovedDescription
+);
/**
* Factory for supported types of route API deprecations.
@@ -152,6 +153,10 @@ export interface RouteInputDeprecation {
body?: RouteInputDeprecationFactory;
}
+export interface RouteDeprecationLink {
+ documentationLink?: string;
+}
+
/**
* Description of deprecations for this HTTP API.
*
@@ -177,7 +182,7 @@ export interface RouteInputDeprecation {
* @default false
* @public
*/
-export type RouteDeprecation = boolean | RouteInputDeprecation;
+export type RouteDeprecation = boolean | (RouteInputDeprecation & RouteDeprecationLink);
/**
* Additional route options.
diff --git a/packages/core/root/core-root-server-internal/src/route_deprecations/route_deprecations.ts b/packages/core/root/core-root-server-internal/src/route_deprecations/route_deprecations.ts
index 9582cb1678d..1b0209b9137 100644
--- a/packages/core/root/core-root-server-internal/src/route_deprecations/route_deprecations.ts
+++ b/packages/core/root/core-root-server-internal/src/route_deprecations/route_deprecations.ts
@@ -13,8 +13,7 @@ import type { Logger } from '@kbn/logging';
import { CoreKibanaRequest, buildDeprecations } from '@kbn/core-http-router-server-internal';
interface Dependencies {
- logRouteApiDeprecations: boolean; // TODO(jloleysens) use this
- log: Logger;
+ log: Logger; // TODO(jloleysens) use this
coreUsageData: InternalCoreUsageDataSetup;
}
@@ -25,20 +24,23 @@ export function createRouteDeprecationsHandler({ coreUsageData }: Dependencies)
options: { deprecated: deprecatedInput },
},
} = req;
- const messages: string[] = [];
+ let deprecatedUsage = false;
if (typeof deprecatedInput === 'boolean' && deprecatedInput === true) {
- // Log route level deprecation
- messages.push(`${req.route.method} ${req.route.path} is deprecated.`);
+ deprecatedUsage = true;
} else if (typeof deprecatedInput === 'object') {
- // Log route input level deprecation + message
const deprecations = buildDeprecations(deprecatedInput);
- for (const { check, message, location } of deprecations) {
- if (check(req[location])) messages.push(`${req.route.method} ${req.route.path} ${message}`);
+ for (const { check, location } of deprecations) {
+ if (check(req[location])) {
+ deprecatedUsage = true;
+ break;
+ }
}
}
- for (const message of messages) {
- coreUsageData.incrementUsageCounter({ counterName: message }); // TODO(jloleysens) figure this part out
+ if (deprecatedUsage) {
+ coreUsageData.incrementDeprecatedApiUsageCounter({
+ counterName: `${req.route.method}${req.route.path}`,
+ });
}
};
}
diff --git a/packages/core/root/core-root-server-internal/src/server.ts b/packages/core/root/core-root-server-internal/src/server.ts
index 71deaf11075..58550c520fe 100644
--- a/packages/core/root/core-root-server-internal/src/server.ts
+++ b/packages/core/root/core-root-server-internal/src/server.ts
@@ -572,11 +572,10 @@ export class Server {
},
});
- coreSetup.http.registerOnPostValidation(
+ coreSetup.http.registerOnPostValidationListener(
createRouteDeprecationsHandler({
coreUsageData: coreSetup.coreUsageData,
log: this.logger.get('http.route-deprecations'),
- logRouteApiDeprecations: true,
})
);
}
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.
preliminary comment
/** | ||
* @internal | ||
*/ | ||
export class Router<Context extends RequestHandlerContextBase = RequestHandlerContextBase> | ||
implements IRouter<Context> | ||
{ | ||
private static ee = new EventEmitter(); |
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.
nit: use descriptive var-name, e.g. eventEmitter.
Closing in favor of #196081 |
Closed in favor of #196081