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

Add metrics command / output to debug bundle #9034

Merged
merged 5 commits into from
Oct 6, 2020
Merged

Conversation

davemay99
Copy link
Contributor

This PR adds a metrics.json file to each interval captured by the nomad operator debug command. A new CLI sub-command nomad operator metrics and associated plumbing was added to support this feature, and also allow easier access for operators to gather metrics requested by support. Data is retrieved using the existing /v1/metrics API endpoint.

Future improvements will optionally collect metrics from each server/client node specified.

@davemay99
Copy link
Contributor Author

I wasn't sure about how to handle updating the vendor directory. What is the preferred method to bring that up to date with my changes?

@drewbailey
Copy link
Contributor

@davemay99 running make sync will take care of the vendor changes

api/operator.go Outdated Show resolved Hide resolved
command/metrics.go Outdated Show resolved Hide resolved
Copy link
Contributor

@drewbailey drewbailey left a comment

Choose a reason for hiding this comment

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

Nice work!

I think I only have one comment, for the api operator Metrics method it would be nice to return something better than a string. For the command use case and operator debug where we are just writing it to a file this is fine, but for other API package users we probably want something easier to work with.

The issue here is that the actual endpoint returns either a MetricsSummary, or a prometheus format depending on the format query param. We could have two api/operator Metrics methods which set the query format for the user, or we could return []byte and let the callers parse the json themselves.

@vercel
Copy link

vercel bot commented Oct 6, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hashicorp/nomad/m8qrpa0an
✅ Preview: https://nomad-git-dmay-debug-metrics.hashicorp.vercel.app

command/metrics_test.go Outdated Show resolved Hide resolved
@davemay99
Copy link
Contributor Author

The issue here is that the actual endpoint returns either a MetricsSummary, or a prometheus format depending on the format query param. We could have two api/operator Metrics methods which set the query format for the user, or we could return []byte and let the callers parse the json themselves.

I originally had the []byte conversion to string in the operator debug code, but thought it would save a step by doing the conversion at the source. I switched back to []byte to leave it open for future expansion.

Copy link
Contributor

@drewbailey drewbailey left a comment

Choose a reason for hiding this comment

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

LGTM!

@davemay99 davemay99 merged commit abfcb10 into master Oct 6, 2020
@davemay99 davemay99 deleted the dmay-debug-metrics branch October 6, 2020 15:47
@drewbailey
Copy link
Contributor

@davemay99 forgot, will need a follow up pr or issue to track adding the CLI docs to the io site

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants