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

[Fleet] Exclude events log from EA diagnostic collection in modal #190469

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

lucabelluccini
Copy link
Contributor

Summary

In 8.15 we've reintroduced the logging of raw events when in debug log via elastic/beats#38767.

Given the Elastic Agent is able to read the flag from the action (https://github.com/elastic/elastic-agent/blob/44528c49507c18e71a4cfd2a6ba4f0904bd32009/internal/pkg/fleetapi/action.go#L483), I'm attempting to draft a PR.

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

  • Is it safe to send the new parameter to Elastic Agents which do not support the exclude_event_logs?
  • Do we need to handle the saved objects types of the Diagnostic Request?

For maintainers

@lucabelluccini
Copy link
Contributor Author

Tested locally with 8.15 and 8.11 to ensure the flag is not preventing the diagnostic collection on older Elastic Agent versions.

We might need to put this on hold until Elastic Agent considers this flag in the protocol and repeat the tests as afaik this requires a protocol change.

@@ -23,7 +23,8 @@ import {
export async function requestDiagnostics(
esClient: ElasticsearchClient,
agentId: string,
additionalMetrics?: RequestDiagnosticsAdditionalMetrics[]
additionalMetrics?: RequestDiagnosticsAdditionalMetrics[],
excludeEventsLog?: boolean,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not 100% sure

@@ -36,7 +36,7 @@ describe('request diagnostics handler', () => {
let mockRequest: KibanaRequest<
{ agentId: string },
undefined,
{ additional_metrics: RequestDiagnosticsAdditionalMetrics[] },
{ additional_metrics: RequestDiagnosticsAdditionalMetrics[]; exclude_events_log?: boolean },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not 100% sure

@lucabelluccini lucabelluccini marked this pull request as ready for review August 22, 2024 18:08
@lucabelluccini lucabelluccini requested review from a team as code owners August 22, 2024 18:08
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Aug 22, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet self-requested a review August 26, 2024 13:06
@nchaulet
Copy link
Member

Got the following error when trying the API

Screenshot 2024-08-26 at 9 27 51 AM

You will have to update the request schema for the POST request diagnostic and bulk operation too here

<EuiCheckbox
id="includeEventsLogCheckbox"
data-test-subj="includeEventsLogCheckbox"
label="Include Events Logs (might contain sensible information)"
Copy link
Member

Choose a reason for hiding this comment

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

It will be nice to have translated value with i18n.translate for the label

Copy link

cla-checker-service bot commented Aug 26, 2024

💚 CLA has been signed

@lucabelluccini
Copy link
Contributor Author

Weird to get the warning - I do not get the popup. I've already updated the request schema.

I've updated the code to use the i18n and the render is still good.

image

The derived Fleet actions SO are:

     {
        "_index": ".fleet-actions-7",
        "_id": "6d9e72e1-504c-4b3f-bad4-3b7791282597",
        "_score": null,
        "_source": {
          "@timestamp": "2024-08-26T21:52:59.529Z",
          "expiration": "2024-08-27T00:52:59.529Z",
          "agents": [
            "a04e9ed5-eccb-43ea-a26b-8d0ee7010ba7"
          ],
          "action_id": "817f60c0-2af5-4b60-8ccf-be58c4601ed4",
          "data": {
            "additional_metrics": [],
            "exclude_events_log": false
          },
          "type": "REQUEST_DIAGNOSTICS",
          "traceparent": "00-cb510e4d60758d79731b1678971bce38-25ae78c9627e0a56-01"
        },
        "sort": [
          1724709179529
        ]
      },
      {
        "_index": ".fleet-actions-7",
        "_id": "fdc86319-ad15-4d2e-8f19-0e719bd46f66",
        "_score": null,
        "_source": {
          "@timestamp": "2024-08-26T21:52:50.542Z",
          "expiration": "2024-08-27T00:52:50.542Z",
          "agents": [
            "a04e9ed5-eccb-43ea-a26b-8d0ee7010ba7"
          ],
          "action_id": "22c2a9f4-71ae-4d43-a3d0-8641fb7ffef0",
          "data": {
            "additional_metrics": [],
            "exclude_events_log": true
          },
          "type": "REQUEST_DIAGNOSTICS",
          "traceparent": "00-6a1b2c5a804f01dc2a2bd30c314a8033-66ccb911e742654b-01"
        },
        "sort": [
          1724709170542
        ]
      }

I still think we need some work on EA side in order to merge IIUC.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 26, 2024

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #41 / apis ESQL sync error messages Using string field type: text and number field type: integer Checking error messages
  • [job] [logs] FTR Configs #41 / apis ESQL sync error messages Using string field type: text and number field type: integer Checking error messages

Metrics [docs]

Async chunks

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

id before after diff
fleet 1.8MB 1.8MB +487.0B

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants