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

[Stack Monitoring] Adds optional debug logging for Stack Monitoring #107711

Merged

Conversation

jasonrhodes
Copy link
Member

@jasonrhodes jasonrhodes commented Aug 5, 2021

Summary

Adds two config options:

monitoring.ui.debug_mode: true: (default: false): When true, will log JSON to the Kibana logs representing every query that is run on every page
monitoring.ui.debug_file_path: <string> (default: ''): when set, will write these logs to a file instead

Each ndjson log line will have the following shape:

{
  "referer_url": "<the URL of the page that made the request in the UI>",
  "api_path": "<the API endpoint called>",
  "query": {
    "params": <the query executed>,
    "result": <the ES result>
  }
}

If monitoring.ui.debug_mode isn't set to true, nothing will change. There are two checks to make sure that nothing gets changed unless that flag is set.

@jasonrhodes jasonrhodes requested a review from a team as a code owner August 5, 2021 00:06
@jasonrhodes jasonrhodes requested review from a team August 5, 2021 00:06
@jasonrhodes jasonrhodes force-pushed the enable-sm-debug-logging-option branch from 8a2726c to 3c407d1 Compare August 5, 2021 00:11
@jasonrhodes jasonrhodes added release_note:skip Skip the PR/issue when compiling release notes v7.15.0 labels Aug 5, 2021
RequestHandlerContextMonitoringPlugin,
} from './types';

import { Globals, EndpointTypes } from './static_globals';
import { instantiateClient } from './es_client/instantiate_client';

// This is used to test the version of kibana
const snapshotRegex = /-snapshot/i;
Copy link
Member Author

Choose a reason for hiding this comment

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

All the changes above this line are just prettier import reorganization

@@ -6,53 +6,52 @@
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

changes at the top of this file are all prettier import reorganization

requireUIRoutes(this.monitoringCore, {
if (config.ui.debug_mode) {
this.log.info('MONITORING DEBUG MODE: ON');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I should add another log line to mention which file it's writing to, if any

const routes = Object.keys(uiRoutes);
const server = config.ui.debug_mode
Copy link
Member Author

Choose a reason for hiding this comment

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

Check 1 of 2 to make sure we only decorate this server object if debug_mode is on...

// bail if the proper config value is not set (extra protection)
if (!config.ui.debug_mode) {
return _server;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Check 2 of 2 to make sure we ONLY screw around with this server object if debug_mode is turned on...

Copy link
Contributor

@Zacqary Zacqary left a comment

Choose a reason for hiding this comment

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

Looks good! Couple suggestions on how to resolve some type TODOs though since _server is an any I'm not sure if they'd work. Feel free to merge either way.

// maintain the rest of _server untouched
..._server,
// TODO: replace any
route: (options: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe replace any with:

type ServerRouteFn = typeof _server.route;
const route: (...args: Parameters<ServerRouteFn>) => ReturnType<ServerRouteFn> = (options) => {
 // ...
}

return {
   ..._server,
   route
}

return {
...cluster,
// TODO: better types?
callWithRequest: async (_req: typeof req, type: string, params: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these the same types as Parameters<typeof cluster.callWithRequest>?

@jasonrhodes jasonrhodes added auto-backport Deprecated - use backport:version if exact versions are needed v8.0.0 labels Aug 5, 2021
@jasonrhodes
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants