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

[HTTP] Log deprecated api usages #207904

Merged
merged 25 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1c09922
add logger with random message
jesuswr Jan 22, 2025
449b082
add config, need to improve log message
jesuswr Jan 22, 2025
0a05840
add correct format to debug log
jesuswr Jan 22, 2025
6694076
import only types
jesuswr Jan 22, 2025
a68f80d
fixed tests, now only need to add new tests
jesuswr Jan 22, 2025
9dbefef
Merge branch 'main' into log-deprecated-api-usages
elasticmachine Jan 22, 2025
55f1fa2
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Jan 22, 2025
3432da2
update func
jesuswr Jan 23, 2025
6d22cd0
tests for get ecs response log
jesuswr Jan 23, 2025
9ca7227
rename config value
jesuswr Jan 23, 2025
3676126
add test for setup
jesuswr Jan 23, 2025
f3a5dd1
add tests for new config value
jesuswr Jan 23, 2025
9bb442d
Merge branch 'main' into log-deprecated-api-usages
elasticmachine Jan 24, 2025
7dee6a8
add documentation for new config value
jesuswr Jan 24, 2025
187cd95
Update docs/settings/logging-settings.asciidoc
jesuswr Jan 27, 2025
cfda527
Update docs/setup/upgrade.asciidoc
jesuswr Jan 27, 2025
9657061
Update docs/setup/upgrade.asciidoc
jesuswr Jan 27, 2025
8f45723
Update docs/settings/logging-settings.asciidoc
jesuswr Jan 27, 2025
13c6a19
Merge branch 'main' into log-deprecated-api-usages
elasticmachine Jan 27, 2025
53d24f4
Update src/core/packages/deprecations/server-internal/src/deprecation…
jesuswr Jan 28, 2025
ab09f10
Update src/core/packages/deprecations/server-internal/src/routes/post…
jesuswr Jan 28, 2025
bf91b1e
Update src/core/packages/http/server-internal/src/logging/get_respons…
jesuswr Jan 28, 2025
e5a91d3
rename config
jesuswr Jan 28, 2025
3963a03
fix reference in docs
jesuswr Jan 28, 2025
c356682
add more tests to get ecs response log
jesuswr Jan 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/settings/logging-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,7 @@ The following table serves as a quick reference for different logging configurat
| `logging.loggers[].<logger>.appenders`
| Determines the appender to apply to a specific logger context as an array. Optional and falls back to the appender(s) of the `root` logger if not specified.

|[[allow-http-debug-logs]] `deprecation.allow_http_debug_logs`
| Optional boolean to log debug messages when a deprecated API is called. Default is `false`.

|===
3 changes: 2 additions & 1 deletion docs/setup/upgrade.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ To upgrade from 7.16.0 or earlier to {version},
Before you upgrade, you must resolve all critical issues identified by the *Upgrade Assistant*.

In addition, we recommend to carefully review the <<breaking-changes-summary,list of breaking changes and deprecations>> and to take any necessary actions to mitigate their impact on the upgrade.
You can enable the <<allow-http-debug-logs, deprecated APIs debug logs config>> to get information about calls to deprecated APIs.

Rolling upgrades are unsupported in {kib}. To upgrade,
you must shut down all {kib} instances, install the new software, and restart {kib}.
Expand Down Expand Up @@ -75,4 +76,4 @@ include::upgrade/saved-objects-migration.asciidoc[]

include::upgrade/resolving-migration-failures.asciidoc[]

include::upgrade/rollback-migration.asciidoc[]
include::upgrade/rollback-migration.asciidoc[]
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { ServiceConfigDescriptor } from '@kbn/core-base-server-internal';
const configSchema = schema.object({
// `deprecation.skip_deprecated_settings` is consistent with the equivalent ES feature and config property
skip_deprecated_settings: schema.arrayOf(schema.string(), { defaultValue: [] }),
allow_http_debug_logs: schema.boolean({ defaultValue: false }),
});

export type DeprecationConfigType = TypeOf<typeof configSchema>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export const DeprecationsFactoryMock = jest
export const registerConfigDeprecationsInfoMock = jest.fn();
export const registerApiDeprecationsInfoMock = jest.fn();

export const loggingMock = {
configure: jest.fn(),
};

jest.doMock('./deprecations', () => ({
registerConfigDeprecationsInfo: registerConfigDeprecationsInfoMock,
registerApiDeprecationsInfo: registerApiDeprecationsInfoMock,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import {
DeprecationsFactoryMock,
registerConfigDeprecationsInfoMock,
loggingMock,
} from './deprecations_service.test.mocks';
import { mockCoreContext } from '@kbn/core-base-server-mocks';
import { httpServerMock, httpServiceMock } from '@kbn/core-http-server-mocks';
Expand All @@ -18,6 +19,7 @@ import { configServiceMock } from '@kbn/config-mocks';
import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks';
import { elasticsearchServiceMock } from '@kbn/core-elasticsearch-server-mocks';
import { DeprecationsService, DeprecationsSetupDeps } from './deprecations_service';
import { firstValueFrom } from 'rxjs';

describe('DeprecationsService', () => {
let coreContext: ReturnType<typeof mockCoreContext.create>;
Expand All @@ -35,7 +37,7 @@ describe('DeprecationsService', () => {
coreUsageData = coreUsageDataServiceMock.createSetupContract();
router = httpServiceMock.createRouter();
http.createRouter.mockReturnValue(router);
deprecationsCoreSetupDeps = { http, coreUsageData };
deprecationsCoreSetupDeps = { http, coreUsageData, logging: loggingMock };
});

afterEach(() => {
Expand Down Expand Up @@ -64,6 +66,32 @@ describe('DeprecationsService', () => {
expect(registerConfigDeprecationsInfoMock).toBeCalledTimes(1);
});

describe('logging.configure tests', () => {
it('calls logging.configure without allow_http_debug_logs', async () => {
const deprecationsService = new DeprecationsService(coreContext);
await deprecationsService.setup(deprecationsCoreSetupDeps);
expect(loggingMock.configure).toBeCalledTimes(1);
const config$ = loggingMock.configure.mock.calls[0][1];
expect(await firstValueFrom(config$)).toStrictEqual({
loggers: [{ name: 'http', level: 'off', appenders: [] }],
});
});

it('calls logging.configure with allow_http_debug_logs set to true', async () => {
const configService = configServiceMock.create({
atPath: { allow_http_debug_logs: true },
});
coreContext = mockCoreContext.create({ configService });
const deprecationsService = new DeprecationsService(coreContext);
await deprecationsService.setup(deprecationsCoreSetupDeps);
expect(loggingMock.configure).toBeCalledTimes(1);
const config$ = loggingMock.configure.mock.calls[0][1];
expect(await firstValueFrom(config$)).toStrictEqual({
loggers: [{ name: 'http', level: 'debug', appenders: [] }],
});
});
});

it('creates DeprecationsFactory with the correct parameters', async () => {
const deprecationsService = new DeprecationsService(coreContext);
await deprecationsService.setup(deprecationsCoreSetupDeps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { firstValueFrom } from 'rxjs';
import { firstValueFrom, map } from 'rxjs';
import type { Logger } from '@kbn/logging';
import type { IConfigService } from '@kbn/config';
import type { CoreContext, CoreService } from '@kbn/core-base-server-internal';
Expand All @@ -21,6 +21,7 @@ import type {
} from '@kbn/core-deprecations-server';
import { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { KibanaRequest } from '@kbn/core-http-server';
import { type InternalLoggingServiceSetup } from '@kbn/core-logging-server-internal';
import { DeprecationsFactory } from './deprecations_factory';
import { registerRoutes } from './routes';
import { config as deprecationConfig, DeprecationConfigType } from './deprecation_config';
Expand Down Expand Up @@ -49,6 +50,7 @@ export type InternalDeprecationsServiceSetup = DeprecationRegistryProvider;
export interface DeprecationsSetupDeps {
http: InternalHttpServiceSetup;
coreUsageData: InternalCoreUsageDataSetup;
logging: InternalLoggingServiceSetup;
}

/** @internal */
Expand All @@ -67,6 +69,7 @@ export class DeprecationsService
public async setup({
http,
coreUsageData,
logging,
}: DeprecationsSetupDeps): Promise<InternalDeprecationsServiceSetup> {
this.logger.debug('Setting up Deprecations service');

Expand All @@ -81,7 +84,18 @@ export class DeprecationsService
},
});

registerRoutes({ http, coreUsageData });
const config$ = this.configService.atPath<DeprecationConfigType>(deprecationConfig.path);
logging.configure(
['deprecations-service'],
config$.pipe(
map((cfg) => {
const logLevel = cfg.allow_http_debug_logs ? 'debug' : 'off';
return { loggers: [{ name: 'http', level: logLevel, appenders: [] }] };
})
)
);

registerRoutes({ http, coreUsageData, logger: this.logger.get('http') });

registerConfigDeprecationsInfo({
deprecationsFactory: this.deprecationsFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import type { InternalHttpServiceSetup } from '@kbn/core-http-server-internal';
import { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import { Logger } from '@kbn/logging';
import type { InternalDeprecationRequestHandlerContext } from '../internal_types';
import { registerGetRoute } from './get';
import { registerMarkAsResolvedRoute } from './resolve_deprecated_api';
Expand All @@ -17,13 +18,15 @@ import { registerApiDeprecationsPostValidationHandler } from './post_validation_
export function registerRoutes({
http,
coreUsageData,
logger,
}: {
http: InternalHttpServiceSetup;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
}) {
const router = http.createRouter<InternalDeprecationRequestHandlerContext>('/api/deprecations');
registerGetRoute(router);

registerApiDeprecationsPostValidationHandler({ http, coreUsageData });
registerApiDeprecationsPostValidationHandler({ http, coreUsageData, logger });
registerMarkAsResolvedRoute(router, { coreUsageData });
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@
*/

import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-server-internal';
import type { CoreKibanaRequest } from '@kbn/core-http-router-server-internal';
import { ensureRawRequest, type CoreKibanaRequest } from '@kbn/core-http-router-server-internal';
import type { InternalHttpServiceSetup } from '@kbn/core-http-server-internal';
import type { PostValidationMetadata } from '@kbn/core-http-server';
import { Logger } from '@kbn/logging';
import { getEcsResponseLog } from '@kbn/core-http-server-internal/src/logging';
import { buildApiDeprecationId } from '../deprecations';
import { getIsRouteApiDeprecation, getIsAccessApiDeprecation } from '../deprecations';

interface Dependencies {
coreUsageData: InternalCoreUsageDataSetup;
http: InternalHttpServiceSetup;
logger: Logger;
}

/**
Expand All @@ -26,14 +29,17 @@ interface Dependencies {
export const registerApiDeprecationsPostValidationHandler = ({
coreUsageData,
http,
logger,
}: Dependencies) => {
http.registerOnPostValidation(createRouteDeprecationsHandler({ coreUsageData }));
http.registerOnPostValidation(createRouteDeprecationsHandler({ coreUsageData, logger }));
};

export function createRouteDeprecationsHandler({
coreUsageData,
logger,
}: {
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
}) {
return (req: CoreKibanaRequest, metadata: PostValidationMetadata) => {
const hasRouteDeprecation = getIsRouteApiDeprecation(metadata);
Expand All @@ -49,6 +55,9 @@ export function createRouteDeprecationsHandler({
const client = coreUsageData.getClient();
// no await we just fire it off.
void client.incrementDeprecatedApi(counterName, { resolved: false });

const { message, meta } = getEcsResponseLog(ensureRawRequest(req), logger);
logger.debug(message, meta);
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"@kbn/core-usage-data-server-internal",
"@kbn/core-usage-data-server-mocks",
"@kbn/core-http-router-server-internal",
"@kbn/core-logging-server-internal",
],
"exclude": [
"target/**/*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface RequestFixtureOptions {
mime?: string;
path?: string;
query?: Record<string, any>;
response?: Record<string, any> | Boom.Boom;
response?: Record<string, any> | Boom.Boom | null;
app?: Record<string, any>;
}

Expand Down Expand Up @@ -345,4 +345,13 @@ describe('getEcsResponseLog', () => {
`);
});
});

test('does not return response in meta object if response is not in the request', () => {
const req = createMockHapiRequest({
response: null,
});
const result = getEcsResponseLog(req, logger);
expect(result.message).toMatchInlineSnapshot(`"GET /path"`);
expect(result.meta.http?.response).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,36 @@ export function getEcsResponseLog(request: Request, log: Logger) {
const query = querystring.stringify(request.query);
const pathWithQuery = query.length > 0 ? `${path}?${query}` : path;

// eslint-disable-next-line @typescript-eslint/naming-convention
const status_code = isBoom(response) ? response.output.statusCode : response.statusCode;

const requestHeaders = cloneAndFilterHeaders(request.headers);
const responseHeaders = cloneAndFilterHeaders(
isBoom(response) ? (response.output.headers as HapiHeaders) : response.headers
);

// borrowed from the hapi/good implementation
const responseTime = (request.info.completed || request.info.responded) - request.info.received;
const responseTimeMsg = !isNaN(responseTime) ? ` ${responseTime}ms` : '';
const responseTimeMsg = !isNaN(responseTime) && responseTime >= 0 ? ` ${responseTime}ms` : '';

const bytes = getResponsePayloadBytes(response, log);
const bytes = response ? getResponsePayloadBytes(response, log) : undefined;
const bytesMsg = bytes ? ` - ${numeral(bytes).format('0.0b')}` : '';

const traceId = (request.app as KibanaRequestState).traceId;

const responseLogObj = response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job making this more robust! Is it possible that we can add some test coverage in src/core/packages/http/server-internal/src/logging/get_response_log.test.ts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

? {
response: {
body: {
bytes,
},
status_code: isBoom(response) ? response.output.statusCode : response.statusCode,
headers: cloneAndFilterHeaders(
isBoom(response) ? (response.output.headers as HapiHeaders) : response.headers
),
responseTime: !isNaN(responseTime) ? responseTime : undefined,
},
}
: {};

const message = response
? `${method} ${pathWithQuery} ${responseLogObj.response?.status_code}${responseTimeMsg}${bytesMsg}`
: `${method} ${pathWithQuery}`;

const meta: LogMeta = {
client: {
ip: request.info.remoteAddress,
Expand All @@ -88,16 +101,7 @@ export function getEcsResponseLog(request: Request, log: Logger) {
// @ts-expect-error ECS custom field: https://github.com/elastic/ecs/issues/232.
headers: requestHeaders,
},
response: {
body: {
bytes,
},
status_code,
// @ts-expect-error ECS custom field: https://github.com/elastic/ecs/issues/232.
headers: responseHeaders,
// responseTime is a custom non-ECS field
responseTime: !isNaN(responseTime) ? responseTime : undefined,
},
...responseLogObj,
},
url: {
path,
Expand All @@ -110,7 +114,7 @@ export function getEcsResponseLog(request: Request, log: Logger) {
};

return {
message: `${method} ${pathWithQuery} ${status_code}${responseTimeMsg}${bytesMsg}`,
message,
meta,
};
}
5 changes: 3 additions & 2 deletions src/core/packages/root/server-internal/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,12 @@ export class Server {
changedDeprecatedConfigPath$: this.configService.getDeprecatedConfigPath$(),
});

const loggingSetup = this.logging.setup();

const deprecationsSetup = await this.deprecations.setup({
http: httpSetup,
coreUsageData: coreUsageDataSetup,
logging: loggingSetup,
});

const savedObjectsSetup = await this.savedObjects.setup({
Expand Down Expand Up @@ -348,8 +351,6 @@ export class Server {
rendering: renderingSetup,
});

const loggingSetup = this.logging.setup();

const coreSetup: InternalCoreSetup = {
analytics: analyticsSetup,
capabilities: capabilitiesSetup,
Expand Down