-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
New Intel PowerStat input plugin #8488
New Intel PowerStat input plugin #8488
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @MaciejMis, cool stuff. I've some comment nevertheless...
- I think the metric output format is costly on the query side. If you e.g. want the "cpu_frequency" you need to constrain the field value if "name" to "cpu_frequency". That means you need to touch all rows of the DB. Making it a field i.e. field: "cpu_frequency_mhz"=value would be much more efficient.
- The whole "services" thing feels like a standalone golang package. Can maybe Intel host that as a package (just the "services", "data", "mocks" thing) as a golang package that we can use here? I'm asking because that's a lot of code not solely bound with telegraf (e.g. someone could use it in a different golang project)...
- The
skipfirst
feels strange. While I couldn't come up with an ad-hoc replacement it feels like circumventing anInit()
case...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice code, but I have the feeling it has too many indirections. Please see my comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I really enjoy reading the code!
One thing worth mentioning is the atomic organization of the metrics. There are multiple metrics emitted with the same name but different fields. One could argue to collect those fields before emitting, but on the other hand the current structure keeps the code clean.
(cherry picked from commit 9166a16)
Required for all PRs:
CCLA completed and signed by Intel Corporation.
Feature Request
We have created a Telegraf PowerStat input plugin which enables monitoring of platform metrics (power, TDP) and per-CPU metrics like temperature, power and utilization.
Intel PowerStat plugin supports Intel based platforms and assumes presence of Linux based OS.
Proposal:
The plugin we have created is able to provide following metrics:
Platform metrics:
Per-CPU metrics:
Use case:
Key source of platform telemetry is power domain that is beneficial for MANO/Monitoring&Analytics systems to take preventive/corrective actions based on platform busyness, CPU temperature, actual CPU utilization and power statistics.
Main use cases are power saving and workload migration.
Closes: #8485