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

KIBANA-150302 fixed displaying sampling rate of N/A if there are no t… #191275

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

KIBANA-150302 fixed displaying sampling rate of N/A if there are no t… #191275

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions x-pack/plugins/apm/server/routes/storage_explorer/get_service_statistics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ async function getMainServiceStatistics({

export type StorageExplorerServiceStatisticsResponse = Array<{
serviceName: string;
sampling: number;
sampling: number | string;
environments: string[];
size: number;
agentName: AgentName;
Expand Down Expand Up @@ -233,7 +233,7 @@ export async function getServiceStatistics({
sampledTransactionDocs / totalTransactionsPerService[serviceName],
1
)
: 0;
: 'N/A';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to return null here. "N/A" is a presentational and locale-specific. It might be slightly more code if we returned null here and made the UI display "N/A" or the appropriate string.

Let me know if I can help in any way with getting this done.

Copy link
Contributor

@smith smith Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

cc @miltonhultgren do we have anything in our API guidelines about handling 0 vs. null/undefined?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that the API should not return a (localized) string in this situation (based on the API guidelines to separate API from UI presentation), and if this is meant to express a lack of a value then I would return a null/undefined (but this part is not stated in the guidelines).

This is for the reason that 0 may be a valid value in some cases, so it's risky to make 0 === N/A, while null is usually the standard invalid value.


return {
...rest,
Expand Down