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

[RAM] Allow users to see event logs from all spaces they have access to #140449

Merged
merged 28 commits into from
Nov 1, 2022

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Sep 9, 2022

Summary

Closes #137995
Screen Shot 2022-10-17 at 3 15 30 PM

Adds a property called namespaces to the global_execution_logs API. This is an array of Space IDs to fetch logs for.

In the global Logs tab, a switch will be enabled if the current user has access to multiple spaces. It will be disabled if the current user can only access one space. When this switch is toggled, it will send all accessible namespaces to the logs API.

Clicking a link to a rule on a different space will switch the user to that space in a new tab.

Checklist

@Zacqary Zacqary added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX 8.6 candidate labels Sep 9, 2022
…itch

# Conflicts:
#	x-pack/plugins/alerting/common/execution_log_types.ts
#	x-pack/plugins/alerting/server/lib/get_execution_log_aggregation.ts
#	x-pack/plugins/alerting/server/rules_client/rules_client.ts
@Zacqary Zacqary changed the title [RAM] Add all_namespaces prop to global logs api [RAM] Allow users to see event logs from all spaces they have access to Oct 17, 2022
@Zacqary Zacqary marked this pull request as ready for review October 18, 2022 20:43
@Zacqary Zacqary requested a review from a team as a code owner October 18, 2022 20:43
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@Zacqary Zacqary added the release_note:feature Makes this part of the condensed release notes label Oct 18, 2022
Copy link
Contributor

@JiaweiWu JiaweiWu 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 overall! I left some comments and it would be great to add some UI tests for this, especially for the spaces routing on click and the spaces switch.

@@ -398,6 +398,7 @@ export interface GetGlobalExecutionKPIParams {
dateStart: string;
dateEnd?: string;
filter?: string;
namespaces?: Array<string | undefined>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to make this type a bit stricter? (I.e. just Array<string>). I see that getNameSpace could return undefined, could we filter those entries out or do they have some significance being undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Default namespace is referred to as undefined internally. As far as I can tell, this changed recently; it used to just be the string default.

@Zacqary Zacqary enabled auto-merge (squash) October 26, 2022 17:00
@Zacqary
Copy link
Contributor Author

Zacqary commented Oct 28, 2022

Just pushed a fix for accessing action error logs from alternate spaces

Copy link
Contributor

@JiaweiWu JiaweiWu left a comment

Choose a reason for hiding this comment

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

I've tested the fix for the error logs, seems to work as expected. I just had some small comments

@@ -389,6 +401,50 @@ export class ClusterClientAdapter<TDoc extends { body: AliasAny; index: string }
}
}

public async queryEventsWithAuthFilter(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we should reduce some of the code duplication between this and queryEventsBySavedObjects, or maybe we can modify queryEventsBySavedObjects to support the new auth filters? Curious to hear your thoughts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to modify the existing queryEventsBySavedObjects functionality too much, I'm not sure if it'll modify expected error behaviors in unpredictable places.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 511 512 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
alerting 373 374 +1
eventLog 106 115 +9
total +10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
triggersActionsUi 676.8KB 679.4KB +2.6KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
eventLog 10 11 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
triggersActionsUi 100.5KB 100.9KB +418.0B
Unknown metric groups

API count

id before after diff
alerting 382 383 +1
eventLog 106 115 +9
total +10

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 57 63 +6
osquery 103 108 +5
securitySolution 439 443 +4
triggersActionsUi 131 132 +1
total +18

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 65 71 +6
osquery 104 110 +6
securitySolution 516 520 +4
triggersActionsUi 134 135 +1
total +19

History

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

Copy link
Contributor

@JiaweiWu JiaweiWu left a comment

Choose a reason for hiding this comment

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

LGTM!

@Zacqary Zacqary merged commit 5c50cd4 into elastic:main Nov 1, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 1, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 1, 2022
* main: (43 commits)
  [Synthetics] Step details page screenshot (elastic#143452)
  [Lens] Datatable expression types improvement. (elastic#144173)
  [packages/kbn-journeys] start apm after browser start and stop after browser is closed (elastic#144267)
  [Files] Make files namespace agnostic (elastic#144019)
  Implement base browser-side logging system (elastic#144107)
  Correct wrong multiplier for byte conversion (elastic#143751)
  [Monaco] Add JSON syntax support to the Monaco editor (elastic#143739)
  CCS Smoke Test for Remote Clusters and Index Management  (elastic#142423)
  [api-docs] Daily api_docs build (elastic#144294)
  chore(NA): include progress on Bazel tasks (elastic#144275)
  [RAM] Allow users to see event logs from all spaces they have access to (elastic#140449)
  [APM] Show recommended minimum size when going below 5 minutes (elastic#144170)
  [typecheck] delete temporary target_types dirs in packages (elastic#144271)
  [Security Solution][Endpoint] adds new alert loading utility and un-skip FTR test for endpoint (elastic#144133)
  [performance/journeys] revert data_stress_test_lens.ts journey step (elastic#144261)
  [TIP] Use search strategies in Threat Intelligence (elastic#143267)
  Optimize react-query dependencies (elastic#144206)
  [babel/node] invalidate cache when synth pkg map is updated (elastic#144258)
  [APM] AWS lambda estimated cost (elastic#143986)
  [Maps] layer group wizard (elastic#144129)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:feature Makes this part of the condensed release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RAM] [META] Global event log deliverable 3/3: Allow superusers to see global event logs and visualizations
5 participants