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

feat(intel_powerstat): Add CPU base frequency metric and add support for new platforms #12452

Merged
merged 4 commits into from
Jan 18, 2023

Conversation

p-zak
Copy link
Collaborator

@p-zak p-zak commented Jan 3, 2023

Required for all PRs

Introduced multiple changes:

  • New cpu_base_frequency_mhz metric added (if configured by adding cpu_base_frequency to package_metrics configuration option).
  • New supported platforms added:
    • Intel Lakefield
    • Intel RocketLake
    • Intel AlderLake
    • Intel RaptorLake
    • Intel MeteorLake

New metric:
cpu_base_frequency_mhz - will expose base frequency of CPU's package (in MHz)

Example of output for new metric:

powerstat_package,host=ubuntu,package_id=0 cpu_base_frequency_mhz=2400i 1669118424000000000

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jan 3, 2023
p-zak and others added 2 commits January 3, 2023 18:02
…for new platforms

Co-authored-by: PanKaker <92713340+PanKaker@users.noreply.github.com>
@Hipska Hipska added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jan 17, 2023
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Thanks for this - just two questions :)

Copy link
Contributor

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks good to me, my only comment is to make the missing of the cpu_base_frequency feature a warn instead of a debug message. What do you think?

plugins/inputs/intel_powerstat/intel_powerstat.go Outdated Show resolved Hide resolved
@telegraf-tiger
Copy link
Contributor

Copy link
Contributor

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks @p-zak!

@srebhan srebhan merged commit 65b23f1 into influxdata:master Jan 18, 2023
@PanKaker
Copy link
Contributor

PanKaker commented Jan 19, 2023

Hi, @srebhan, @powersj!

Is it possible to add me to Co-authored list of the PR/commit to master?
1a931cd

I was a person who implemented this feature, but because of lack CCLA permissions, we've decided to make this PR under p-zak's name.

It will be great to see my account under the contribution section.
Thank you

@srebhan
Copy link
Contributor

srebhan commented Jan 20, 2023

Hey @PanKaker! Sorry for me missing out on mentioning you in the commit. However as we already merged this, adding you would require to edit git history with forced-push which will in turn give all current PR contributors a big headache.

Please accept my apologies!

@srebhan srebhan added this to the v1.26.0 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants