Skip to content
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.

Sign up for GitHub

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

(SPM) Add span kind selector #2012

Merged
merged 13 commits into from
Dec 4, 2023
Merged

(SPM) Add span kind selector #2012

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f2552cb
Add span kind selector
albertteoh Dec 2, 2023
7b86eca
Rename single word to two words
albertteoh Dec 2, 2023
49c0401
Update snapshots and test
albertteoh Dec 2, 2023
78937f0
Merge branch 'main' into add-span-kind-selector
albertteoh Dec 2, 2023
c6bade9
Fix import formatting
albertteoh Dec 2, 2023
3cb5605
yarn prettier
albertteoh Dec 2, 2023
07e3c26
Remove redundant 'Choose' from title
Dec 2, 2023
46989de
Default to 'server'
Dec 2, 2023
ccd6cc7
Remove optionality in API query param fields
Dec 2, 2023
62a71da
Add behavioural tests for spanKind selector
Dec 3, 2023
c1ed381
Fix import formatting
Dec 3, 2023
71c43af
Fix import formatting
Dec 3, 2023
ff1ab7b
yarn prettier
Dec 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename single word to two words 
Signed-off-by: albertteoh <see.kwang.teoh@gmail.com>
  • Loading branch information
@albertteoh
albertteoh committed Dec 2, 2023
commit 7b86eca964f6d55eb89e44204651f75e1be48685
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/Monitor/ServicesView/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ limitations under the License.
font-weight: 800;
}

.spankind-selector-header {
.span-kind-selector-header {
padding-left: 10px;
font-size: 18px;
font-weight: 800;
}

.spankind-selector {
.span-kind-selector {
padding-left: 10px;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/Monitor/ServicesView/index.track.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ const SPM_CATEGORY_BASE = 'jaeger/ux/trace/spm';

export const CATEGORY_VIEW_ALL_TRACES = `${SPM_CATEGORY_BASE}/view-all-traces`;
export const CATEGORY_SELECT_SERVICE = `${SPM_CATEGORY_BASE}/select-service`;
export const CATEGORY_SELECT_SPANKIND = `${SPM_CATEGORY_BASE}/select-spankind`;
export const CATEGORY_SELECT_SPAN_KIND = `${SPM_CATEGORY_BASE}/select-span-kind`;
export const CATEGORY_SELECT_TIMEFRAME = `${SPM_CATEGORY_BASE}/select-timeframe`;
export const CATEGORY_SEARCH_OPERATION = `${SPM_CATEGORY_BASE}/search-operation`;

export const trackViewAllTraces = () => trackEvent(CATEGORY_VIEW_ALL_TRACES, 'click');
export const trackSelectService = (service: string) => trackEvent(CATEGORY_SELECT_SERVICE, service);
export const trackSelectSpankind = (spankind: string) => trackEvent(CATEGORY_SELECT_SPANKIND, spankind);
export const trackSelectSpanKind = (spanKind: string) => trackEvent(CATEGORY_SELECT_SPAN_KIND, spanKind);
export const trackSelectTimeframe = (timeframe: string) => trackEvent(CATEGORY_SELECT_TIMEFRAME, timeframe);
export const trackSearchOperation = (searchQuery: string) =>
trackEvent(CATEGORY_SEARCH_OPERATION, searchQuery);
38 changes: 19 additions & 19 deletions packages/jaeger-ui/src/components/Monitor/ServicesView/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import './index.css';
import { getConfigValue } from '../../../utils/config/get-config';
import {
trackSearchOperation,
trackSelectService, trackSelectSpankind,
trackSelectService, trackSelectSpanKind,
trackSelectTimeframe,
trackViewAllTraces,
} from './index.track';
Expand All @@ -64,7 +64,7 @@ type TReduxProps = {
servicesLoading: boolean;
metrics: MetricsReduxState;
selectedService: string;
selectedSpankind: spanKinds;
selectedSpanKind: spanKinds;
selectedTimeFrame: number;
};

Expand Down Expand Up @@ -95,7 +95,7 @@ export const timeFrameOptions = [
{ label: 'Last 24 hours', value: 24 * oneHourInMilliSeconds },
{ label: 'Last 2 days', value: 48 * oneHourInMilliSeconds },
];
export const spankindOptions = [
export const spanKindOptions = [
{ label: 'Server', value: 'server' },
{ label: 'Client', value: 'client' },
]
Expand Down Expand Up @@ -171,9 +171,9 @@ export class MonitorATMServicesViewImpl extends React.PureComponent<TPropsWithIn
}

componentDidUpdate(prevProps: TProps) {
const { selectedService, selectedSpankind, selectedTimeFrame, services } = this.props;
const { selectedService, selectedSpanKind, selectedTimeFrame, services } = this.props;

if (prevProps.selectedService !== selectedService || prevProps.selectedSpankind !== selectedSpankind || prevProps.selectedTimeFrame !== selectedTimeFrame) {
if (prevProps.selectedService !== selectedService || prevProps.selectedSpanKind !== selectedSpanKind || prevProps.selectedTimeFrame !== selectedTimeFrame) {
this.fetchMetrics();
} else if (!_isEqual(prevProps.services, services)) {
this.fetchMetrics();
Expand Down Expand Up @@ -207,7 +207,7 @@ export class MonitorATMServicesViewImpl extends React.PureComponent<TPropsWithIn
fetchMetrics() {
const {
selectedService,
selectedSpankind,
selectedSpanKind,
selectedTimeFrame,
fetchAllServiceMetrics,
fetchAggregatedServiceMetrics,
Expand All @@ -217,7 +217,7 @@ export class MonitorATMServicesViewImpl extends React.PureComponent<TPropsWithIn

if (currentService) {
this.endTime = Date.now();
store.set('lastAtmSearchSpankind', selectedSpankind);
store.set('lastAtmSearchSpanKind', selectedSpanKind);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we change this to SPM, a couple more improvements:

  • use constants instead of raw strings
  • keep abbreviations in uppercase, SPM not Spm (but constants should probably all be named in uppercase)

store.set('lastAtmSearchTimeframe', selectedTimeFrame);
store.set('lastAtmSearchService', this.getSelectedService());

Expand All @@ -227,7 +227,7 @@ export class MonitorATMServicesViewImpl extends React.PureComponent<TPropsWithIn
lookback: selectedTimeFrame,
step: 60 * 1000,
ratePer: 10 * 60 * 1000,
spanKind: selectedSpankind,
spanKind: selectedSpanKind,
};

fetchAllServiceMetrics(currentService, metricQueryPayload);
Expand All @@ -243,7 +243,7 @@ export class MonitorATMServicesViewImpl extends React.PureComponent<TPropsWithIn
}

render() {
const { services, metrics, selectedSpankind, selectedTimeFrame, servicesLoading } = this.props;
const { services, metrics, selectedSpanKind, selectedTimeFrame, servicesLoading } = this.props;
const serviceLatencies = metrics.serviceMetrics ? metrics.serviceMetrics.service_latencies : null;
const displayTimeUnit = calcDisplayTimeUnit(serviceLatencies);
const serviceErrorRate = metrics.serviceMetrics ? metrics.serviceMetrics.service_error_rate : null;
Expand Down Expand Up @@ -297,24 +297,24 @@ export class MonitorATMServicesViewImpl extends React.PureComponent<TPropsWithIn
</Field>
</Col>
<Col span={6}>
<h2 className="spankind-selector-header">Choose kind</h2>
<h2 className="span-kind-selector-header">Choose kind</h2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<h2 className="span-kind-selector-header">Choose kind</h2>
<h2 className="span-kind-selector-header">Span Kind</h2>

Similarly, s/Choose service/Service/ in L287, using the word "choose" in the UX is redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed "Choose" in: 07e3c26

<Field
name="spankind"
name="spanKind"
component={reduxFormFieldAdapter({ AntInputComponent: SearchableSelect })}
placeholder="Select A Span Kind"
onChange={(e, value: string) => {
const { label } = spankindOptions.find(option => option.value === value)!;
trackSelectSpankind(label);
const { label } = spanKindOptions.find(option => option.value === value)!;
trackSelectSpanKind(label);
}}
props={{
className: 'spankind-selector',
defaultValue: spankindOptions[0],
value: selectedSpankind,
className: 'span-kind-selector',
defaultValue: spanKindOptions[0],
value: selectedSpanKind,
disabled: metrics.operationMetricsLoading,
loading: metrics.operationMetricsLoading,
}}
>
{spankindOptions.map(option => (
{spanKindOptions.map(option => (
<Option key={option.value} value={option.value}>
{option.label}
</Option>
Expand Down Expand Up @@ -469,8 +469,8 @@ export function mapStateToProps(state: ReduxState): TReduxProps {
servicesLoading: services.loading,
metrics,
selectedService: serviceFormSelector(state, 'service') || store.get('lastAtmSearchService'),
selectedSpankind:
serviceFormSelector(state, 'spankind') || store.get('lastAtmSearchSpankind') || 'server',
selectedSpanKind:
serviceFormSelector(state, 'spanKind') || store.get('lastAtmSearchSpanKind') || 'server',
selectedTimeFrame:
serviceFormSelector(state, 'timeframe') || store.get('lastAtmSearchTimeframe') || oneHourInMilliSeconds,
};
Expand Down