-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ref(tracemetrics): Add more analytics events for metrics #102824
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import type {Sort} from 'sentry/utils/discover/fields'; | |
| import {DiscoverDatasets} from 'sentry/utils/discover/types'; | ||
| import {MutableSearch} from 'sentry/utils/tokenizeSearch'; | ||
| import useOrganization from 'sentry/utils/useOrganization'; | ||
| import usePageFilters from 'sentry/utils/usePageFilters'; | ||
| import type {TimeSeries} from 'sentry/views/dashboards/widgets/common/types'; | ||
| import {useLogsAutoRefreshEnabled} from 'sentry/views/explore/contexts/logs/logsAutoRefreshContext'; | ||
| import {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode'; | ||
|
|
@@ -19,6 +20,8 @@ import type {TracesTableResult} from 'sentry/views/explore/hooks/useExploreTrace | |
| import {useTopEvents} from 'sentry/views/explore/hooks/useTopEvents'; | ||
| import {type useLogsAggregatesTable} from 'sentry/views/explore/logs/useLogsAggregatesTable'; | ||
| import type {UseInfiniteLogsQueryResult} from 'sentry/views/explore/logs/useLogsQuery'; | ||
| import {useMetricAggregatesTable} from 'sentry/views/explore/metrics/hooks/useMetricAggregatesTable'; | ||
| import {useMetricSamplesTable} from 'sentry/views/explore/metrics/hooks/useMetricSamplesTable'; | ||
| import type {ReadableExploreQueryParts} from 'sentry/views/explore/multiQueryMode/locationUtils'; | ||
| import { | ||
| useQueryParamsFields, | ||
|
|
@@ -27,6 +30,7 @@ import { | |
| useQueryParamsTitle, | ||
| useQueryParamsVisualizes, | ||
| } from 'sentry/views/explore/queryParams/context'; | ||
| import type {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams'; | ||
| import {Visualize} from 'sentry/views/explore/queryParams/visualize'; | ||
| import {useSpansDataset} from 'sentry/views/explore/spans/spansQueryParams'; | ||
| import { | ||
|
|
@@ -685,6 +689,260 @@ function computeEmptyBuckets( | |
| }); | ||
| } | ||
|
|
||
| export function useMetricsPanelAnalytics({ | ||
| interval, | ||
| isTopN, | ||
| metricAggregatesTableResult, | ||
| metricSamplesTableResult, | ||
| metricTimeseriesResult, | ||
| mode, | ||
| yAxis, | ||
| sortBys, | ||
| aggregateSortBys, | ||
| }: { | ||
| aggregateSortBys: readonly Sort[]; | ||
| interval: string; | ||
| isTopN: boolean; | ||
| metricAggregatesTableResult: ReturnType<typeof useMetricAggregatesTable>; | ||
| metricSamplesTableResult: ReturnType<typeof useMetricSamplesTable>; | ||
| metricTimeseriesResult: ReturnType<typeof useSortedTimeSeries>; | ||
| mode: Mode; | ||
| sortBys: readonly Sort[]; | ||
| yAxis: string; | ||
| }) { | ||
| const organization = useOrganization(); | ||
|
|
||
| const dataset = DiscoverDatasets.METRICS; | ||
| const dataScanned = metricSamplesTableResult.result.meta?.dataScanned ?? ''; | ||
| const search = useQueryParamsSearch(); | ||
| const query = useQueryParamsQuery(); | ||
| const fields = useQueryParamsFields(); | ||
|
|
||
| const tableError = | ||
| mode === Mode.AGGREGATE | ||
| ? (metricAggregatesTableResult.result.error?.message ?? '') | ||
| : (metricSamplesTableResult.error?.message ?? ''); | ||
| const query_status = tableError ? 'error' : 'success'; | ||
|
|
||
| const aggregatesResultLengthBox = useBox( | ||
| metricAggregatesTableResult.result.data?.length || 0 | ||
| ); | ||
| const resultLengthBox = useBox(metricSamplesTableResult.result.data?.length || 0); | ||
| const fieldsBox = useBox(fields); | ||
| const yAxesBox = useBox([yAxis]); | ||
| const sortBysBox = useBox(sortBys.map(formatSort)); | ||
| const aggregateSortBysBox = useBox(aggregateSortBys.map(formatSort)); | ||
|
|
||
| const timeseriesData = useBox(metricTimeseriesResult.data); | ||
|
|
||
| useEffect(() => { | ||
| if (mode !== Mode.SAMPLES) { | ||
| return; | ||
| } | ||
|
|
||
| if (metricSamplesTableResult.result.isFetching) { | ||
| return; | ||
| } | ||
|
|
||
| trackAnalytics('metrics.explorer.panel.metadata', { | ||
| organization, | ||
| dataset, | ||
| dataScanned, | ||
| columns: fieldsBox.current, | ||
| columns_count: fieldsBox.current.length, | ||
| confidences: computeConfidence(yAxesBox.current, timeseriesData.current), | ||
| empty_buckets_percentage: computeEmptyBuckets( | ||
| yAxesBox.current, | ||
| timeseriesData.current | ||
| ), | ||
| interval, | ||
| query_status, | ||
| sample_counts: computeVisualizeSampleTotals( | ||
| yAxesBox.current, | ||
| timeseriesData.current, | ||
| isTopN | ||
| ), | ||
| table_result_length: resultLengthBox.current, | ||
| table_result_missing_root: 0, | ||
| table_result_mode: 'metric samples', | ||
| table_result_sort: sortBysBox.current, | ||
| user_queries: search.formatString(), | ||
| user_queries_count: search.tokens.length, | ||
| }); | ||
|
|
||
| info( | ||
| fmt`metric.explorer.panel.metadata: | ||
| organization: ${organization.slug} | ||
| dataScanned: ${dataScanned} | ||
| dataset: ${dataset} | ||
| query: ${query} | ||
| fields: ${fieldsBox.current} | ||
| query_status: ${query_status} | ||
| result_length: ${String(resultLengthBox.current)} | ||
| user_queries: ${search.formatString()} | ||
| user_queries_count: ${String(search.tokens.length)} | ||
| `, | ||
| {isAnalytics: true} | ||
| ); | ||
| }, [ | ||
| organization, | ||
| dataset, | ||
| dataScanned, | ||
| query, | ||
| fieldsBox, | ||
| interval, | ||
| query_status, | ||
| isTopN, | ||
| metricSamplesTableResult.result.isFetching, | ||
| search, | ||
| timeseriesData, | ||
| mode, | ||
| resultLengthBox, | ||
| sortBysBox, | ||
| yAxesBox, | ||
| ]); | ||
|
|
||
| useEffect(() => { | ||
| if (mode !== Mode.AGGREGATE) { | ||
| return; | ||
| } | ||
|
|
||
| if (metricAggregatesTableResult.result.isPending) { | ||
| return; | ||
| } | ||
|
|
||
| trackAnalytics('metrics.explorer.panel.metadata', { | ||
| organization, | ||
| dataset, | ||
| dataScanned, | ||
| columns: fieldsBox.current, | ||
| columns_count: fieldsBox.current.length, | ||
| confidences: computeConfidence([yAxis], timeseriesData.current), | ||
| empty_buckets_percentage: computeEmptyBuckets([yAxis], timeseriesData.current), | ||
| interval, | ||
| query_status, | ||
| sample_counts: computeVisualizeSampleTotals( | ||
| [yAxis], | ||
| timeseriesData.current, | ||
| isTopN | ||
| ), | ||
| table_result_length: aggregatesResultLengthBox.current, | ||
| table_result_missing_root: 0, | ||
| table_result_mode: 'aggregates', | ||
| table_result_sort: aggregateSortBysBox.current, | ||
| user_queries: search.formatString(), | ||
| user_queries_count: search.tokens.length, | ||
| }); | ||
|
|
||
| info( | ||
| fmt`metric.explorer.panel.metadata: | ||
| organization: ${organization.slug} | ||
| dataScanned: ${dataScanned} | ||
| dataset: ${dataset} | ||
| query: ${query} | ||
| fields: ${fieldsBox.current} | ||
| query_status: ${query_status} | ||
| result_length: ${String(aggregatesResultLengthBox.current)} | ||
| user_queries: ${search.formatString()} | ||
| user_queries_count: ${String(search.tokens.length)} | ||
| `, | ||
| {isAnalytics: true} | ||
| ); | ||
| }, [ | ||
| aggregateSortBysBox, | ||
| aggregatesResultLengthBox, | ||
| dataScanned, | ||
| dataset, | ||
| fieldsBox, | ||
| interval, | ||
| isTopN, | ||
| metricAggregatesTableResult.result.isPending, | ||
| timeseriesData, | ||
| mode, | ||
| organization, | ||
| query, | ||
| query_status, | ||
| search, | ||
| yAxis, | ||
| ]); | ||
| } | ||
|
|
||
| export function useMetricsAnalytics({ | ||
| interval, | ||
| metricQueries, | ||
| }: { | ||
| interval: string; | ||
| metricQueries: Array<{queryParams: ReadableQueryParams}>; | ||
| }) { | ||
| const organization = useOrganization(); | ||
| const {selection} = usePageFilters(); | ||
|
|
||
| const { | ||
| data: {hasExceededPerformanceUsageLimit}, | ||
| isLoading: isLoadingSubscriptionDetails, | ||
| } = usePerformanceSubscriptionDetails({traceItemDataset: 'default'}); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Use Metrics Dataset for Trace Usage ChecksThe |
||
|
|
||
| const metricQueriesCount = useBox(metricQueries.length); | ||
| const metricPanelsWithGroupBysCount = useBox( | ||
| metricQueries.filter(mq => | ||
| mq.queryParams.groupBys.some((gb: string) => gb.trim().length > 0) | ||
| ).length | ||
| ); | ||
| const metricPanelsWithFiltersCount = useBox( | ||
| metricQueries.filter(mq => mq.queryParams.query.trim().length > 0).length | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (isLoadingSubscriptionDetails) { | ||
| return; | ||
| } | ||
|
|
||
| const datetimeSelection = `${selection.datetime.start || ''}-${selection.datetime.end || ''}-${selection.datetime.period || ''}`; | ||
| const projectCount = selection.projects.length; | ||
| const environmentCount = selection.environments.length; | ||
|
|
||
| trackAnalytics('metrics.explorer.metadata', { | ||
| organization, | ||
| datetime_selection: datetimeSelection, | ||
| environment_count: environmentCount, | ||
| has_exceeded_performance_usage_limit: hasExceededPerformanceUsageLimit, | ||
| interval, | ||
| metric_panels_with_filters_count: metricPanelsWithFiltersCount.current, | ||
| metric_panels_with_group_bys_count: metricPanelsWithGroupBysCount.current, | ||
| metric_queries_count: metricQueriesCount.current, | ||
| project_count: projectCount, | ||
| }); | ||
|
|
||
| info( | ||
| fmt`metrics.explorer.metadata: | ||
| organization: ${organization.slug} | ||
| datetime_selection: ${datetimeSelection} | ||
| environment_count: ${String(environmentCount)} | ||
| interval: ${interval} | ||
| metric_queries_count: ${String(metricQueriesCount.current)} | ||
| metric_panels_with_group_bys_count: ${String(metricPanelsWithGroupBysCount.current)} | ||
| metric_panels_with_filters_count: ${String(metricPanelsWithFiltersCount.current)} | ||
| project_count: ${String(projectCount)} | ||
| has_exceeded_performance_usage_limit: ${String(hasExceededPerformanceUsageLimit)} | ||
| `, | ||
| {isAnalytics: true} | ||
| ); | ||
| }, [ | ||
| hasExceededPerformanceUsageLimit, | ||
| interval, | ||
| isLoadingSubscriptionDetails, | ||
| metricQueriesCount, | ||
| metricPanelsWithGroupBysCount, | ||
| metricPanelsWithFiltersCount, | ||
| organization, | ||
| selection.datetime.end, | ||
| selection.datetime.period, | ||
| selection.datetime.start, | ||
| selection.environments.length, | ||
| selection.projects.length, | ||
| ]); | ||
| } | ||
|
|
||
| function useBox<T>(value: T): RefObject<T> { | ||
| const box = useRef(value); | ||
| box.current = value; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,26 @@ import {useState} from 'react'; | |
|
|
||
| import Panel from 'sentry/components/panels/panel'; | ||
| import PanelBody from 'sentry/components/panels/panelBody'; | ||
| import {useMetricsPanelAnalytics} from 'sentry/views/explore/hooks/useAnalytics'; | ||
| import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval'; | ||
| import {useTopEvents} from 'sentry/views/explore/hooks/useTopEvents'; | ||
| import {TraceSamplesTableColumns} from 'sentry/views/explore/metrics/constants'; | ||
| import {useMetricAggregatesTable} from 'sentry/views/explore/metrics/hooks/useMetricAggregatesTable'; | ||
| import {useMetricSamplesTable} from 'sentry/views/explore/metrics/hooks/useMetricSamplesTable'; | ||
| import {useMetricTimeseries} from 'sentry/views/explore/metrics/hooks/useMetricTimeseries'; | ||
| import {useTableOrientationControl} from 'sentry/views/explore/metrics/hooks/useOrientationControl'; | ||
| import {SideBySideOrientation} from 'sentry/views/explore/metrics/metricPanel/sideBySideOrientation'; | ||
| import {StackedOrientation} from 'sentry/views/explore/metrics/metricPanel/stackedOrientation'; | ||
| import {type TraceMetric} from 'sentry/views/explore/metrics/metricQuery'; | ||
| import {getMetricTableColumnType} from 'sentry/views/explore/metrics/utils'; | ||
| import { | ||
| useQueryParamsAggregateSortBys, | ||
| useQueryParamsMode, | ||
| useQueryParamsSortBys, | ||
| } from 'sentry/views/explore/queryParams/context'; | ||
|
|
||
| const RESULT_LIMIT = 50; | ||
| const TWO_MINUTE_DELAY = 120; | ||
|
|
||
| interface MetricPanelProps { | ||
| queryIndex: number; | ||
|
|
@@ -25,6 +40,41 @@ export function MetricPanel({traceMetric, queryIndex}: MetricPanelProps) { | |
| enabled: Boolean(traceMetric.name), | ||
| }); | ||
|
|
||
| const columns = TraceSamplesTableColumns; | ||
| const fields = columns.filter(c => getMetricTableColumnType(c) !== 'stat'); | ||
|
|
||
| const metricSamplesTableResult = useMetricSamplesTable({ | ||
| disabled: !traceMetric?.name, | ||
| limit: RESULT_LIMIT, | ||
| traceMetric, | ||
| fields, | ||
| ingestionDelaySeconds: TWO_MINUTE_DELAY, | ||
| }); | ||
|
|
||
| const metricAggregatesTableResult = useMetricAggregatesTable({ | ||
| enabled: Boolean(traceMetric.name), | ||
| limit: RESULT_LIMIT, | ||
| traceMetric, | ||
| }); | ||
|
|
||
| const mode = useQueryParamsMode(); | ||
| const sortBys = useQueryParamsSortBys(); | ||
| const aggregateSortBys = useQueryParamsAggregateSortBys(); | ||
| const [interval] = useChartInterval(); | ||
| const topEvents = useTopEvents(); | ||
|
|
||
| useMetricsPanelAnalytics({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to fire once per panel and again when the query changes for the panel. Is this the intended behaviour? |
||
| interval, | ||
| isTopN: !!topEvents, | ||
| metricAggregatesTableResult, | ||
| metricSamplesTableResult, | ||
| metricTimeseriesResult: timeseriesResult, | ||
| mode, | ||
| yAxis: traceMetric.name || '', | ||
| sortBys, | ||
| aggregateSortBys, | ||
| }); | ||
|
|
||
| return ( | ||
| <Panel> | ||
| <PanelBody> | ||
|
|
||
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.
Bug:
useMetricsAnalyticsusestraceItemDataset: 'default'instead of'metrics', leading to incorrect usage limit checks.Severity: MEDIUM | Confidence: 0.90
🔍 Detailed Analysis
The
useMetricsAnalyticshook incorrectly invokesusePerformanceSubscriptionDetailswithtraceItemDataset: 'default'. This configuration causes the system to evaluate usage limits againsttransactions.usageExceededorspans.usageExceededinstead of the intendedtraceMetrics.usageExceeded. As a result, themetrics.explorer.metadataanalytics event will report an inaccurate value forhas_exceeded_performance_usage_limitconcerning metrics usage.💡 Suggested Fix
Modify the call to
usePerformanceSubscriptionDetailswithinuseMetricsAnalyticsto usetraceItemDataset: 'metrics'instead oftraceItemDataset: 'default'to ensure accurate usage limit evaluation.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.