From c28adbb6a6014aefa868d6da075595fb7a869e54 Mon Sep 17 00:00:00 2001 From: Hariom Gupta <102638746+hari45678@users.noreply.github.com> Date: Wed, 8 Jan 2025 09:12:23 +0530 Subject: [PATCH] [fix]: Remove redux-form dependency from Monitor page (#2562) ## Which problem is this PR solving? - part of #2556 ## Description of the changes - This PR removes dependency of Monitor page on `redux-form`. - To keep this PR small and easy to review, I haven't removed other instances of `redux-form` in this same PR. Neither have I removed `redux-form` package as of now. - `Antd` `SearchableSelect` requires loading type to be `boolean | undefined` so I have changed the related types in other files to fix ts-lint errors. ### Why this change was needed? - Monitor page has a few selectors that doesn't need any global state sync to other components. `redux-form` is anyway deprecating so in-built react state management is a good choice. - React uses `state` object for state management, and `props` for injecting props to the component. `redux-form` use to inject those Dropdown Selector values `selectedService`, `selectedSpanKind`, and `selectedTimeFrame` from redux store. - This PR moves that to in-built react state, so we won't have to inject those from store anymore. ## How was this change tested? - `npm run test` - `npm run start` with monitor page view. everything seems to be working fine. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Hariom Gupta Co-authored-by: Yuri Shkuro --- .../__snapshots__/index.test.js.snap | 158 ++++------------ .../Monitor/ServicesView/index.test.js | 13 +- .../components/Monitor/ServicesView/index.tsx | 172 ++++++++---------- .../operationDetailsTable/index.tsx | 2 +- .../jaeger-ui/src/reducers/metrics.mock.js | 2 +- packages/jaeger-ui/src/reducers/metrics.tsx | 2 +- packages/jaeger-ui/src/types/metrics.tsx | 2 +- 7 files changed, 115 insertions(+), 236 deletions(-) diff --git a/packages/jaeger-ui/src/components/Monitor/ServicesView/__snapshots__/index.test.js.snap b/packages/jaeger-ui/src/components/Monitor/ServicesView/__snapshots__/index.test.js.snap index 498d0c9169..151eb6181d 100644 --- a/packages/jaeger-ui/src/components/Monitor/ServicesView/__snapshots__/index.test.js.snap +++ b/packages/jaeger-ui/src/components/Monitor/ServicesView/__snapshots__/index.test.js.snap @@ -14,19 +14,11 @@ exports[` ATM snapshot test 1`] = ` > Service - - + ATM snapshot test 1`] = ` > Span Kind - - + ATM snapshot test 1`] = ` className="timeframe-selector" span={8} > - - + @@ -443,7 +414,6 @@ exports[` ATM snapshot test 1`] = ` "opsLatencies": null, } } - loading={null} lookback={3600000} serviceName="s1" /> @@ -486,19 +456,11 @@ exports[` ATM snapshot test with no metrics 1`] = ` > Service - ATM snapshot test with no metrics 1`] = ` > Span Kind - - + ATM snapshot test with no metrics 1`] = ` className="timeframe-selector" span={8} > - - + @@ -821,7 +762,6 @@ exports[` ATM snapshot test with no metrics 1`] = ` "opsLatencies": null, } } - loading={null} lookback={3600000} serviceName="s1" /> @@ -864,19 +804,11 @@ exports[` render one service latency 1`] = ` > Service - render one service latency 1`] = ` > Span Kind - - + render one service latency 1`] = ` className="timeframe-selector" span={8} > - - + @@ -1200,7 +1111,6 @@ exports[` render one service latency 1`] = ` "opsLatencies": null, } } - loading={null} lookback={3600000} serviceName="s1" /> diff --git a/packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js b/packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js index 48c12ad90b..2074988d93 100644 --- a/packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js +++ b/packages/jaeger-ui/src/components/Monitor/ServicesView/index.test.js @@ -207,9 +207,7 @@ describe('', () => { it('should update state after choosing a new timeframe', () => { const firstGraphXDomain = wrapper.state().graphXDomain; - wrapper.setProps({ - selectedTimeFrame: 3600000 * 2, - }); + wrapper.instance().handleTimeFrameChange(3600000 * 2); expect(wrapper.state().graphXDomain).not.toBe(firstGraphXDomain); }); @@ -308,13 +306,13 @@ describe('', () => { wrapper.find('Search').simulate('change', { target: { value: newValue } }); expect(trackSearchOperationSpy).toHaveBeenCalledWith(newValue); - wrapper.find('Field').first().simulate('change', null, newValue); + wrapper.find('SearchableSelect').first().prop('onChange')(newValue); expect(trackSelectServiceSpy).toHaveBeenCalledWith(newValue); - wrapper.find({ name: 'spanKind' }).simulate('change', null, spanKindOption.value); + wrapper.find('.span-kind-selector').prop('onChange')(spanKindOption.value); expect(trackSelectSpanKindSpy).toHaveBeenCalledWith(spanKindOption.label); - wrapper.find('Field').last().simulate('change', null, timeFrameOption.value); + wrapper.find('SearchableSelect').last().prop('onChange')(timeFrameOption.value); expect(trackSelectTimeframeSpy).toHaveBeenCalledWith(timeFrameOption.label); wrapper.find({ children: 'View all traces' }).simulate('click'); @@ -367,9 +365,6 @@ describe('mapStateToProps()', () => { expect(mapStateToProps(state)).toEqual({ metrics: originInitialState, services: [], - selectedService: 's1', - selectedSpanKind: 'server', - selectedTimeFrame: 3600000, }); }); }); diff --git a/packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx b/packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx index d901e53452..0eb888ad06 100644 --- a/packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx +++ b/packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx @@ -18,13 +18,11 @@ import { ActionFunction, Action } from 'redux-actions'; import _debounce from 'lodash/debounce'; import _isEqual from 'lodash/isEqual'; import _isEmpty from 'lodash/isEmpty'; -import { Field, formValueSelector, InjectedFormProps, reduxForm } from 'redux-form'; // @ts-ignore import store from 'store'; import { connect } from 'react-redux'; import { bindActionCreators, Dispatch } from 'redux'; import { Link } from 'react-router-dom'; -import reduxFormFieldAdapter from '../../../utils/redux-form-field-adapter'; import * as jaegerApiActions from '../../../actions/jaeger-api'; import ServiceGraph from './serviceGraph'; import OperationTableDetails from './operationDetailsTable'; @@ -59,15 +57,15 @@ type StateType = { serviceOpsMetrics: ServiceOpsMetrics[] | undefined; searchOps: string; graphXDomain: number[]; + selectedService: string; + selectedSpanKind: spanKinds; + selectedTimeFrame: number; }; type TReduxProps = { services: string[]; servicesLoading: boolean; metrics: MetricsReduxState; - selectedService: string; - selectedSpanKind: spanKinds; - selectedTimeFrame: number; }; type TProps = TReduxProps & TDispatchProps; @@ -83,7 +81,6 @@ const trackSearchOperationDebounced = _debounce(searchQuery => trackSearchOperat const Search = Input.Search; const Option = Select.Option; -const serviceFormSelector = formValueSelector('serviceForm'); const oneHourInMilliSeconds = 3600000; const oneMinuteInMilliSeconds = 60000; export const timeFrameOptions = [ @@ -143,22 +140,23 @@ const convertServiceErrorRateToPercentages = (serviceErrorRate: null | ServiceMe return { ...serviceErrorRate, metricPoints: convertedMetricsPoints }; }; -type TPropsWithInjectedFormProps = TProps & InjectedFormProps; - // export for tests -export class MonitorATMServicesViewImpl extends React.PureComponent { +export class MonitorATMServicesViewImpl extends React.PureComponent { docsLink: string; graphDivWrapper: React.RefObject; serviceSelectorValue = ''; endTime: number = Date.now(); - state = { + state: StateType = { graphWidth: 300, serviceOpsMetrics: undefined, searchOps: '', graphXDomain: [], + selectedService: store.get('lastAtmSearchService') || '', + selectedSpanKind: store.get('lastAtmSearchSpanKind') || 'server', + selectedTimeFrame: store.get('lastAtmSearchTimeframe') || oneHourInMilliSeconds, }; - constructor(props: TPropsWithInjectedFormProps) { + constructor(props: TProps) { super(props); this.graphDivWrapper = React.createRef(); this.docsLink = getConfigValue('monitor.docsLink'); @@ -176,21 +174,11 @@ export class MonitorATMServicesViewImpl extends React.PureComponent ({ + graphXDomain: [currentTime - prevState.selectedTimeFrame, currentTime], + })); } updateDimensions() { @@ -212,15 +200,33 @@ export class MonitorATMServicesViewImpl extends React.PureComponent { + this.setState({ selectedService: value }, () => { + trackSelectService(value); + this.fetchMetrics(); + }); + }; + + handleSpanKindChange = (value: string) => { + this.setState({ selectedSpanKind: value as spanKinds }, () => { + const { label } = spanKindOptions.find(option => option.value === value)!; + trackSelectSpanKind(label); + this.fetchMetrics(); + }); + }; + + handleTimeFrameChange = (value: number) => { + this.setState({ selectedTimeFrame: value }, () => { + const { label } = timeFrameOptions.find(option => option.value === value)!; + trackSelectTimeframe(label); + this.fetchMetrics(); + this.calcGraphXDomain(); + }); + }; + fetchMetrics() { - const { - selectedService, - selectedSpanKind, - selectedTimeFrame, - fetchAllServiceMetrics, - fetchAggregatedServiceMetrics, - services, - } = this.props; + const { fetchAllServiceMetrics, fetchAggregatedServiceMetrics, services } = this.props; + const { selectedService, selectedSpanKind, selectedTimeFrame } = this.state; const currentService = selectedService || services[0]; if (currentService) { @@ -246,12 +252,14 @@ export class MonitorATMServicesViewImpl extends React.PureComponent

Service

- trackSelectService(newValue)} - name="service" - component={reduxFormFieldAdapter({ AntInputComponent: SearchableSelect })} + {services.map((service: string) => ( ))} - +

