Skip to content

Commit

Permalink
Fix broken vulcan tests (backport #2268) (#2271)
Browse files Browse the repository at this point in the history
Co-authored-by: roy-dydx <133032749+roy-dydx@users.noreply.github.com>
  • Loading branch information
mergify[bot] and roy-dydx authored Sep 17, 2024
1 parent 9f93175 commit df12ead
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1092,15 +1092,16 @@ function expectStats(orderWasReplaced: boolean = false): void {
expect(stats.timing).toHaveBeenCalledWith(
`vulcan.${STATS_FUNCTION_NAME}.timing`,
expect.any(Number),
{ className: 'OrderPlaceHandler', fnName: 'place_order_cache_update' },
{ className: 'OrderPlaceHandler', fnName: 'place_order_cache_update', instance: '' },
);

if (orderWasReplaced) {
expect(stats.increment).toHaveBeenCalledWith('vulcan.place_order_handler.replaced_order', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.place_order_handler.replaced_order', 1, { instance: '' });
} else {
expect(stats.increment).not.toHaveBeenCalledWith(
'vulcan.place_order_handler.replaced_order',
expect.any(Number),
{ instance: '' },
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1974,7 +1974,7 @@ describe('OrderRemoveHandler', () => {
);

expect(producerSendSpy).not.toHaveBeenCalled();
expect(stats.increment).toHaveBeenCalledWith('vulcan.indexer_expired_order_not_found', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.indexer_expired_order_not_found', 1, { instance: '' });

await Promise.all([
expectOrderStatus(expectedOrderUuid, OrderStatus.OPEN),
Expand Down Expand Up @@ -2117,7 +2117,7 @@ describe('OrderRemoveHandler', () => {
expect(producerSendSpy).not.toHaveBeenCalled();
expect(
stats.increment,
).toHaveBeenCalledWith('vulcan.indexer_expired_order_is_not_expired', 1);
).toHaveBeenCalledWith('vulcan.indexer_expired_order_is_not_expired', 1, { instance: '' });

await Promise.all([
expectOrderStatus(expectedOrderUuid, OrderStatus.OPEN),
Expand Down Expand Up @@ -2282,6 +2282,6 @@ function expectTimingStat(fnName: string) {
expect(stats.timing).toHaveBeenCalledWith(
`vulcan.${STATS_FUNCTION_NAME}.timing`,
expect.any(Number),
{ className: 'OrderRemoveHandler', fnName },
{ className: 'OrderRemoveHandler', fnName, instance: '' },
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ describe('OrderUpdateHandler', () => {
expect(stats.increment).toHaveBeenCalledWith(
'vulcan.order_update_total_filled_exceeds_size',
1,
{ instance: '' },
);
expect(logger.info).toHaveBeenCalledWith(expect.objectContaining({
at: 'OrderUpdateHandler#getCappedNewTotalFilledQuantums',
Expand Down Expand Up @@ -413,6 +414,7 @@ describe('OrderUpdateHandler', () => {
expect(stats.increment).toHaveBeenCalledWith(
'vulcan.order_update_old_total_filled_exceeds_size',
1,
{ instance: '' },
);
expectTimingStats();
},
Expand Down Expand Up @@ -535,6 +537,7 @@ describe('OrderUpdateHandler', () => {
1,
{
orderFlags: String(redisTestConstants.orderUpdate.orderUpdate.orderId!.orderFlags),
instance: '',
},
);
});
Expand Down Expand Up @@ -571,6 +574,7 @@ describe('OrderUpdateHandler', () => {
1,
{
orderFlags: String(statefulOrderUpdate.orderUpdate.orderId!.orderFlags),
instance: '',
},
);
});
Expand Down Expand Up @@ -600,7 +604,7 @@ function expectTimingStat(fnName: string) {
expect(stats.timing).toHaveBeenCalledWith(
`vulcan.${STATS_FUNCTION_NAME}.timing`,
expect.any(Number),
{ className: 'OrderUpdateHandler', fnName },
{ className: 'OrderUpdateHandler', fnName, instance: '' },
);
}

Expand Down
19 changes: 13 additions & 6 deletions indexer/services/vulcan/__tests__/lib/on-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,14 @@ describe('onMessage', () => {
expect(handleUpdateMock).toHaveBeenCalledWith(update, message.headers ?? {});
expect(handleUpdateMock).toHaveBeenCalledTimes(1);

expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1, { instance: '' });
expect(stats.timing).toHaveBeenCalledWith(
'vulcan.message_time_in_queue',
expect.any(Number),
1,
{
topic: KafkaTopics.TO_VULCAN,
instance: '',
},
);
expect(stats.timing).toHaveBeenCalledWith(
Expand All @@ -83,6 +84,7 @@ describe('onMessage', () => {
{
success: 'true',
messageType,
instance: '',
},
);

Expand All @@ -104,13 +106,14 @@ describe('onMessage', () => {
expect(mockHandler).not.toHaveBeenCalled();
});

expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1, { instance: '' });
expect(stats.timing).toHaveBeenCalledWith(
'vulcan.message_time_in_queue',
expect.any(Number),
1,
{
topic: KafkaTopics.TO_VULCAN,
instance: '',
},
);
expect(logger.crit).toHaveBeenCalledWith(
Expand All @@ -131,13 +134,14 @@ describe('onMessage', () => {
expect(mockHandler).not.toHaveBeenCalled();
});

expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1, { instance: '' });
expect(stats.timing).toHaveBeenCalledWith(
'vulcan.message_time_in_queue',
expect.any(Number),
1,
{
topic: KafkaTopics.TO_VULCAN,
instance: '',
},
);
expect(stats.timing).toHaveBeenCalledWith(
Expand All @@ -147,6 +151,7 @@ describe('onMessage', () => {
{
success: 'false',
messageType: 'unknown',
instance: '',
},
);
expect(logger.crit).toHaveBeenCalledWith(
Expand All @@ -164,13 +169,14 @@ describe('onMessage', () => {
);

await expect(onMessage(message)).rejects.toEqual(unexpectedError);
expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1, { instance: '' });
expect(stats.timing).toHaveBeenCalledWith(
'vulcan.message_time_in_queue',
expect.any(Number),
1,
{
topic: KafkaTopics.TO_VULCAN,
instance: '',
},
);
expect(stats.timing).toHaveBeenCalledWith(
Expand All @@ -180,6 +186,7 @@ describe('onMessage', () => {
{
success: 'false',
messageType: 'orderPlace',
instance: '',
},
);
expect(logger.error).toHaveBeenCalledWith(
Expand All @@ -192,8 +199,8 @@ describe('onMessage', () => {
it('logs error and does not process message if message is empty', async () => {
await onMessage(undefined as any as KafkaMessage);

expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.empty_kafka_message', 1);
expect(stats.increment).toHaveBeenCalledWith('vulcan.received_kafka_message', 1, { instance: '' });
expect(stats.increment).toHaveBeenCalledWith('vulcan.empty_kafka_message', 1, { instance: '' });
expect(logger.error).toHaveBeenCalledWith(
expect.objectContaining({
message: 'Empty message',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ function expectStats(
const tags: {[name: string]: string} = {
topic,
success: success.toString(),
instance: '',
};
expect(timingSpy).toHaveBeenCalledWith(timingStat, expect.any(Number), STATS_NO_SAMPLING, tags);
expect(histogramSpy).toHaveBeenCalledWith(sizeStat, size, STATS_NO_SAMPLING, tags);
Expand Down
3 changes: 2 additions & 1 deletion indexer/services/vulcan/src/handlers/handler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { logger, ParseMessageError } from '@dydxprotocol-indexer/base';
import { getInstanceId, logger, ParseMessageError } from '@dydxprotocol-indexer/base';
import {
ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION,
} from '@dydxprotocol-indexer/kafka';
Expand Down Expand Up @@ -65,6 +65,7 @@ export abstract class Handler {
return {
className: this.constructor.name,
fnName,
instance: getInstanceId(),
};
}
}

0 comments on commit df12ead

Please sign in to comment.