-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Infra] Limit the number of metrics accepted by Metrics Explorer API #188112
[Infra] Limit the number of metrics accepted by Metrics Explorer API #188112
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
/ci |
'labels.*', | ||
'tags', | ||
]; | ||
export const METRICS_EXPLORER_MAX_METRICS_PER_QUERY = 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An arbitrary value to limit the metrics array size. I don't know if this is too few or too many.
6b51b3d
to
dcbfc4e
Compare
/ci |
1 similar comment
/ci |
ae15341
to
7d533a9
Compare
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
expected head sha didn’t match current head ref. |
…plorer-api-limit-number-of-metrics
@elasticmachine merge upstream |
'labels.*', | ||
'tags', | ||
]; | ||
export const METRICS_EXPLORER_API_MAX_METRICS = 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of curiosity why 20?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just saw your comment: #188112 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an arbitrary value to limit the number of items. It was aligned with @roshan-elastic here #188181 (comment)
const client = createSearchClient(requestContext, framework); | ||
const interval = await findIntervalForMetrics(client, options); | ||
try { | ||
if (options.metrics.length > METRICS_EXPLORER_API_MAX_METRICS) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this endpoint potentially be used in alerts or other scenarios where a user might exceed the 20-metric limit and start getting this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean our internal alerts? This endpoint was used to render the Metrics Threshold rule charts, but it has been replaced by Lens in this PR #184950. So it's no longer used anywhere else but the Metrics Explorer page.
export const MetricsExplorerMetrics = ({ options, onChange, autoFocus = false }: Props) => { | ||
const { metricsView } = useMetricsDataViewContext(); | ||
const colors = Object.keys(Color) as Array<keyof typeof Color>; | ||
const [shouldFocus, setShouldFocus] = useState(autoFocus); | ||
|
||
const maxMetricsReached = useMemo( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this useMemo
needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left a few comments
…plorer-api-limit-number-of-metrics
⏳ Build in-progress
History
|
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…lastic#188112) part of [3628](elastic/observability-dev#3628) - private ## Summary After adding 20 items, users can no longer add more fields and will see the message below <img width="1725" alt="image" src="https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e"> ### Extra - There was an unused and duplicate `metrics_explorer` route in infra. I removed it. It should've been removed when the `metrics_data_access` plugin was created. - Cleaned up `constants` field in `metrics_data_access` and `infra` plugins ### How to test - Start a local Kibana instance pointing to an oblt cluster - Navigate to Infrastructure > Metrics Explorer - Try to select more than 20 fields in the metrics field --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 5ec5b99) # Conflicts: # x-pack/plugins/observability_solution/infra/public/pages/metrics/metrics_explorer/hooks/use_metrics_explorer_data.ts # x-pack/plugins/observability_solution/infra/server/routes/metrics_explorer/index.ts # x-pack/plugins/observability_solution/metrics_data_access/server/routes/metrics_explorer/index.ts
…lastic#188112) part of [3628](elastic/observability-dev#3628) - private ## Summary After adding 20 items, users can no longer add more fields and will see the message below <img width="1725" alt="image" src="https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e"> ### Extra - There was an unused and duplicate `metrics_explorer` route in infra. I removed it. It should've been removed when the `metrics_data_access` plugin was created. - Cleaned up `constants` field in `metrics_data_access` and `infra` plugins ### How to test - Start a local Kibana instance pointing to an oblt cluster - Navigate to Infrastructure > Metrics Explorer - Try to select more than 20 fields in the metrics field --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: kibanamachine <[email protected]>
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…er API (#188112) (#188442) # Backport This will backport the following commits from `main` to `8.15`: - [[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)](#188112) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Carlos Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-16T13:44:05Z","message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:all-open","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.16.0"],"number":188112,"url":"https://github.com/elastic/kibana/pull/188112","mergeCommit":{"message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188112","number":188112,"mergeCommit":{"message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <[email protected]>
…er API (#188112) (#188447) # Backport This will backport the following commits from `main` to `7.17`: - [[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)](#188112) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Carlos Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-16T13:44:05Z","message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:all-open","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.16.0"],"number":188112,"url":"https://github.com/elastic/kibana/pull/188112","mergeCommit":{"message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188112","number":188112,"mergeCommit":{"message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},{"url":"https://github.com/elastic/kibana/pull/188442","number":188442,"branch":"8.15","state":"OPEN"}]}] BACKPORT--> --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: kibanamachine <[email protected]>
…er API (elastic#188112) (elastic#188447) # Backport This will backport the following commits from `main` to `7.17`: - [[Infra] Limit the number of metrics accepted by Metrics Explorer API (elastic#188112)](elastic#188112) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Carlos Crespo","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-16T13:44:05Z","message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (elastic#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:all-open","ci:project-deploy-observability","Team:obs-ux-infra_services","v8.16.0"],"number":188112,"url":"https://github.com/elastic/kibana/pull/188112","mergeCommit":{"message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (elastic#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188112","number":188112,"mergeCommit":{"message":"[Infra] Limit the number of metrics accepted by Metrics Explorer API (elastic#188112)\n\npart of [3628](https://github.com/elastic/observability-dev/issues/3628)\r\n- private\r\n\r\n## Summary\r\n\r\nAfter adding 20 items, users can no longer add more fields and will see\r\nthe message below\r\n\r\n\r\n<img width=\"1725\" alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/2767137/fd504212-0e0f-485d-a8fe-b991c829950e\">\r\n\r\n\r\n### Extra\r\n\r\n- There was an unused and duplicate `metrics_explorer` route in infra. I\r\nremoved it. It should've been removed when the `metrics_data_access`\r\nplugin was created.\r\n- Cleaned up `constants` field in `metrics_data_access` and `infra`\r\nplugins\r\n\r\n### How to test\r\n\r\n- Start a local Kibana instance pointing to an oblt cluster\r\n- Navigate to Infrastructure > Metrics Explorer\r\n- Try to select more than 20 fields in the metrics field\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine <[email protected]>\r\nCo-authored-by: kibanamachine <[email protected]>","sha":"5ec5b994dcc4c47912ad29e6b8da92c4f855041f"}},{"url":"https://github.com/elastic/kibana/pull/188442","number":188442,"branch":"8.15","state":"OPEN"}]}] BACKPORT--> --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: kibanamachine <[email protected]>
part of 3628 - private
Summary
After adding 20 items, users can no longer add more fields and will see the message below
Extra
metrics_explorer
route in infra. I removed it. It should've been removed when themetrics_data_access
plugin was created.constants
field inmetrics_data_access
andinfra
pluginsHow to test