Span Kind

- { - const { label } = spanKindOptions.find(option => option.value === value)!; - trackSelectSpanKind(label); - }} - props={{ - className: 'span-kind-selector', - defaultValue: 'server', - value: selectedSpanKind, - disabled: metrics.operationMetricsLoading, - loading: metrics.operationMetricsLoading, - }} + className="span-kind-selector" + disabled={metrics.operationMetricsLoading} + loading={metrics.operationMetricsLoading} > {spanKindOptions.map(option => ( ))} - + @@ -350,28 +346,20 @@ export class MonitorATMServicesViewImpl extends React.PureComponent - { - const { label } = timeFrameOptions.find(option => option.value === value)!; - trackSelectTimeframe(label); - }} - props={{ - className: 'select-a-timeframe-input', - defaultValue: timeFrameOptions[3], - value: selectedTimeFrame, - disabled: metrics.operationMetricsLoading, - loading: metrics.operationMetricsLoading, - }} + className="select-a-timeframe-input" + disabled={metrics.operationMetricsLoading} + loading={metrics.operationMetricsLoading} > {timeFrameOptions.map(option => ( ))} - + @@ -386,13 +374,13 @@ export class MonitorATMServicesViewImpl extends React.PureComponent yAxisTickFormat(timeInMs, displayTimeUnit)} - xDomain={this.state.graphXDomain} + xDomain={graphXDomain} /> @@ -401,12 +389,12 @@ export class MonitorATMServicesViewImpl extends React.PureComponent @@ -415,12 +403,12 @@ export class MonitorATMServicesViewImpl extends React.PureComponent @@ -433,7 +421,7 @@ export class MonitorATMServicesViewImpl extends React.PureComponent) => { const filteredData = metrics.serviceOpsMetrics!.filter(({ name }: { name: string }) => { @@ -476,11 +464,6 @@ export function mapStateToProps(state: ReduxState): TReduxProps { services: services.services || [], servicesLoading: services.loading, metrics, - selectedService: serviceFormSelector(state, 'service') || store.get('lastAtmSearchService'), - selectedSpanKind: - serviceFormSelector(state, 'spanKind') || store.get('lastAtmSearchSpanKind') || 'server', - selectedTimeFrame: - serviceFormSelector(state, 'timeframe') || store.get('lastAtmSearchTimeframe') || oneHourInMilliSeconds, }; } @@ -497,13 +480,4 @@ export function mapDispatchToProps(dispatch: Dispatch): TDispatchPro }; } -export default withRouteProps( - connect( - mapStateToProps, - mapDispatchToProps - )( - reduxForm({ - form: 'serviceForm', - })(MonitorATMServicesViewImpl) - ) -); +export default withRouteProps(connect(mapStateToProps, mapDispatchToProps)(MonitorATMServicesViewImpl)); diff --git a/packages/jaeger-ui/src/components/Monitor/ServicesView/operationDetailsTable/index.tsx b/packages/jaeger-ui/src/components/Monitor/ServicesView/operationDetailsTable/index.tsx index a274fc981d..333ddecb0e 100644 --- a/packages/jaeger-ui/src/components/Monitor/ServicesView/operationDetailsTable/index.tsx +++ b/packages/jaeger-ui/src/components/Monitor/ServicesView/operationDetailsTable/index.tsx @@ -29,7 +29,7 @@ import { trackSortOperations, trackViewTraces } from './index.track'; type TProps = { data: ServiceOpsMetrics[] | undefined; error: MetricsReduxState['opsError']; - loading: boolean | null; + loading: boolean | undefined; endTime: number; lookback: number; serviceName: string; diff --git a/packages/jaeger-ui/src/reducers/metrics.mock.js b/packages/jaeger-ui/src/reducers/metrics.mock.js index 12e85e9c8f..63ae524f95 100644 --- a/packages/jaeger-ui/src/reducers/metrics.mock.js +++ b/packages/jaeger-ui/src/reducers/metrics.mock.js @@ -721,7 +721,7 @@ const originInitialState = { }, isATMActivated: null, loading: false, - operationMetricsLoading: null, + operationMetricsLoading: undefined, serviceMetrics: null, serviceOpsMetrics: undefined, }; diff --git a/packages/jaeger-ui/src/reducers/metrics.tsx b/packages/jaeger-ui/src/reducers/metrics.tsx index 83e7a7f037..7b0b6e6a8b 100644 --- a/packages/jaeger-ui/src/reducers/metrics.tsx +++ b/packages/jaeger-ui/src/reducers/metrics.tsx @@ -46,7 +46,7 @@ const initialState: MetricsReduxState = { }, isATMActivated: null, loading: false, - operationMetricsLoading: null, + operationMetricsLoading: undefined, serviceMetrics: null, serviceOpsMetrics: undefined, }; diff --git a/packages/jaeger-ui/src/types/metrics.tsx b/packages/jaeger-ui/src/types/metrics.tsx index 6553056e73..b3e37f4dd0 100644 --- a/packages/jaeger-ui/src/types/metrics.tsx +++ b/packages/jaeger-ui/src/types/metrics.tsx @@ -124,7 +124,7 @@ export type MetricsReduxState = { }; isATMActivated: null | boolean; loading: boolean; - operationMetricsLoading: null | boolean; + operationMetricsLoading: undefined | boolean; serviceMetrics: ServiceMetrics | null; serviceOpsMetrics: ServiceOpsMetrics[] | undefined; };