Browse Source

fix(insights): Tighten filters for Requests module (#70875)

Some clients are seeing wacky data because they have spans with ops like
`http.client.response_body`. This does _not_ affect metrics (those are
strict) but it does affect samples, we fetch incorrect ones sometimes. I
opted to make it consistent everywhere, and only fetch the correct span
ops, it seems more explicit.

- Extract base filters to variable
    
    Instead of remembering to pass `ModuleName` filter in every place,
    remember to pass the base filters to every place, which is slightly
    better.

- Add `span.op` base filter
    
    `span.module:http` isn't tight enough. Some SDKs create spans that
    plausibly have span ops that belong to the HTTP module (e.g.,
    `http.client.response_body` but should not be in the metrics or
    shown in the samples lists.
    
    Make this explicit with a new default filter.
George Gritsouk 10 months ago
parent
commit
f33aa8776c

+ 5 - 5
static/app/views/performance/http/httpDomainSummaryPage.spec.tsx

@@ -93,7 +93,7 @@ describe('HTTPSummaryPage', function () {
           partial: 1,
           per_page: 50,
           project: [],
-          query: 'span.module:http span.domain:"\\*.sentry.dev"',
+          query: 'span.module:http span.op:http.client span.domain:"\\*.sentry.dev"',
           referrer: 'api.performance.http.domain-summary-throughput-chart',
           statsPeriod: '10d',
           topEvents: undefined,
@@ -118,7 +118,7 @@ describe('HTTPSummaryPage', function () {
           partial: 1,
           per_page: 50,
           project: [],
-          query: 'span.module:http span.domain:"\\*.sentry.dev"',
+          query: 'span.module:http span.op:http.client span.domain:"\\*.sentry.dev"',
           referrer: 'api.performance.http.domain-summary-duration-chart',
           statsPeriod: '10d',
           topEvents: undefined,
@@ -143,7 +143,7 @@ describe('HTTPSummaryPage', function () {
           partial: 1,
           per_page: 50,
           project: [],
-          query: 'span.module:http span.domain:"\\*.sentry.dev"',
+          query: 'span.module:http span.op:http.client span.domain:"\\*.sentry.dev"',
           referrer: 'api.performance.http.domain-summary-response-code-chart',
           statsPeriod: '10d',
           topEvents: undefined,
@@ -175,7 +175,7 @@ describe('HTTPSummaryPage', function () {
           ],
           per_page: 50,
           project: [],
-          query: 'span.module:http span.domain:"\\*.sentry.dev"',
+          query: 'span.module:http span.op:http.client span.domain:"\\*.sentry.dev"',
           referrer: 'api.performance.http.domain-summary-metrics-ribbon',
           statsPeriod: '10d',
         },
@@ -205,7 +205,7 @@ describe('HTTPSummaryPage', function () {
           per_page: 20,
           project: [],
           cursor: '0:20:0',
-          query: 'span.module:http span.domain:"\\*.sentry.dev"',
+          query: 'span.module:http span.op:http.client span.domain:"\\*.sentry.dev"',
           sort: '-time_spent_percentage()',
           referrer: 'api.performance.http.domain-summary-transactions-list',
           statsPeriod: '10d',

+ 3 - 3
static/app/views/performance/http/httpDomainSummaryPage.tsx

@@ -33,6 +33,7 @@ import {DomainStatusLink} from 'sentry/views/performance/http/components/domainS
 import {HTTPSamplesPanel} from 'sentry/views/performance/http/httpSamplesPanel';
 import {Referrer} from 'sentry/views/performance/http/referrers';
 import {
+  BASE_FILTERS,
   MODULE_TITLE,
   NULL_DOMAIN_DESCRIPTION,
   RELEASE_LEVEL,
@@ -49,7 +50,7 @@ import {getTimeSpentExplanation} from 'sentry/views/starfish/components/tableCel
 import {useSpanMetrics} from 'sentry/views/starfish/queries/useDiscover';
 import {useSpanMetricsSeries} from 'sentry/views/starfish/queries/useDiscoverSeries';
 import type {SpanMetricsQueryFilters} from 'sentry/views/starfish/types';
-import {ModuleName, SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
+import {SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
 
@@ -76,9 +77,8 @@ export function HTTPDomainSummaryPage() {
   });
 
   const project = projects.find(p => projectId === p.id);
-
   const filters: SpanMetricsQueryFilters = {
-    'span.module': ModuleName.HTTP,
+    ...BASE_FILTERS,
     'span.domain': domain === '' ? EMPTY_OPTION_VALUE : escapeFilterValue(domain),
   };
 

+ 4 - 4
static/app/views/performance/http/httpLandingPage.spec.tsx

@@ -162,7 +162,7 @@ describe('HTTPLandingPage', function () {
           partial: 1,
           per_page: 50,
           project: [],
-          query: 'span.module:http',
+          query: 'span.module:http span.op:http.client',
           referrer: 'api.performance.http.landing-throughput-chart',
           statsPeriod: '10d',
           topEvents: undefined,
@@ -187,7 +187,7 @@ describe('HTTPLandingPage', function () {
           partial: 1,
           per_page: 50,
           project: [],
-          query: 'span.module:http',
+          query: 'span.module:http span.op:http.client',
           referrer: 'api.performance.http.landing-duration-chart',
           statsPeriod: '10d',
           topEvents: undefined,
@@ -212,7 +212,7 @@ describe('HTTPLandingPage', function () {
           partial: 1,
           per_page: 50,
           project: [],
-          query: 'span.module:http',
+          query: 'span.module:http span.op:http.client',
           referrer: 'api.performance.http.landing-response-code-chart',
           statsPeriod: '10d',
           topEvents: undefined,
@@ -246,7 +246,7 @@ describe('HTTPLandingPage', function () {
           ],
           per_page: 10,
           project: [],
-          query: 'span.module:http span.domain:*git*',
+          query: 'span.module:http span.op:http.client span.domain:*git*',
           referrer: 'api.performance.http.landing-domains-list',
           sort: '-time_spent_percentage()',
           statsPeriod: '10d',

+ 7 - 4
static/app/views/performance/http/httpLandingPage.tsx

@@ -23,7 +23,11 @@ import {DurationChart} from 'sentry/views/performance/http/charts/durationChart'
 import {ResponseRateChart} from 'sentry/views/performance/http/charts/responseRateChart';
 import {ThroughputChart} from 'sentry/views/performance/http/charts/throughputChart';
 import {Referrer} from 'sentry/views/performance/http/referrers';
-import {MODULE_TITLE, RELEASE_LEVEL} from 'sentry/views/performance/http/settings';
+import {
+  BASE_FILTERS,
+  MODULE_TITLE,
+  RELEASE_LEVEL,
+} from 'sentry/views/performance/http/settings';
 import {
   DomainsTable,
   isAValidSort,
@@ -34,7 +38,6 @@ import Onboarding from 'sentry/views/performance/onboarding';
 import {useSynchronizeCharts} from 'sentry/views/starfish/components/chart';
 import {useSpanMetrics} from 'sentry/views/starfish/queries/useDiscover';
 import {useSpanMetricsSeries} from 'sentry/views/starfish/queries/useDiscoverSeries';
-import {ModuleName} from 'sentry/views/starfish/types';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 export function HTTPLandingPage() {
@@ -54,11 +57,11 @@ export function HTTPLandingPage() {
   });
 
   const chartFilters = {
-    'span.module': ModuleName.HTTP,
+    ...BASE_FILTERS,
   };
 
   const tableFilters = {
-    'span.module': ModuleName.HTTP,
+    ...BASE_FILTERS,
     'span.domain': query['span.domain'] ? `*${query['span.domain']}*` : undefined,
   };
 

+ 6 - 5
static/app/views/performance/http/httpSamplesPanel.spec.tsx

@@ -191,7 +191,8 @@ describe('HTTPSamplesPanel', () => {
             ],
             per_page: 50,
             project: [],
-            query: 'span.module:http !has:span.domain transaction:/api/0/users',
+            query:
+              'span.module:http span.op:http.client !has:span.domain transaction:/api/0/users',
             referrer: 'api.performance.http.samples-panel-metrics-ribbon',
             statsPeriod: '10d',
           },
@@ -215,7 +216,7 @@ describe('HTTPSamplesPanel', () => {
             per_page: 50,
             project: [],
             query:
-              'span.module:http !has:span.domain transaction:/api/0/users span.status_code:[300,301,302,303,304,305,307,308]',
+              'span.module:http span.op:http.client !has:span.domain transaction:/api/0/users span.status_code:[300,301,302,303,304,305,307,308]',
             referrer: 'api.performance.http.samples-panel-response-code-chart',
             statsPeriod: '10d',
             topEvents: '5',
@@ -232,7 +233,7 @@ describe('HTTPSamplesPanel', () => {
           query: expect.objectContaining({
             dataset: 'spansIndexed',
             query:
-              'span.module:http transaction:/api/0/users span.status_code:[300,301,302,303,304,305,307,308] ( !has:span.domain OR span.domain:[""] )',
+              'span.module:http span.op:http.client transaction:/api/0/users span.status_code:[300,301,302,303,304,305,307,308] ( !has:span.domain OR span.domain:[""] )',
             project: [],
             field: [
               'project',
@@ -349,7 +350,7 @@ describe('HTTPSamplesPanel', () => {
             per_page: 50,
             project: [],
             query:
-              'span.module:http span.domain:"\\*.sentry.dev" transaction:/api/0/users',
+              'span.module:http span.op:http.client span.domain:"\\*.sentry.dev" transaction:/api/0/users',
             referrer: 'api.performance.http.samples-panel-duration-chart',
             statsPeriod: '10d',
             yAxis: 'avg(span.self_time)',
@@ -364,7 +365,7 @@ describe('HTTPSamplesPanel', () => {
           method: 'GET',
           query: expect.objectContaining({
             query:
-              'span.module:http transaction:/api/0/users span.domain:"\\*.sentry.dev"',
+              'span.module:http span.op:http.client transaction:/api/0/users span.domain:"\\*.sentry.dev"',
             project: [],
             additionalFields: [
               'trace',

+ 3 - 3
static/app/views/performance/http/httpSamplesPanel.tsx

@@ -31,6 +31,7 @@ import {useSpanSamples} from 'sentry/views/performance/http/data/useSpanSamples'
 import decodePanel from 'sentry/views/performance/http/queryParameterDecoders/panel';
 import decodeResponseCodeClass from 'sentry/views/performance/http/queryParameterDecoders/responseCodeClass';
 import {Referrer} from 'sentry/views/performance/http/referrers';
+import {BASE_FILTERS} from 'sentry/views/performance/http/settings';
 import {SpanSamplesTable} from 'sentry/views/performance/http/tables/spanSamplesTable';
 import {useDebouncedState} from 'sentry/views/performance/http/useDebouncedState';
 import {MetricReadout} from 'sentry/views/performance/metricReadout';
@@ -43,7 +44,6 @@ import {useSpanMetricsSeries} from 'sentry/views/starfish/queries/useDiscoverSer
 import {useIndexedSpans} from 'sentry/views/starfish/queries/useIndexedSpans';
 import {useSpanMetricsTopNSeries} from 'sentry/views/starfish/queries/useSpanMetricsTopNSeries';
 import {
-  ModuleName,
   SpanFunction,
   SpanIndexedField,
   SpanMetricsField,
@@ -109,7 +109,7 @@ export function HTTPSamplesPanel() {
 
   // The ribbon is above the data selectors, and not affected by them. So, it has its own filters.
   const ribbonFilters: SpanMetricsQueryFilters = {
-    'span.module': ModuleName.HTTP,
+    ...BASE_FILTERS,
     'span.domain':
       query.domain === '' ? EMPTY_OPTION_VALUE : escapeFilterValue(query.domain),
     transaction: query.transaction,
@@ -117,7 +117,7 @@ export function HTTPSamplesPanel() {
 
   // These filters are for the charts and samples tables
   const filters: SpanMetricsQueryFilters = {
-    'span.module': ModuleName.HTTP,
+    ...BASE_FILTERS,
     'span.domain':
       query.domain === '' ? EMPTY_OPTION_VALUE : escapeFilterValue(query.domain),
     transaction: query.transaction,

+ 6 - 0
static/app/views/performance/http/settings.ts

@@ -1,5 +1,6 @@
 import type {BadgeType} from 'sentry/components/badge/featureBadge';
 import {t} from 'sentry/locale';
+import {ModuleName} from 'sentry/views/starfish/types';
 
 export const MODULE_TITLE = t('Requests');
 
@@ -16,3 +17,8 @@ export const releaseLevelAsBadgeProps = {
 
 export const CHART_HEIGHT = 160;
 export const SPAN_ID_DISPLAY_LENGTH = 16;
+
+export const BASE_FILTERS = {
+  'span.module': ModuleName.HTTP,
+  'span.op': 'http.client', // `span.module` alone isn't enough, since some SDKs create other `http.*` spans like `http.client.response_body`
+};