Add JSON-style reports #18047

twalluxio · 2023-08-22T07:19:43Z

Use jackson JSON as a standard format for reports.

Output example for bin/alluxio info report summary:

{
    "mSafeMode":false,
    "mZookeeper":false,
    "mRaftJournal":true,
    "version":"304-SNAPSHOT",
    "uptime":"0 day(s), 0 hour(s), 3 minute(s), and 7 second(s)",
    "rpcPort":19998,
    "webPort":19999,
    "masterAddress":"Ec2Cluster-masters-0:19998",
    "masterVersions":[
        {
            "version":"304-SNAPSHOT",
            "state":"PRIMARY",
            "host":"Ec2Cluster-masters-0",
            "port":19998
        }
    ],
    "started":"08-24-2023 06:43:32:856",
    "zookeeperAddress":[

    ],
    "raftJournalAddress":[
        "Ec2Cluster-masters-0:19200"
    ],
    "liveWorkers":2,
    "lostWorkers":0,
    "freeCapacity":"2048.00MB",
    "totalCapacityOnTiers":{
        "MEM":"2048.00MB"
    },
    "usedCapacityOnTiers":{
        "MEM":"0B"
    }
}

Xenorith · 2023-08-22T08:08:12Z

dora/shell/src/main/java/alluxio/cli/fsadmin/report/UfsCommand.java

    for (Map.Entry<String, MountPointInfo> entry : mountTable.entrySet()) {
+      ObjectNode mountInfo = mapper.createObjectNode();


try to represent information as classes to json serialize instead of nodes. although you have to set up the setters/getters (maybe can find some way to help generate the code), a class is more friendly to be used by different formats.

public class MountInfoOutput { public String mUri; public String mMountPoint; public String mUfsType; public String mTotalCapacity; public String mUsedCapacity; public boolean mReadOnly; public Shared mShared; // add setter and getter methods for json to work on this object }

for the two capacity values, it would be even better to use a long type for the field and use an annotation to declare a custom serialization function that will execute the FormatUtils.getSizeFromBytes

actually i noticed that the original MountPointInfo object is already mostly ready to be directly used to json serialize. the only field we don't want to show is the Properties map. so there should be a way to add an annotation to tell the json serializer to avoid serializing the field. this is the best situation because it reuses the existing code as much as possible.

one way to serialize the annotated field only:

add annotations to all the fields you need:

@JsonProperty("readOnly") public boolean mReadOnly;

set ObjectMapper to ignore the getters/setters and only focus on the annotation:

ObjectMapper mapper = new ObjectMapper(); mapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE); mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); String result = mapper.writeValueAsString(obj)

I'm not very sure about this change. Does this mean if I'd like my command to print something more, instead of a println, now I need to update the object? I mean trying to objectify printouts seems awkward unless you have a very good reason?

Objectifying the printed output ensures consistency. Printlns are at the whims of the particular dev; the lack of consistency in general is why both the command line format (ex. Flags, arguments, command tree structure) and the output format are a total mess. Objectifying outputs also allows for various output format (ex. Json for machine parseable output, yaml for more human readable output)

Xenorith

looks much cleaner now for the ufs command

dora/core/common/src/main/java/alluxio/wire/MountPointInfo.java

Kai-Zhang · 2023-08-24T09:27:05Z

some of the keys are inconsistent and ambiguous, such as "mZookeeper" and "mRaftJournal". it'll be better if the names are "useZookeeper" and "useRaftJournal"
since the golang code will reformat the result, i think maybe it'll be better to output the capacity in bytes and leave the "prettification" to the golang code
if the CapacityCommand is not used anymore, we can revert the changes or even delete it completely.

jiacheliu3

Thanks for the work. I left some comments. PTAL thanks!

dora/shell/src/main/java/alluxio/cli/fsadmin/report/CapacityCommand.java

jiacheliu3 · 2023-08-27T03:48:43Z

dora/shell/src/main/java/alluxio/cli/fsadmin/report/MetricsOutput.java

+     * @param name name of the metrics to check
+     * @return true if a metric is an Alluxio metric, false otherwise
+     */
+    private boolean isAlluxioMetric(String name) {


is this copied from some utility class?

It comes from the original MetricsOutput class, still used in JSON format

dora/shell/src/main/java/alluxio/cli/fsadmin/report/SummaryCommand.java

jiacheliu3 · 2023-08-27T03:50:15Z

dora/shell/src/main/java/alluxio/cli/fsadmin/report/SummaryOutput.java

+    public SummaryOutput(MasterInfo masterInfo, BlockMasterInfo blockMasterInfo, String dateFormatPattern) {
+        // give values to internal properties
+        mMasterAddress = masterInfo.getLeaderMasterAddress();
+        mWebPort = masterInfo.getWebPort();


the web port and raft journal port are all here, are they displayed too?

As to the content to display, I followed the original format

dora/shell/src/main/java/alluxio/cli/fsadmin/report/SummaryOutput.java

dora/shell/src/main/java/alluxio/cli/fsadmin/report/UfsCommand.java

Xenorith · 2023-09-04T01:19:45Z

overall looks good, but please fix the checkstyle and unit test problems in the build. for example, the unit test failure is:

2023-09-04T00:49:21.4403211Z 00:49:21.439 [ERROR] alluxio.wire.MountPointInfoTest.json  Time elapsed: 0.415 s  <<< ERROR!
2023-09-04T00:49:21.4403868Z com.fasterxml.jackson.databind.exc.InvalidFormatException: 
2023-09-04T00:49:21.4406722Z Cannot deserialize value of type `long` from String "-4863669631326369771B": not a valid `long` value
2023-09-04T00:49:21.4410414Z  at [Source: (byte[])"{"ufsUri":"u","ufsType":"O","ufsCapacityBytes":"-4863669631326369771B","ufsUsedBytes":"4358.55PB","readOnly":false,"mountId":149227371441089240,"shared":false,"properties":{"v0k":"GyRv","P":"JdO","sroV":"","BLO":"oqRQ","iN":"","ql2i":"","XfEZ":"iF0","iNks":"oW","30uq":"gUgr","FWo":"hy"}}"; line: 1, column: 48] (through reference chain: alluxio.wire.MountPointInfo["ufsCapacityBytes"])

alluxio-bot · 2023-09-05T04:13:21Z

Automated checks report:

PR title follows the conventions: FAIL
- The title of the PR does not pass all the checks. Please fix the following issues:
  - First word of title ("Add") is not an imperative verb. Please use one of the valid words
Commits associated with Github account: PASS

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

alluxio-bot · 2023-09-05T05:57:52Z

Automated checks report:

PR title follows the conventions: PASS
Commits associated with Github account: PASS

All checks passed!

Xenorith

thanks for the improvement. yay for deleting more lines!

@jiacheliu3 @Kai-Zhang do you want to review before merging?

Kai-Zhang

LGTM

jiacheliu3

Mostly LGTM with some comments, thanks for the work!

dora/shell/pom.xml

dora/shell/src/main/java/alluxio/cli/fsadmin/report/JobServiceMetricsCommand.java

dora/shell/src/main/java/alluxio/cli/fsadmin/report/MetricsOutput.java

jiacheliu3 · 2023-09-12T05:59:54Z

dora/shell/src/main/java/alluxio/cli/fsadmin/report/MetricsOutput.java

+    mMetricsInfo = new ArrayList<>();
+    for (Map.Entry<String, MetricValue> entry : metrics.entrySet()) {
+      String key = entry.getKey();
+      if (!isAlluxioMetric(key)) {


what is not alluxio metric? Do you have an example?

If the metrics entry set has elements which don't start with instance types such as cluster, worker, etc. It is not an Alluxio metrics. I'm not quite sure when non-alluxio metrics will emerge, but the previous version did so

dora/shell/src/main/java/alluxio/cli/fsadmin/report/SummaryCommand.java

jiacheliu3 · 2023-09-12T06:11:16Z

dora/shell/src/main/java/alluxio/cli/fsadmin/report/SummaryCommand.java

-      print("Tier: " + usedBytesTier.getKey()
-          + "  Size: " + FormatUtils.getSizeFromBytes(usedBytesTier.getValue()));
+    ObjectMapper objectMapper = new ObjectMapper();
+    SummaryOutput summaryInfo = new SummaryOutput(masterInfo, blockMasterInfo, mDateFormatPattern);


I think your JobServiceOutput can follow this pattern, get the info by RPC and pass it to the output object purely for display.

And even if you cannot get certain info from RPC, you can still continue to display with broken information like:

BlockMasterInfo blockMasterInfo = null; try { blockMasterInfo = mBlockMasterClient.getBlockMasterInfo(blockMasterInfoFilter); } catch (Exception e) { // log the exception and actually you can try to continue, if that's necessary } SummaryOutput summaryInfo = new SummaryOutput(blockMasterInfo, ...)

And in SummaryOutput when it tries to display, if certain info is null you can simply display something like "Failed to connect to master for Block/Worker information" and skip that part.

Actually you may choose to implement this in a follow-up PR (don't have to finish all that before merging).

dora/shell/src/main/java/alluxio/cli/fsadmin/report/SummaryOutput.java

dora/shell/src/test/java/alluxio/cli/fsadmin/report/MetricsCommandTest.java

jiacheliu3

LGTM, thanks for the work!

jiacheliu3 · 2023-09-14T06:48:55Z

alluxio-bot, merge this please

twalluxio added 2 commits August 22, 2023 09:56

json-style capacity report

9d7a923

json-style reports

e566134

Xenorith reviewed Aug 22, 2023

View reviewed changes

new json-style ufs report

8490c83

Xenorith reviewed Aug 23, 2023

View reviewed changes

dora/core/common/src/main/java/alluxio/wire/MountPointInfo.java Outdated Show resolved Hide resolved

twalluxio added 4 commits August 24, 2023 11:00

fix MountPointInfo.java

e4c4141

new json-style summary report

d9cd0bd

new json-style job service report

32e094c

new json-style metrics report

fe050c4

Kai-Zhang requested a review from jiacheliu3 August 24, 2023 09:19

jiacheliu3 requested changes Aug 27, 2023

View reviewed changes

twalluxio added 6 commits August 28, 2023 10:25

revert CapacityCommand.java

b61b187

hygiene on metrics report

3950f4a

improve property readability

c64f186

improve exception catcher and indention

326ede0

remove capacity command in alluxio fsadmin shell

9d696ae

add an empty line after SummaryOutput.java

d332806

twalluxio requested a review from jiacheliu3 August 29, 2023 07:35

change logger to org.slf4j

4b69c7e

twalluxio added 4 commits September 4, 2023 15:29

pass style check & unit tests

f9f74ad

add license

2da6e29

fix unit tests

92db746

checkstyle fix

3c20ea1

re-submit unit test changes

fb6f842

Xenorith approved these changes Sep 5, 2023

View reviewed changes

Kai-Zhang approved these changes Sep 6, 2023

View reviewed changes

Xenorith force-pushed the main branch from dfa74af to 5fb603d Compare September 11, 2023 01:34

jiacheliu3 requested changes Sep 12, 2023

View reviewed changes

twalluxio added 2 commits September 14, 2023 10:42

fix on reviews

9d303ee

fix bug

f0bdeb0

twalluxio requested a review from jiacheliu3 September 14, 2023 03:37

jiacheliu3 approved these changes Sep 14, 2023

View reviewed changes

jiacheliu3 added the type-feature This issue is a feature request label Sep 14, 2023

alluxio-bot merged commit 959c527 into Alluxio:main Sep 14, 2023
12 checks passed

twalluxio deleted the shell-better-report branch September 15, 2023 07:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSON-style reports #18047

Add JSON-style reports #18047

twalluxio commented Aug 22, 2023 •

edited

Loading

Xenorith Aug 22, 2023

Kai-Zhang Aug 22, 2023 •

edited

Loading

jiacheliu3 Aug 24, 2023

Xenorith Aug 24, 2023

Xenorith left a comment

Kai-Zhang commented Aug 24, 2023

jiacheliu3 left a comment

jiacheliu3 Aug 27, 2023

twalluxio Aug 28, 2023

jiacheliu3 Aug 27, 2023

twalluxio Aug 28, 2023

Xenorith commented Sep 4, 2023

alluxio-bot commented Sep 5, 2023

alluxio-bot commented Sep 5, 2023

Xenorith left a comment

Kai-Zhang left a comment

jiacheliu3 left a comment

jiacheliu3 Sep 12, 2023

twalluxio Sep 14, 2023

jiacheliu3 Sep 14, 2023

jiacheliu3 Sep 12, 2023

jiacheliu3 left a comment

jiacheliu3 commented Sep 14, 2023

		for (Map.Entry<String, MountPointInfo> entry : mountTable.entrySet()) {
		ObjectNode mountInfo = mapper.createObjectNode();

Add JSON-style reports #18047

Add JSON-style reports #18047

Conversation

twalluxio commented Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Kai-Zhang Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xenorith left a comment

Choose a reason for hiding this comment

Kai-Zhang commented Aug 24, 2023

jiacheliu3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xenorith commented Sep 4, 2023

alluxio-bot commented Sep 5, 2023

alluxio-bot commented Sep 5, 2023

Xenorith left a comment

Choose a reason for hiding this comment

Kai-Zhang left a comment

Choose a reason for hiding this comment

jiacheliu3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiacheliu3 left a comment

Choose a reason for hiding this comment

jiacheliu3 commented Sep 14, 2023

twalluxio commented Aug 22, 2023 •

edited

Loading

Kai-Zhang Aug 22, 2023 •

edited

Loading