-
Notifications
You must be signed in to change notification settings - Fork 708
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
Use PDH native functions as alternative to Registry Calls #1350
Comments
I'm very much in favour of moving to PDH, though I'm not currently in a position to implement and test this myself. Are you comfortable implementing this, if you have the time? |
I play with PDH function today. However I got different values back. It seems like that the perfcounter exporter mutate the values (e.g. from 100ns to 1sec). ref: windows_exporter/pkg/perflib/unmarshal.go Lines 87 to 94 in 470f5d5
Additionally, I have no clue, what the "secondValue" is. windows_exporter/pkg/perflib/perflib.go Line 348 in 470f5d5
I can't find anything at the MS documentation and no clue, whats the source of |
This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs. |
@breed808 I thought after the future use of PHD in favor registry based collectors. Since we can't guarantee the exact metrics value between registry based collectors PHD, I had the idea the offer both source at once. What did you think would be the best possible approach here?
Everyone else in the community is invited to provide feedback here. |
I like the global switch option - something like |
At the moment, I do not really have an Idea, what the "best" way of implementing the PDH native function. Currently, the exporter invoke perfdata once and collection can consume data from it. From code point of view, this is quite complex. Alternately, each collector invokes PDH calls on they own, but again, no idea if there are any downside, if the exporter holds multiple handles to the Windows API. At the end, invoking the PDH calls are really complex and it looks like there is no go library which make everything just developer friendly. And the code at telegraph looks very complex, https://github.com/influxdata/telegraf/blob/master/plugins/inputs/win_perf_counters/win_perf_counters.go and other implements need to validate, e.g. (https://github.com/elastic/beats/blob/main/metricbeat/helper/windows/pdh/pdh_windows.go) At the end, it's an really, really time consuming task. It's hard to find an start here. |
Yes I see, this is far from easy! You have already done a lot of work in the PR #1459 and it looks very good :) I have tried it out an run in a issue (German System hehe) Seems like windows does create the object names in the systems language.
|
I guess, the hypen is the issue here. Prometheus has UTF-8 support. But good catch. I may have to take a look to keep data non-localized. |
@DiniFarb could you please try out the lastest version of #1459 ? It's available here: https://github.com/prometheus-community/windows_exporter/actions/runs/10658016872/artifacts/1880022574 The functionally and options has been reduced. Instead using the implementation from telegraf, I build a own one. While it's supporting less, the a bit easier to debug in case of issue. For example, wildcards at counters has been removed. The counter values has been compared with the values from the other collectors and the values are equal. I would like to hear your feedback. |
I did a quick smoke test on a german win11 system with: PS C:\Users\*******\windows_exporter_binaries> .\windows_exporter-0.28.1-4-gdfc8d37-amd64.exe --log.level=debug --collectors.enabled="perfdata" --collector.perfdata.objects='[{"object":"Processor Information","instances":["*"],"counters": {"% Processor Time": {}}},{"object":"Memory","counters": {"Cache Faults/sec": {"type": "counter"}}}]'
ts=2024-09-02T07:47:21.713Z caller=exporter.go:147 level=debug msg="Logging has Started"
ts=2024-09-02T07:47:21.728Z caller=perfdata.go:97 level=warn msg="The perfdata collector is in an experimental state! The configuration may change in future. Please report any issues."
ts=2024-09-02T07:47:22.758Z caller=exporter.go:216 level=info msg="Running as *********"
ts=2024-09-02T07:47:22.759Z caller=exporter.go:223 level=info msg="Enabled collectors: perfdata"
ts=2024-09-02T07:47:22.759Z caller=exporter.go:258 level=info msg="Starting windows_exporter" version="(version=0.28.1-4-gdfc8d37, branch=HEAD, revision=dfc8d37dae0311bd2e2de503ed5c9efdd13069c4)"
ts=2024-09-02T07:47:22.759Z caller=exporter.go:259 level=info msg="Build context" build_context="(go=go1.22.6, platform=windows/amd64, user=runneradmin@fv-az1390-362, date=20240901-23:04:09, tags=unknown)"
ts=2024-09-02T07:47:22.759Z caller=exporter.go:260 level=debug msg="Go MAXPROCS" procs=16
ts=2024-09-02T07:47:22.759Z caller=tls_config.go:313 level=info msg="Listening on" address=[::]:9182
ts=2024-09-02T07:47:22.759Z caller=tls_config.go:316 level=info msg="TLS is disabled." http2=false address=[::]:9182
ts=2024-09-02T07:47:47.533Z caller=prometheus.go:191 level=debug msg="collector perfdata succeeded after 0.000000s." /metrics result
looks good 👍 only thing was that the query took a bit long: prefdata was ok:
but the perflib snapshot was set with (even though I had just the perfdata collector active)
As soon as I added another "classic" collector like For testing I changed the function like:
and it worked fast as always. This was of course there already but maybe not recognized, cos why would you run the classic win exporter with no collectors. But now with the new perfdata collector it is different. I think that if only the perfdata collector is active all functionality of perflib should be disabled. P.S. there is the possibility that I did configure something wrong - had not much time to look into it. I can manage some more time in the coming days if needed. |
Normally, I develop this exporter on my windows 10 machine and I did not have any issues. But I can remember that other users hat the issue as well. Ref: #1458 |
ah yes sure, there are non perflib collectors already - my mistake, what was I thinking. So yes this is a already existing issue and seems only to happen on win11. |
I will plan this change on 0.30 |
Nice let me know if I can help in any way :) |
@DiniFarb The generic perf counter collector will be part of the next release. After that, current collectors will be switched to the new system. In terms of the generic collector, think about your use-cases, testing testing testing. |
I see that in the v0.30.0-beta.0 release you use an environment variable |
Good call, added in #1723 |
Current Progress:
Reading the documentation about PerfCounter
Microsoft highly recommends to fetch data via PHD function instead native registry. It seems like that PDH function are more performance.
We have issues like #724 and the exporter does not work well under load and the mentioned Zabbix exporter is using PDH functions, too. Also datadog and telegraf using PDF library.
Ref:
We should look into it, since the Registry supports V1 PerfCounter only, while the PDH functions support both V1 and V2.
Since telegraph is using the same OSS license, we should consider to use telegraf libraries as base instead starting from scratch. The OTEL collector is doing the same.
The text was updated successfully, but these errors were encountered: