[APM] Correlations Beta (#86477)#89952
Conversation
82d477e to
63da7e6
Compare
x-pack/plugins/apm/public/components/app/Correlations/correlations_table.tsx
Outdated
Show resolved
Hide resolved
|
Pinging @elastic/apm-ui (Team:apm) |
x-pack/plugins/apm/public/components/app/Correlations/useFieldNames.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/apm/public/components/app/Correlations/useFieldNames.ts
Outdated
Show resolved
Hide resolved
| const [defaultFieldNames, setDefaultFieldNames] = useState( | ||
| getDefaultFieldNames(indexPattern, isRumAgent) | ||
| ); |
There was a problem hiding this comment.
It feels off that this is stateful. If for perf reasons you can use useMemo.
But first try to strip it down and see if it's needed
There was a problem hiding this comment.
It's stateful because useDynamicIndexPatternFetcher is an async fetcher, so it needs to trigger a re-render when the response comes back.
- 'show_correlations_flyout' - 'customize_correlations_fields' - 'select_significant_term'
| }) { | ||
| const { start, end, apmEventClient } = setup; | ||
| const { intervalString } = getBucketSize({ start, end, numBuckets: 30 }); | ||
| const { intervalString } = getBucketSize({ start, end, numBuckets: 15 }); |
There was a problem hiding this comment.
I think this will still return a dynamic number of buckets to aim for a "nice" bucket size.
Some feedback I got was to make the number of buckets fixed.
There was a problem hiding this comment.
We'll fix this in a separate issue: #91574
| const buckets = dropRightWhile( | ||
| distribution.dist_filtered_by_latency.buckets, | ||
| (bucket, index) => bucket.doc_count === 0 && index > intervalBuckets - 1 | ||
| ); |
There was a problem hiding this comment.
I don't understand why we are doing this. If we want fewer bucket returned shouldn't we specify a larger interval?
There was a problem hiding this comment.
Btw. all this should matter less if we use a logarithmic scale. Did we decide to punt on that?
There was a problem hiding this comment.
I created a separate issue for the distribution scale: #91574
|
jenkins run the e2e |
| "apm:enableServiceOverview": { | ||
| "type": "boolean" | ||
| }, | ||
| "apm:enableCorrelations": { |
There was a problem hiding this comment.
FYI, You'd need to open a corresponding mapping update request in the infra repo to have this key removed from the telemetry mappings.
If you don't care that it's not there in versions from 7.12, then you won't need to open an issue for that.
There was a problem hiding this comment.
We'll probably remove it from the mapping in the infra repo in 7.13. That way we'll be able to tell if users are still using it as a custom UI Setting for version 7.12.
TinaHeiligers
left a comment
There was a problem hiding this comment.
I reviewed both the ui_settings key removal and the usage collection schema update for telemetry.
LGTM
|
jenkins run the e2e |
| { | ||
| range: { | ||
| [TRANSACTION_DURATION]: { gte: durationForPercentile }, | ||
| filter: backgroundFilters, |
There was a problem hiding this comment.
What's the reason for removing the range query for duration? Will removing it not hurt perf since more documents are retrieved?
There was a problem hiding this comment.
moving the range query to the function_score provided us with a score that we needed to order foreground results that were most important to us (those with higher durations)
| [TRANSACTION_DURATION]: { | ||
| lt: durationForPercentile, | ||
| }, |
There was a problem hiding this comment.
Isn't the foreground supposed to be a subset of the background?
There was a problem hiding this comment.
... if not you may have to set this
"background_is_superset": falseThere was a problem hiding this comment.
We found that the existing background filter included too much uninteresting data and it was skewing the results of significant terms agg to be not as helpful see(https://youtu.be/azP15yvbOBA)
| bgCount: bucket.bg_count, | ||
| fgCount: bucket.doc_count, | ||
| fieldCount: agg.doc_count, | ||
| valueCount: bucket.doc_count, |
There was a problem hiding this comment.
Not sure about this change. Can you elaborate a bit?
There was a problem hiding this comment.
the previous bgCount wasn't needed since it represents only values for that field/value pair, when we really wanted to express the percentage in terms of the total number of background documents the aggregation applies to. E.g. user_agent.name: HeadlessChrome at the 75th percentile threshold accounts for only 8% of traffic.
There was a problem hiding this comment.
previously it showed something like "80% of documents with user_agent.name: HeadlessChrome exist in the 75th percentile threshold", so it's a question of what is easier to understand for the user.
|
jenkins run the e2e |
1 similar comment
|
jenkins run the e2e |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
* [APM] Correlations GA (elastic#86477) * polish and improvements to correlations UI * more improvements and polish * added impact bar * added descriptions * make custom field persistence be unique per service * make custom threshold unique per service in latency correlations * adds telemetry for apm correlations feature. Events: - 'show_correlations_flyout' - 'customize_correlations_fields' - 'select_significant_term' * adds more telemetry for correlations (elastic#90622) * removes the raw score column * replaces experiemental callout with beta badge * replaces threshold number input with percentile option selector * improvements to latency correlations scoring and percentage reporting * removes the 'apm:enableCorrelations' UI setting * - rename useFieldNames.ts -> use_field_names.ts - filter out fields that are not type 'keyword' - feedback improvements * Fixes casing issue for the 'correlations' dir * [APM] Moves correlations button to service details tabslist row (elastic#91080) * [APM] Adds license check for correlations (elastic#90766) * [APM] Adds metrics tracking for correlations views and license prompts (elastic#90622) * Updated the API integration tests to check for new default fields and 15 buckets Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* [APM] Correlations GA (#86477) * polish and improvements to correlations UI * more improvements and polish * added impact bar * added descriptions * make custom field persistence be unique per service * make custom threshold unique per service in latency correlations * adds telemetry for apm correlations feature. Events: - 'show_correlations_flyout' - 'customize_correlations_fields' - 'select_significant_term' * adds more telemetry for correlations (#90622) * removes the raw score column * replaces experiemental callout with beta badge * replaces threshold number input with percentile option selector * improvements to latency correlations scoring and percentage reporting * removes the 'apm:enableCorrelations' UI setting * - rename useFieldNames.ts -> use_field_names.ts - filter out fields that are not type 'keyword' - feedback improvements * Fixes casing issue for the 'correlations' dir * [APM] Moves correlations button to service details tabslist row (#91080) * [APM] Adds license check for correlations (#90766) * [APM] Adds metrics tracking for correlations views and license prompts (#90622) * Updated the API integration tests to check for new default fields and 15 buckets Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* master: (157 commits) [DOCS] Adds machine learning to the security section of alerting (elastic#91501) [Uptime] Ping list step screenshot caption formatting (elastic#91403) [Vislib] Use timestamp on brush event instead of iso dates (elastic#91483) [Application Usage] Remove deprecated & unused legacy.appChanged API (elastic#91464) Migrate logstash, monitoring, url_drilldowns, xpack_legacy to ts projects (elastic#91194) [APM] Wrap Elasticsearch client errors (elastic#91125) [APM] Fix optimize-tsconfig script (elastic#91487) [Discover][docs] Add searchFieldsFromSource description (elastic#90980) Adds support for 'ip' data type (elastic#85087) [Detection Rules] Add updates from 7.11.2 rules (elastic#91553) [SECURITY SOLUTION] Eql in timeline (elastic#90816) [APM] Correlations Beta (elastic#86477) (elastic#89952) [Security Solutions][Detection Engine] Adds a warning banner when the alerts data has not been migrated yet. (elastic#90258) [Security Solution] [Timeline] Endpoint row renderers (2nd batch) (elastic#91446) skip flaky suite (elastic#91450) skip flaky suite (elastic#91592) [Security Solution][Endpoint][Admin] Endpoint Details UX Enhancements (elastic#90870) [ML] Add better UI support for runtime fields Transforms (elastic#90363) [Security Solution] [Detections] Replace 'partial failure' with 'warning' for rule statuses (elastic#91167) [Security Solution][Detections] Adds Indicator path config for indicator match rules (elastic#91260) ...
Closes #86477.
Closes #91080.
Closes #90766.
Closes #90622.