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.

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

Refactor collectMetrics command #17111

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

ssz1997
Copy link
Contributor

@ssz1997 ssz1997 commented Mar 17, 2023

  1. We added more information to the response JSON string. Make the whole thing a JSON string for easier parsing.
  2. Refactor the get metrics part for reusing.

Copy link
Contributor

@jiacheliu3 jiacheliu3 left a comment

Choose a reason for hiding this comment

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

LGTM, could you attach one sample output file/content to the PR? Thanks!

@@ -122,62 +158,46 @@ private void masterMetrics(StringWriter outputBuffer, int i) throws IOException
// Do not break the loop since the HTTP failure can be due to many reasons
// Return the error message instead
LOG.error("Failed to get Alluxio master metrics from URL {}. Exception: ", url, e);
metricsResponse = String.format("Url: %s%nError: %s", url, e.getMessage());
metricsResponse = String.format("{Url: \"%s\",%n\"Error\": %s}", url, e.getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

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

im fine with this manual wrapping for now. An alternative is to wrap it with an object like

class CollectMetricsResponse {
  String url;
  String error;
  String body;
}

Then use jackson ObjectMapper to convert to json. Up to you :)

@ssz1997
Copy link
Contributor Author

ssz1997 commented Mar 20, 2023

collectMetrics.txt
By default we take 3 snapshots of the metrics. Running Alluxio locally w/ 1 master and 1 worker and this is the file containing all records under <ALLUXIO_WORK_DIR>/.collectInfo/collectMetrics/collectMetrics.txt

@Xenorith
Copy link
Contributor

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit b7866e4 into Alluxio:master Mar 20, 2023
@Xenorith Xenorith added the area-client Alluxio client label Mar 22, 2023
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 17, 2023
1. We added more information to the response JSON string. Make the whole
thing a JSON string for easier parsing.
2. Refactor the get metrics part for reusing.

pr-link: Alluxio#17111
change-id: cid-0675889969cfe77f069f84c2eb02ae1aa55a0b0b
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 18, 2023
1. We added more information to the response JSON string. Make the whole
thing a JSON string for easier parsing.
2. Refactor the get metrics part for reusing.

pr-link: Alluxio#17111
change-id: cid-0675889969cfe77f069f84c2eb02ae1aa55a0b0b
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 19, 2023
1. We added more information to the response JSON string. Make the whole
thing a JSON string for easier parsing.
2. Refactor the get metrics part for reusing.

pr-link: Alluxio#17111
change-id: cid-0675889969cfe77f069f84c2eb02ae1aa55a0b0b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-client Alluxio client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants