Skip to content

Commit

Permalink
chore: add metrics for conflict creation detection (#6022)
Browse files Browse the repository at this point in the history
This PR adds a 'change-request-conflict-created' event whenever someone
save a strategy update for a strategy that's used in either pending or
scheduled change requests.

Data for pending change requests will only be sent if change requests
are enabled. Data for scheduled change requests will be sent regardless.

Getting this data is somewhat involved, so I've extracted as much of the
logic into a separate file as possible.

The event re-uses the existing `change_request` metric and sends the
following data for each change request that we discover conflicts on:

```ts 
{
  state: ChangeRequestState,
  changeRequest: string, // <unleash identifier>#<change request id>
  action: 'edit-strategy',
  eventType: 'conflict-created'
}
```

There's only one action for this for now, but we could expand this event
to things such as strategy deletion, feature archival, in the future.
That said, I'd be happy to take it out.

## Discussion points

### Has the strategy actually been updated?

This does not check whether a strategy has actually changed before
emitting the event, only that you save your strategy changes.

This assumes that most people will simply close the modal by
clicking/tapping outside it or using the escape key instead of pressing
save.

However, it will likely lead to some false positives. If we think that
is an issue, I would suggest adding a check that something in the
strategy has actually changed in a follow-up PR.
  • Loading branch information
thomasheartman committed Jan 31, 2024
1 parent d77e539 commit 5417662
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 5 deletions.
Expand Up @@ -28,6 +28,11 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequests/usePendingChangeRequests';
import { usePlausibleTracker } from 'hooks/usePlausibleTracker';
import { useScheduledChangeRequestsWithStrategy } from 'hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy';
import {
getChangeRequestConflictCreatedData,
getChangeRequestConflictCreatedDataFromScheduleData,
} from './change-request-conflict-data';

const useTitleTracking = () => {
const [previousTitle, setPreviousTitle] = useState<string>('');
Expand Down Expand Up @@ -92,7 +97,7 @@ export const FeatureStrategyEdit = () => {
const navigate = useNavigate();
const { addChange } = useChangeRequestApi();
const { isChangeRequestConfigured } = useChangeRequestsEnabled(projectId);
const { refetch: refetchChangeRequests } =
const { refetch: refetchChangeRequests, data: pendingChangeRequests } =
usePendingChangeRequests(projectId);
const { setPreviousTitle } = useTitleTracking();

Expand Down Expand Up @@ -123,6 +128,37 @@ export const FeatureStrategyEdit = () => {
}
}, [feature]);

const { trackEvent } = usePlausibleTracker();
const { changeRequests: scheduledChangeRequestThatUseStrategy } =
useScheduledChangeRequestsWithStrategy(projectId, strategyId);

const pendingCrsUsingThisStrategy = getChangeRequestConflictCreatedData(
pendingChangeRequests,
featureId,
strategyId,
uiConfig,
);

const scheduledCrsUsingThisStrategy =
getChangeRequestConflictCreatedDataFromScheduleData(
scheduledChangeRequestThatUseStrategy,
uiConfig,
);

const emitConflictsCreatedEvents = (): void =>
[
...pendingCrsUsingThisStrategy,
...scheduledCrsUsingThisStrategy,
].forEach((data) =>
trackEvent('change_request', {
props: {
...data,
action: 'edit-strategy',
eventType: 'conflict-created',
},
}),
);

const {
segments: savedStrategySegments,
refetchSegments: refetchSavedStrategySegments,
Expand Down Expand Up @@ -182,6 +218,7 @@ export const FeatureStrategyEdit = () => {
} else {
await onStrategyEdit(payload);
}
emitConflictsCreatedEvents();
refetchFeature();
navigate(formatFeaturePath(projectId, featureId));
} catch (error: unknown) {
Expand Down
@@ -0,0 +1,113 @@
import { IUiConfig } from 'interfaces/uiConfig';
import {
getChangeRequestConflictCreatedData,
getChangeRequestConflictCreatedDataFromScheduleData,
} from './change-request-conflict-data';

const uiConfig: Pick<IUiConfig, 'baseUriPath' | 'versionInfo'> = {
baseUriPath: '/some-base-uri',
};
const unleashIdentifier = uiConfig.baseUriPath;
const featureId = 'flag-with-deleted-scheduler';
const strategyId = 'ed2ffa14-004c-4ed1-931b-78761681c54a';

const changeRequestWithStrategy = {
id: 105,
features: [
{
name: featureId,
changes: [
{
action: 'updateStrategy' as const,
payload: {
id: strategyId,
},
},
],
},
],
state: 'In review' as const,
};

const changeRequestWithoutStrategy = {
id: 106,
features: [
{
name: featureId,
changes: [
{
action: 'deleteStrategy' as const,
payload: {
id: strategyId,
},
},
],
},
{
name: featureId,
changes: [
{
action: 'addStrategy' as const,
payload: {},
},
],
},
],
state: 'In review' as const,
};

test('it finds crs that update a strategy', () => {
const results = getChangeRequestConflictCreatedData(
[changeRequestWithStrategy],
featureId,
strategyId,
uiConfig,
);

expect(results).toStrictEqual([
{
state: changeRequestWithStrategy.state,
changeRequest: `${unleashIdentifier}#${changeRequestWithStrategy.id}`,
},
]);
});

test('it does not return crs that do not update a strategy', () => {
const results = getChangeRequestConflictCreatedData(
[changeRequestWithoutStrategy],
featureId,
strategyId,
uiConfig,
);

expect(results).toStrictEqual([]);
});

test('it maps scheduled change request data', () => {
const scheduledChanges = [
{
id: 103,
environment: 'development',
},
{
id: 104,
environment: 'development',
},
];

const results = getChangeRequestConflictCreatedDataFromScheduleData(
scheduledChanges,
uiConfig,
);

expect(results).toStrictEqual([
{
state: 'Scheduled',
changeRequest: `${unleashIdentifier}#103`,
},
{
state: 'Scheduled',
changeRequest: `${unleashIdentifier}#104`,
},
]);
});
@@ -0,0 +1,55 @@
import {
ChangeRequestState,
ChangeRequestType,
IChangeRequestFeature,
IFeatureChange,
} from 'component/changeRequest/changeRequest.types';
import { ScheduledChangeRequestViewModel } from 'hooks/api/getters/useScheduledChangeRequestsWithStrategy/useScheduledChangeRequestsWithStrategy';
import { IUiConfig } from 'interfaces/uiConfig';
import { getUniqueChangeRequestId } from 'utils/unique-change-request-id';

type ChangeRequestConflictCreatedData = {
changeRequest: string;
state: ChangeRequestState;
};

export const getChangeRequestConflictCreatedData = (
changeRequests:
| {
state: ChangeRequestType['state'];
id: ChangeRequestType['id'];
features: {
name: IChangeRequestFeature['name'];
changes: (Pick<IFeatureChange, 'action'> & {
payload: { id?: number | string };
})[];
}[];
}[]
| undefined,
featureId: string,
strategyId: string,
uiConfig: Pick<IUiConfig, 'baseUriPath' | 'versionInfo'>,
): ChangeRequestConflictCreatedData[] =>
changeRequests
?.filter((cr) =>
cr.features
.find((feature) => feature.name === featureId)
?.changes.some(
(change) =>
change.action === 'updateStrategy' &&
change.payload.id === strategyId,
),
)
.map((cr) => ({
changeRequest: getUniqueChangeRequestId(uiConfig, cr.id),
state: cr.state,
})) ?? [];

export const getChangeRequestConflictCreatedDataFromScheduleData = (
changeRequests: Pick<ScheduledChangeRequestViewModel, 'id'>[] | undefined,
uiConfig: Pick<IUiConfig, 'baseUriPath' | 'versionInfo'>,
): ChangeRequestConflictCreatedData[] =>
changeRequests?.map((cr) => ({
changeRequest: getUniqueChangeRequestId(uiConfig, cr.id),
state: 'Scheduled' as const,
})) ?? [];
7 changes: 3 additions & 4 deletions frontend/src/utils/unique-change-request-id.ts
Expand Up @@ -3,8 +3,7 @@ import { IUiConfig } from 'interfaces/uiConfig';
export const getUniqueChangeRequestId = (
uiConfig: Pick<IUiConfig, 'baseUriPath' | 'versionInfo'>,
changeRequestId: number,
) => {
return `${
) =>
`${
uiConfig.baseUriPath || uiConfig.versionInfo?.instanceId
}/change-requests/${changeRequestId}?version=${uiConfig.versionInfo}`;
};
}#${changeRequestId}`;

0 comments on commit 5417662

Please sign in to comment.