-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[APM] Anomaly detection setup link with alert if job doesn't exist #71229
[APM] Anomaly detection setup link with alert if job doesn't exist #71229
Conversation
…ing in the home header
Pinging @elastic/apm-ui (Team:apm) |
Already added feedback here: ogupte#3 |
x-pack/plugins/apm/public/components/shared/Links/apm/AnomalyDetectionSetupLink.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/shared/Links/apm/AnomalyDetectionSetupLink.tsx
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
environment === ENVIRONMENT_NOT_DEFINED | ||
? ENVIRONMENT_NOT_DEFINED_FILTER | ||
: { term: { [SERVICE_ENVIRONMENT]: environment } }, | ||
...[getEnvironmentUiFilterES(environment)], |
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.
Thanks for fixing this also 👍
const environmentFilter = getEnvironmentUiFilterES(environment); | ||
if (environmentFilter) { | ||
filter.push(environmentFilter); | ||
} |
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.
Slightly cleaner:
const environmentFilter = getEnvironmentUiFilterES(environment); | |
if (environmentFilter) { | |
filter.push(environmentFilter); | |
} | |
const filter: ESFilter[] = [ | |
{ range: rangeFilter(start, end) }, | |
{ term: { [SERVICE_NAME]: serviceName } }, | |
...[getEnvironmentUiFilterES(environment)] | |
]; |
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.
I tried this at first, but it fails the type check since getEnvironmentUiFilterES
returns ESFilter | undefined
. I could change it to return ESFilter[]
, so we can always safely spread it in (...getEnvironmentUiFilterES(environment)
).
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.
done in 4bf17ee
x-pack/plugins/apm/public/components/shared/Links/apm/AnomalyDetectionSetupLink.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/shared/Links/apm/AnomalyDetectionSetupLink.tsx
Outdated
Show resolved
Hide resolved
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 a few small nits. LGTM 👍
…ned` to `ESFilter[]` for ease of use.
@elasticmachine merge upstream |
...getKueryUiFilterES(uiFilters.kuery), | ||
...getEnvironmentUiFilterES(uiFilters.environment), | ||
].concat(mappedFilters) as ESFilter[]; |
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.
Nice change, thanks! Can we use spread for mappedFilters
instead of concat (for consistency)?
...getKueryUiFilterES(uiFilters.kuery), | |
...getEnvironmentUiFilterES(uiFilters.environment), | |
].concat(mappedFilters) as ESFilter[]; | |
...getKueryUiFilterES(uiFilters.kuery), | |
...getEnvironmentUiFilterES(uiFilters.environment), | |
...mappedFilters | |
] as ESFilter[]; |
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
…lastic#71229) * Closes elastic#70440 by adding a setup link to anomaly detection setting in the home header * PR feedback and type error fix * Code cleanup and PR feedback * Modified getEnvironmentUiFilterES return type from `ESFilter | undefined` to `ESFilter[]` for ease of use. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* master: (28 commits) skip flaky suite (elastic#71361) [Ingest Manager] Add UI to enroll standalone agent (elastic#71288) Node options from cfg file for production (elastic#62468) [APM] Improvements to the ML Settings page (elastic#71309) add old .chromium to gitignore to prevent it from being accidentally committed [Ingest Manager] Simplify add/edit package config (integration) form (elastic#71187) Ensure Other bucket works on scripted fields. (elastic#71329) [APM] Anomaly detection setup link with alert if job doesn't exist (elastic#71229) [APM] Anomaly detection integration with transaction duration chart (elastic#71230) inclusive language (elastic#71438) [Ingest Manager] During fleet setup create an enrollment for every config (elastic#71308) Improvements to our developer guide (elastic#67764) [SIEM][Detections] Fixes index patterns order (elastic#71270) [Metrics + Logs UI] Add test for logs and metrics telemetry (elastic#70858) [Maps] Inclusive language (elastic#71427) [Logs UI] Unskip log highlight api integration test (elastic#71058) [Security_Solution][Resolver] Style adjustments per UX (elastic#71179) [Functional test] Increase the timeout to click new vis function (elastic#71226) [Discover] Migrate async import of embeddable factory to actual embeddable (elastic#70920) fix overflow (elastic#70723) ...
…71229) (#71459) * Closes #70440 by adding a setup link to anomaly detection setting in the home header * PR feedback and type error fix * Code cleanup and PR feedback * Modified getEnvironmentUiFilterES return type from `ESFilter | undefined` to `ESFilter[]` for ease of use. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Closes elastic#70440 by adding a setup link to anomaly detection setting in the home header.