Skip to content

Commit

Permalink
Add setting to turn extending numeric precision on or off (#8837)
Browse files Browse the repository at this point in the history
* Add setting to turn extending numeric precision on or off

Signed-off-by: Miki <[email protected]>

* Changeset file for PR #8837 created/updated

---------

Signed-off-by: Miki <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
AMoo-Miki and opensearch-changeset-bot[bot] authored Dec 20, 2024
1 parent f85cd6d commit 791f5d8
Show file tree
Hide file tree
Showing 16 changed files with 336 additions and 26 deletions.
5 changes: 5 additions & 0 deletions changelogs/fragments/8837.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fix:
- Fix a typo while inspecting values for large numerals in OSD and the JS client ([#8837](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8837))

feat:
- Add setting to turn extending numeric precision on or off ([#8837](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8837))
2 changes: 1 addition & 1 deletion src/plugins/console/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "opensearchDashboards",
"server": true,
"ui": true,
"requiredPlugins": ["devTools"],
"requiredPlugins": ["devTools", "data"],
"optionalPlugins": ["usageCollection", "home", "dataSource"],
"requiredBundles": [
"opensearchUiShared",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ const dummyArgs: OpenSearchRequestArgs = {
};

describe('test sendRequestToOpenSearch', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('test request success, json', () => {
const mockHttpResponse = createMockHttpResponse(
200,
Expand All @@ -47,9 +51,9 @@ describe('test sendRequestToOpenSearch', () => {
});
});

it('test request success, json with long numerals', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n;
it('test request success, json with long numerals when precision enabled', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n + 1n;
const mockHttpResponse = createMockHttpResponse(
200,
'ok',
Expand All @@ -60,14 +64,59 @@ describe('test sendRequestToOpenSearch', () => {
}
);

jest.spyOn(opensearch, 'send').mockResolvedValue(mockHttpResponse);
sendRequestToOpenSearch(dummyArgs).then((result) => {
const send = jest.spyOn(opensearch, 'send');
send.mockResolvedValue(mockHttpResponse);
sendRequestToOpenSearch({
...dummyArgs,
withLongNumeralsSupport: true,
}).then((result) => {
expect(send).toHaveBeenCalledWith(
expect.anything(),
dummyArgs.requests[0].method,
dummyArgs.requests[0].url,
dummyArgs.requests[0].data.join('\n') + '\n',
undefined,
true
);
const value = (result as any)[0].response.value;
expect(value).toMatch(new RegExp(`"long-max": ${longPositive}[,\n]`));
expect(value).toMatch(new RegExp(`"long-min": ${longNegative}[,\n]`));
});
});

it('test request success, json with long numerals when precision disabled', () => {
const longPositive = BigInt(Number.MAX_SAFE_INTEGER) * 2n + 1n;
const longNegative = BigInt(Number.MIN_SAFE_INTEGER) * 2n + 1n;
const mockHttpResponse = createMockHttpResponse(
200,
'ok',
[['Content-Type', 'application/json, utf-8']],
{
'long-max': Number(longPositive),
'long-min': Number(longNegative),
}
);

const send = jest.spyOn(opensearch, 'send');
send.mockResolvedValue(mockHttpResponse);
sendRequestToOpenSearch({
...dummyArgs,
withLongNumeralsSupport: false,
}).then((result) => {
expect(send).toHaveBeenCalledWith(
expect.anything(),
dummyArgs.requests[0].method,
dummyArgs.requests[0].url,
dummyArgs.requests[0].data.join('\n') + '\n',
undefined,
false
);
const value = (result as any)[0].response.value;
expect(value).toMatch(new RegExp(`"long-max": ${Number(longPositive)}[,\n]`));
expect(value).toMatch(new RegExp(`"long-min": ${Number(longNegative)}[,\n]`));
});
});

it('test request success, text', () => {
const mockHttpResponse = createMockHttpResponse(
200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface OpenSearchRequestArgs {
http: HttpSetup;
requests: any;
dataSourceId?: string;
withLongNumeralsSupport?: boolean;
}

export interface OpenSearchRequestObject {
Expand Down Expand Up @@ -104,7 +105,8 @@ export function sendRequestToOpenSearch(
opensearchMethod,
opensearchPath,
opensearchData,
args.dataSourceId
args.dataSourceId,
args.withLongNumeralsSupport
);
if (reqId !== CURRENT_REQ_ID) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
* under the License.
*/

import { UI_SETTINGS } from '../../../../../data/common';

jest.mock('./send_request_to_opensearch', () => ({ sendRequestToOpenSearch: jest.fn() }));
jest.mock('../../contexts/editor_context/editor_registry', () => ({
instance: { getInputEditor: jest.fn() },
Expand Down Expand Up @@ -74,9 +76,63 @@ describe('useSendCurrentRequestToOpenSearch', () => {

const { result } = renderHook(() => useSendCurrentRequestToOpenSearch(), { wrapper: contexts });
await act(() => result.current());

expect(sendRequestToOpenSearch).toHaveBeenCalledWith({
requests: ['test'],
http: mockContextValue.services.http,
});

// Second call should be the request success
const [, [requestSucceededCall]] = (dispatch as jest.Mock).mock.calls;
expect(requestSucceededCall).toEqual({ type: 'requestSuccess', payload: { data: [] } });
});

it('calls sendRequestToOpenSearch turning withLongNumeralsSupport on when long-numerals is enabled', async () => {
// Set up mocks
(mockContextValue.services.settings.toJSON as jest.Mock).mockReturnValue({});
(mockContextValue.services.uiSettings.get as jest.Mock).mockImplementation((key: string) =>
Promise.resolve(key === UI_SETTINGS.DATA_WITH_LONG_NUMERALS ? true : undefined)
);

// This request should succeed
(sendRequestToOpenSearch as jest.Mock).mockResolvedValue([]);
(editorRegistry.getInputEditor as jest.Mock).mockImplementation(() => ({
getRequestsInRange: () => ['test'],
}));

const { result } = renderHook(() => useSendCurrentRequestToOpenSearch(), { wrapper: contexts });
await act(() => result.current());
expect(sendRequestToOpenSearch).toHaveBeenCalledWith({
requests: ['test'],
http: mockContextValue.services.http,
withLongNumeralsSupport: true,
});

// Second call should be the request success
const [, [requestSucceededCall]] = (dispatch as jest.Mock).mock.calls;
expect(requestSucceededCall).toEqual({ type: 'requestSuccess', payload: { data: [] } });
});

it('calls sendRequestToOpenSearch turning withLongNumeralsSupport off when long-numerals is disabled', async () => {
// Set up mocks
(mockContextValue.services.settings.toJSON as jest.Mock).mockReturnValue({});
(mockContextValue.services.uiSettings.get as jest.Mock).mockImplementation((key: string) =>
Promise.resolve(key === UI_SETTINGS.DATA_WITH_LONG_NUMERALS ? false : undefined)
);

// This request should succeed
(sendRequestToOpenSearch as jest.Mock).mockResolvedValue([]);
(editorRegistry.getInputEditor as jest.Mock).mockImplementation(() => ({
getRequestsInRange: () => ['test'],
}));

const { result } = renderHook(() => useSendCurrentRequestToOpenSearch(), { wrapper: contexts });
await act(() => result.current());

expect(sendRequestToOpenSearch).toHaveBeenCalledWith({
requests: ['test'],
http: mockContextValue.services.http,
withLongNumeralsSupport: false,
});

// Second call should be the request success
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ import { track } from './track';

// @ts-ignore
import { retrieveAutoCompleteInfo } from '../../../lib/mappings/mappings';
import { UI_SETTINGS } from '../../../../../data/common';

export const useSendCurrentRequestToOpenSearch = (dataSourceId?: string) => {
const {
services: { history, settings, notifications, trackUiMetric, http },
services: { history, settings, notifications, trackUiMetric, http, uiSettings },
} = useServicesContext();

const dispatch = useRequestActionContext();
Expand All @@ -64,7 +65,14 @@ export const useSendCurrentRequestToOpenSearch = (dataSourceId?: string) => {
// Fire and forget
setTimeout(() => track(requests, editor, trackUiMetric), 0);

const results = await sendRequestToOpenSearch({ http, requests, dataSourceId });
const withLongNumeralsSupport = await uiSettings.get(UI_SETTINGS.DATA_WITH_LONG_NUMERALS);

const results = await sendRequestToOpenSearch({
http,
requests,
dataSourceId,
withLongNumeralsSupport,
});

results.forEach(({ request: { path, method, data } }) => {
try {
Expand Down Expand Up @@ -112,5 +120,14 @@ export const useSendCurrentRequestToOpenSearch = (dataSourceId?: string) => {
});
}
}
}, [dispatch, http, dataSourceId, settings, notifications.toasts, trackUiMetric, history]);
}, [
dispatch,
http,
dataSourceId,
settings,
notifications.toasts,
trackUiMetric,
history,
uiSettings,
]);
};
5 changes: 3 additions & 2 deletions src/plugins/console/public/lib/opensearch/opensearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ export async function send(
method: string,
path: string,
data: any,
dataSourceId?: string
dataSourceId?: string,
withLongNumeralsSupport?: boolean
): Promise<HttpResponse> {
return await http.post<HttpResponse>('/api/console/proxy', {
query: {
Expand All @@ -57,7 +58,7 @@ export async function send(
body: data,
prependBasePath: true,
asResponse: true,
withLongNumeralsSupport: true,
withLongNumeralsSupport,
});
}

Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,5 @@ export const UI_SETTINGS = {
QUERY_DATAFRAME_HYDRATION_STRATEGY: 'query:dataframe:hydrationStrategy',
SEARCH_QUERY_LANGUAGE_BLOCKLIST: 'search:queryLanguageBlocklist',
NEW_HOME_PAGE: 'home:useNewHomePage',
DATA_WITH_LONG_NUMERALS: 'data:withLongNumerals',
} as const;
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { IndexPatternsService, IndexPattern } from '.';
import { fieldFormatsMock } from '../../field_formats/mocks';
import { stubbedSavedObjectIndexPattern } from '../../../../../fixtures/stubbed_saved_object_index_pattern';
import { UiSettingsCommon, SavedObjectsClientCommon, SavedObject } from '../types';
import { UI_SETTINGS } from '../../constants';

const createFieldsFetcher = jest.fn().mockImplementation(() => ({
getFieldsForWildcard: jest.fn().mockImplementation(() => {
Expand All @@ -51,6 +52,7 @@ function setDocsourcePayload(id: string | null, providedPayload: any) {
describe('IndexPatterns', () => {
let indexPatterns: IndexPatternsService;
let savedObjectsClient: SavedObjectsClientCommon;
const uiSettingsGet = jest.fn();

beforeEach(() => {
const indexPatternObj = { id: 'id', version: 'a', attributes: { title: 'title' } };
Expand Down Expand Up @@ -83,9 +85,12 @@ describe('IndexPatterns', () => {
};
});

uiSettingsGet.mockClear();
uiSettingsGet.mockReturnValue(Promise.resolve(false));

indexPatterns = new IndexPatternsService({
uiSettings: ({
get: () => Promise.resolve(false),
get: uiSettingsGet,
getAll: () => {},
} as any) as UiSettingsCommon,
savedObjectsClient: (savedObjectsClient as unknown) as SavedObjectsClientCommon,
Expand Down Expand Up @@ -249,4 +254,13 @@ describe('IndexPatterns', () => {

expect(indexPatterns.savedObjectToSpec(savedObject)).toMatchSnapshot();
});

test('correctly detects long-numerals support', async () => {
expect(await indexPatterns.isLongNumeralsSupported()).toBe(false);

uiSettingsGet.mockImplementation((key: string) =>
Promise.resolve(key === UI_SETTINGS.DATA_WITH_LONG_NUMERALS ? true : undefined)
);
expect(await indexPatterns.isLongNumeralsSupported()).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,10 @@ export class IndexPatternsService {
indexPatternCache.clear(indexPatternId);
return this.savedObjectsClient.delete('index-pattern', indexPatternId);
}

isLongNumeralsSupported() {
return this.config.get(UI_SETTINGS.DATA_WITH_LONG_NUMERALS);
}
}

export type IndexPatternsContract = PublicMethodsOf<IndexPatternsService>;
11 changes: 11 additions & 0 deletions src/plugins/data/server/ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,17 @@ export function getUiSettings(
}),
schema: schema.string(),
},
[UI_SETTINGS.DATA_WITH_LONG_NUMERALS]: {
name: i18n.translate('data.advancedSettings.data.withLongNumeralsTitle', {
defaultMessage: 'Extend Numeric Precision',
}),
value: true,
description: i18n.translate('data.advancedSettings.data.withLongNumeralsText', {
defaultMessage:
"Turn on for precise handling of extremely large numbers. Turn off to optimize performance when high precision for large values isn't required.",
}),
schema: schema.boolean(),
},
[UI_SETTINGS.TIMEPICKER_REFRESH_INTERVAL_DEFAULTS]: {
name: i18n.translate('data.advancedSettings.timepicker.refreshIntervalDefaultsTitle', {
defaultMessage: 'Time filter refresh interval',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ async function mountDoc(update = false, indexPatternGetter: any = null) {
};
const indexPatternService = {
get: indexPatternGetter ? indexPatternGetter : jest.fn(() => Promise.resolve(indexPattern)),
isLongNumeralsSupported: jest.fn(),
} as any;

const props = {
Expand Down
Loading

0 comments on commit 791f5d8

Please sign in to comment.