Skip to content

[ML] AIOps: Refactors function argument structure for Log Rate Analysis.#187669

Merged
walterra merged 9 commits intoelastic:mainfrom
walterra:ml-aiops-refactor-fn-args
Jul 8, 2024
Merged

[ML] AIOps: Refactors function argument structure for Log Rate Analysis.#187669
walterra merged 9 commits intoelastic:mainfrom
walterra:ml-aiops-refactor-fn-args

Conversation

@walterra
Copy link
Copy Markdown
Contributor

@walterra walterra commented Jul 5, 2024 •
edited
Loading

Summary

Part of #178613.

Refactors the function argument structure of code used on Kibana server for Log Rate Analysis from individual arguments to single objects that contain all options. The options structure looks like this:

{
  // "meta" args like dependencies, general callbacks etc. on the outer most level
  esClient,
  abortSignal,
  ...
  // within "arguments" we pass in actual options that are necessary for the logic of the function
  arguments: {
    start,
    end,
    query,
    fields,
    ...
  }
}

The main benefit is that code where these functions are used become easier to read. Instead of the strict order of args that sometimes included undefined or just a value where it's hard to guess for which argument it's used for, this enforces to have the names of options show up in the consuming code. Here's an example:

Before:

await fetchHistogramsForFields(
                client,
                requestBody.index,
                histogramQuery,
                [
                  {
                    fieldName: requestBody.timeFieldName,
                    type: KBN_FIELD_TYPES.DATE,
                    interval: overallTimeSeries.interval,
                    min: overallTimeSeries.stats[0],
                    max: overallTimeSeries.stats[1],
                  },
                ],
                -1,
                undefined,
                abortSignal,
                stateHandler.sampleProbability(),
                RANDOM_SAMPLER_SEED
              )

After:

                (await fetchHistogramsForFields({
                  esClient,
                  abortSignal,
                  arguments: {
                    indexPattern: requestBody.index,
                    query: histogramQuery,
                    fields: [
                      {
                        fieldName: requestBody.timeFieldName,
                        type: KBN_FIELD_TYPES.DATE,
                        interval: overallTimeSeries.interval,
                        min: overallTimeSeries.stats[0],
                        max: overallTimeSeries.stats[1],
                      },
                    ],
                    samplerShardSize: -1,
                    randomSamplerProbability: stateHandler.sampleProbability(),
                    randomSamplerSeed: RANDOM_SAMPLER_SEED,
                  },
                })) as [NumericChartData]

Checklist

@walterra walterra added :ml release_note:skip Skip the PR/issue when compiling release notes Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v8.16.0 labels Jul 5, 2024
@walterra walterra self-assigned this Jul 5, 2024
@walterra walterra requested a review from a team as a code owner July 5, 2024 12:29
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 5, 2024

Pinging @elastic/ml-ui (:ml)

@walterra walterra force-pushed the ml-aiops-refactor-fn-args branch from 7198b48 to be4457b Compare July 5, 2024 15:13
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 8, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #2 / useSetupTechnology should not update agent policy and selected policy tab when setup technology matches the current one

Metrics [docs]

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/ml-agg-utils 2 1 -1
Unknown metric groups

API count

id before after diff
@kbn/ml-agg-utils 102 90 -12

History

cc @walterra

peteharverson
peteharverson approved these changes Jul 8, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Code LGTM - a nice refactor to make the code easier to read.

alvarezmelissa87
alvarezmelissa87 approved these changes Jul 8, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡

@walterra walterra merged commit 94cab93 into elastic:main Jul 8, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Jul 8, 2024
@walterra walterra deleted the ml-aiops-refactor-fn-args branch July 8, 2024 18:06
This was referenced Jul 9, 2024
Closed
Closed
@walterra walterra mentioned this pull request Aug 5, 2024
Merged
3 tasks
walterra added a commit that referenced this pull request Aug 7, 2024
@walterra
[ML] AIOps: Tweak function API for fetchTopCategories/fetchTopTerms (#…
0a01fdd 
…189863)

## Summary

Follow up to #187669.
Part of #187684.
Fixes #176387. (Ran the flaky test runner on AIOps functional tests)

- Fixes the `size: 0` option to be properly nested for
`createCategoryRequest()`.
- Changes the arguments structure for `fetchTopCategories` and
`fetchTopTerms` from individual arguments to an options object to be
more in line with the other functions used for log rate analysis.
- Adds jest unit test for `fetchTopCategories` and `fetchTopTerms`.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 approved these changes

@peteharverson peteharverson peteharverson approved these changes

Assignees

@walterra walterra

Labels

backport:skip This PR does not require backporting Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis :ml release_note:skip Skip the PR/issue when compiling release notes v8.16.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@walterra @elasticmachine @alvarezmelissa87 @peteharverson @kibanamachine