Skip to content

Commit

Permalink
feat(analytics-sku): remove feature flag mentions [MA-3136] (#1566)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mihai-peteu
mihai-peteu authored Aug 29, 2024
1 parent 6c9bb04 commit 4370394
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 225 deletions.
11 changes: 2 additions & 9 deletions packages/analytics/analytics-config-store/src/stores/analytics-config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export type ConfigStoreState = null | AnalyticsConfigV2

export const useAnalyticsConfigStore = defineStore('analytics-config', () => {
const analyticsConfig = ref<ConfigStoreState>(null)
const skuFeatureFlag = ref<boolean>(false)

const queryBridge: AnalyticsBridge | undefined = inject(INJECT_QUERY_PROVIDER)

Expand All @@ -31,21 +30,15 @@ export const useAnalyticsConfigStore = defineStore('analytics-config', () => {
console.warn('Error fetching analytics config')
console.warn(err)
})

skuFeatureFlag.value = queryBridge.evaluateFeatureFlagFn('MA-2527-analytics-sku-config-endpoint', false)
}

const longRetention = computed<boolean>(() => {
const retentionMs = analyticsConfig.value?.analytics?.retention_ms
return !!retentionMs && retentionMs >= THIRTY_DAYS_MS
})

const defaultQueryTimeForOrg = computed<'24h' | '7d' | '30d'>(() => {
if (skuFeatureFlag.value) {
return '7d'
}

return longRetention.value ? '30d' : '24h'
const defaultQueryTimeForOrg = computed<'7d'>(() => {
return '7d'
})

const loading = computed<boolean>(() => !analyticsConfig.value)
Expand Down
141 changes: 5 additions & 136 deletions packages/analytics/analytics-metric-provider/src/components/MetricsProvider.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ import { mockExploreResponse } from '../mockExploreResponse'
import { INJECT_QUERY_PROVIDER } from '../constants'
import { createPinia, setActivePinia } from 'pinia'

interface MakeQueryBridgeOptions extends MockOptions {
hasTrendAccess?: boolean
skuFeatureFlag?: boolean
}

describe('<AnalyticsMetricProvider />', () => {

// General note when working on these tests: SWRV tends to cache the results of queries between tests.
Expand All @@ -29,7 +24,7 @@ describe('<AnalyticsMetricProvider />', () => {
setActivePinia(createPinia())
})

const makeQueryBridge = (opts?: MakeQueryBridgeOptions): AnalyticsBridge => {
const makeQueryBridge = (opts?: MockOptions): AnalyticsBridge => {
const queryFn = (dsAwareQuery: DatasourceAwareQuery): Promise<ExploreResultV4> => {
const { query } = dsAwareQuery as AdvancedDatasourceQuery

Expand Down Expand Up @@ -62,19 +57,17 @@ describe('<AnalyticsMetricProvider />', () => {
return Promise.resolve(result)
}

const hasTrendAccess = opts?.hasTrendAccess ?? true

const configFn = (): Promise<AnalyticsConfigV2> => Promise.resolve({
analytics: {
percentiles: true,
retention_ms: hasTrendAccess ? 2592000000 : 86400000, // 30d | 1d
retention_ms: 2592000000, // 30d
},
requests: {
retention_ms: 86400000,
},
})

const evaluateFeatureFlagFn: AnalyticsBridge['evaluateFeatureFlagFn'] = () => (opts?.skuFeatureFlag ?? true) as any
const evaluateFeatureFlagFn: AnalyticsBridge['evaluateFeatureFlagFn'] = () => (true) as any

return {
queryFn: cy.spy(queryFn).as('fetcher'),
Expand Down Expand Up @@ -168,36 +161,6 @@ describe('<AnalyticsMetricProvider />', () => {
cy.get('.metricscard-title').eq(2).should('have.text', 'Average Latency')
})

it('renders percentiles if the feature flag is not set', () => {
const queryBridge = makeQueryBridge({ skuFeatureFlag: false })

cy.mount(MetricsTestHarness, {
props: {
render: 'global',
longCardTitles: true,
additionalFilter: [{ type: 'in', dimension: 'api_product', values: ['renders percentiles if the feature flag is not set'] } as ExploreFilter],
},
global: {
provide: {
[INJECT_QUERY_PROVIDER]: queryBridge,
},
},
})

cy.get('@fetcher').should('have.been.calledTwice')

cy.get('@fetcher').should('have.been.calledWithMatch', Cypress.sinon.match({
datasource: 'advanced',
query: { metrics: ['response_latency_p99'] },
}))
cy.get('@fetcher').should('always.have.not.been.calledWithMatch', Cypress.sinon.match({ query: { metrics: ['response_latency_average'] } }))

cy.get('.metricscard').should('exist')
cy.get('.metricscard-title').eq(0).should('have.text', 'Number of Requests')
cy.get('.metricscard-title').eq(1).should('have.text', 'Average Error Rate')
cy.get('.metricscard-title').eq(2).should('have.text', 'P99 Latency')
})

it('renders percentiles if the override is set', () => {
const queryBridge = makeQueryBridge()

Expand Down Expand Up @@ -246,46 +209,8 @@ describe('<AnalyticsMetricProvider />', () => {
cy.get('.container-description').eq(0).should('contain', 'Lorem ipsum golden signal details')
})

it('displays "30 days" if trend access allows', () => {
const queryBridge = makeQueryBridge({ skuFeatureFlag: false })

cy.mount(MetricsTestHarness, {
props: {
render: 'global',
description: 'Lorem ipsum golden signal details',
},
global: {
provide: {
[INJECT_QUERY_PROVIDER]: queryBridge,
},
},
})

cy.get('.metricscard').should('exist')
cy.get('.metricscard').eq(0).find('.metricscard-trend-range').should('contain', 'vs previous 30 days')
})

it('displays "24 hours" if trend access allows', () => {
const queryBridge = makeQueryBridge({ hasTrendAccess: false, skuFeatureFlag: false })

cy.mount(MetricsTestHarness, {
props: {
render: 'global',
description: 'Lorem ipsum golden signal details',
},
global: {
provide: {
[INJECT_QUERY_PROVIDER]: queryBridge,
},
},
})

cy.get('.metricscard').should('exist')
cy.get('.metricscard').eq(0).find('.metricscard-trend-range').should('contain', 'vs previous 24 hours')
})

it('displays "7 days" if the feature flag is set', () => {
const queryBridge = makeQueryBridge({ hasTrendAccess: false, skuFeatureFlag: true })
it('displays "7 days" by default', () => {
const queryBridge = makeQueryBridge()

cy.mount(MetricsTestHarness, {
props: {
Expand All @@ -303,31 +228,6 @@ describe('<AnalyticsMetricProvider />', () => {
cy.get('.metricscard').eq(0).find('.metricscard-trend-range').should('contain', 'vs previous 7 days')
})

it('handles no trend', () => {
const queryBridge = makeQueryBridge({ hasTrendAccess: false, skuFeatureFlag: false })

cy.mount(MetricsTestHarness, {
props: {
render: 'global',
},
global: {
provide: {
[INJECT_QUERY_PROVIDER]: queryBridge,
},
},
})

cy.get('.metricscard').should('exist')

cy.getTestId('metric-value').eq(0).should('have.text', '5K')
cy.getTestId('metric-value').eq(1).should('have.text', '40.00%')
cy.getTestId('metric-value').eq(2).should('have.text', '1001ms')

cy.get('.metricscard-trend-change > div').eq(0).should('have.text', 'N/A')
cy.get('.metricscard-trend-change > div').eq(1).should('have.text', 'N/A')
cy.get('.metricscard-trend-change > div').eq(2).should('have.text', 'N/A')
})

it('handles queryReady', () => {
const queryBridge = makeQueryBridge({ hasTrendAccess: false })

Expand Down Expand Up @@ -454,37 +354,6 @@ describe('<AnalyticsMetricProvider />', () => {
cy.get('.metricscard-trend-change > div').eq(2).should('have.text', '49.98%')
})

it('displays single-entity metrics with no trend', () => {
const queryBridge = makeQueryBridge({
dimensionNames: ['blah'],
hasTrendAccess: false,
skuFeatureFlag: false,
})

cy.mount(MetricsTestHarness, {
props: {
render: 'single',
},
global: {
provide: {
[INJECT_QUERY_PROVIDER]: queryBridge,
},
},
})

cy.get('@fetcher').should('have.been.calledTwice')

cy.get('.metricscard').should('exist')

cy.getTestId('metric-value').eq(0).should('have.text', '5K')
cy.getTestId('metric-value').eq(1).should('have.text', '40.00%')
cy.getTestId('metric-value').eq(2).should('have.text', '1001ms')

cy.get('.metricscard-trend-change > div').eq(0).should('have.text', 'N/A')
cy.get('.metricscard-trend-change > div').eq(1).should('have.text', 'N/A')
cy.get('.metricscard-trend-change > div').eq(2).should('have.text', 'N/A')
})

it('displays multi-entity metrics', () => {
const queryBridge = makeQueryBridge({ dimensionNames: ['blah', 'arrgh'] })

Expand Down
34 changes: 4 additions & 30 deletions packages/analytics/analytics-metric-provider/src/components/MetricsProvider.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,20 @@ if (props.dimension && queryableExploreDimensions.findIndex(x => x === props.dim
const queryBridge: AnalyticsBridge | undefined = inject(INJECT_QUERY_PROVIDER)
let queryFn: AnalyticsBridge['queryFn']
let evaluateFeatureFlagFn: AnalyticsBridge['evaluateFeatureFlagFn']
if (!queryBridge) {
console.warn('Analytics dashboards require a query bridge supplied via provide / inject.')
console.warn("Please ensure your application has a query bridge provided under the key 'analytics-query-provider', as described in")
console.warn('https://github.com/Kong/public-ui-components/blob/main/packages/analytics/analytics-metric-provider/README.md#requirements')
queryFn = () => Promise.reject(new Error('Query bridge required'))
evaluateFeatureFlagFn = (_key, defaultValue) => defaultValue
} else {
queryFn = queryBridge.queryFn
evaluateFeatureFlagFn = queryBridge.evaluateFeatureFlagFn
}
const analyticsConfigStore = useAnalyticsConfigStore()
// Check if the current org has long enough retention to make a sane trend query.
// If the feature flag is set, trend access is always true.
const hasTrendAccess = computed<boolean>(() => skuFeatureFlag.value || analyticsConfigStore.longRetention)
const hasTrendAccess = computed<boolean>(() => true)
// Don't attempt to issue a query until we know what we can query for.
const queryReady = computed(() => !analyticsConfigStore.loading && props.queryReady)
Expand All @@ -89,43 +85,21 @@ const tz = computed(() => {
return (new Intl.DateTimeFormat()).resolvedOptions().timeZone
})
const skuFeatureFlag = computed<boolean>(() => {
// The feature flag client is guaranteed to be initialized by the time the code gets to this place.
return evaluateFeatureFlagFn('MA-2527-analytics-sku-config-endpoint', false)
})
const resolvedDatasource = computed<QueryDatasource>(() => {
if (props.datasource) {
return props.datasource
}
return skuFeatureFlag.value ? 'basic' : 'advanced'
return 'basic'
})
// Note: the component implicitly assumes the values it feeds to the composables aren't going to change.
// If they do need to change, then the `useMetricCardGoldenSignals` composable, and dependencies,
// needs to be reactive as well. Ideally, this would be the case; we don't have any guarantee that the
// tier data has finished loading by the time the component mounts, for example.
const timeframe = computed<Timeframe>(() => {
if (props.overrideTimeframe) {
// Trust that the host component calculated the timeframe for us.
return props.overrideTimeframe
}
if (skuFeatureFlag.value) {
return TimePeriods.get(TimeframeKeys.SEVEN_DAY)!
}
const retval = hasTrendAccess.value
? TimePeriods.get(props.maxTimeframe)
: TimePeriods.get(TimeframeKeys.ONE_DAY)
if (!retval) {
throw new Error('Metrics provider failed to resolve fallback timeframe.')
}
return retval
return props.overrideTimeframe || TimePeriods.get(TimeframeKeys.SEVEN_DAY)!
})
const averageLatencies = computed<boolean>(() => {
Expand All @@ -134,7 +108,7 @@ const averageLatencies = computed<boolean>(() => {
return false
}
return skuFeatureFlag.value
return true
})
const {
Expand Down
Loading

0 comments on commit 4370394

Please sign in to comment.