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

Addition of pcp2openmetrics tool #1885

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

lmchilton
Copy link
Contributor

Add tool pcp2openmetrics to support conversion of pcp metrics to open metric format.

Updated relevant QA tests to test basic functionality of tool as well as HTTP transfer function.

Updated pcp2json QA test 1589 to use a pythonserver instead of nc to test HTTP functionality.

Copy link
Member

@natoscott natoscott left a comment

Choose a reason for hiding this comment

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

@lmchilton nice work Lauren, comments inline.

qa/1131 Show resolved Hide resolved
qa/1589 Show resolved Hide resolved
qa/1589 Outdated Show resolved Hide resolved
qa/1589.out Outdated Show resolved Hide resolved
qa/1589.out Outdated Show resolved Hide resolved
src/pcp2openmetrics/pcp2openmetrics.py Outdated Show resolved Hide resolved
src/pcp2openmetrics/pcp2openmetrics.py Outdated Show resolved Hide resolved
src/pcp2openmetrics/pcp2openmetrics.py Outdated Show resolved Hide resolved
src/pcp2openmetrics/pcp2openmetrics.py Outdated Show resolved Hide resolved
src/pcp2openmetrics/pcp2openmetrics.py Show resolved Hide resolved
Copy link
Member

@natoscott natoscott left a comment

Choose a reason for hiding this comment

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

@lmchilton there's a few QA issues lurking still:

  • the output in test 1131.out is not filtered (most labels)m
  • 1827.out contains unfiltered domainname label value,
  • not sure what "+Metric hinv.ncpu details (last fetch: 1707935533):" is? but it looks at least unfiltered, possibly just a debug diagnostic?
  • maybe update the header comment in qa/1131 also to say it tests both pcp2json and pcp2openmetrics now

pcp2openmetrics.1 has some incorrect dates in the copyright notice as well.
Otherwise looking good I think.

Copy link
Member

@natoscott natoscott left a comment

Choose a reason for hiding this comment

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

Just a few small QA things left, then this is good to go.

qa/1131 Outdated Show resolved Hide resolved
qa/1589.out Outdated Show resolved Hide resolved
qa/1589.out Outdated Show resolved Hide resolved
qa/1827 Outdated Show resolved Hide resolved
qa/1827 Show resolved Hide resolved
qa/1827.out Outdated Show resolved Hide resolved
…rmat

including three sets of revisions made from initial PR for pcp2openmetrics tool.
Made several changes to qa tests to filter output.
@natoscott natoscott merged commit e906ad3 into performancecopilot:main Mar 20, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants