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

Inputs.win_perf_counters.object sends one counter per interval #2895

Closed
bj2001holt opened this issue Jun 7, 2017 · 7 comments · Fixed by #4036
Closed

Inputs.win_perf_counters.object sends one counter per interval #2895

bj2001holt opened this issue Jun 7, 2017 · 7 comments · Fixed by #4036
Labels
area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) bug unexpected problem or unintended behavior platform/windows
Milestone

Comments

@bj2001holt
Copy link

Bug report

When using an "inputs.win_perf_counters.object" where you are collecting multiple counters you will only see a single counter collected at each interval. For example, the PhysicalDisk Object collects 8 counters, however it only collects one of them per interval. This removes the ability to correlate performance data.

Relevant telegraf.conf:

[agent]
interval = "5s"
round_interval = true
metric_buffer_limit = 1000
flush_buffer_when_full = true
flush_interval = "5s"
flush_jitter = "0s"

[[outputs.influxdb]]
precision = "s"

[[inputs.win_perf_counters.object]]
ObjectName = "PhysicalDisk"
Instances = ["*"]
Counters = [
"Disk Read Bytes/sec",
"Disk Write Bytes/sec",
"Current Disk Queue Length",
"Disk Reads/sec",
"Disk Writes/sec",
"% Disk Time",
"% Disk Read Time",
"% Disk Write Time",
]
Measurement = "win_diskio"

System info:

Influx and Grafana on CentOS 7, telegraf on Windows Server 2012. Tested with telegraf version 1.3.1 and 1.0.0 beta 3.

Steps to reproduce:

  1. Install telegraf on windows system using default win_perf_counters
  2. From Influx query
    SELECT Current_Disk_Queue_Length as "C: Current Queue Length", Disk_Reads_persec as "C: Read/Sec", Disk_Writes_persec as "C: Writes/Sec" FROM "win_diskio" WHERE "host" = '' AND "instance" = '0 C:'

Expected behavior:

All counters should be collected with every interval

Actual behavior:

One counter is collected in each interval

@danielnelson danielnelson added area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) bug unexpected problem or unintended behavior platform/windows labels Jun 7, 2017
@cdjc
Copy link

cdjc commented Apr 17, 2018

The following diff fixes this problem. That is, it collects counters for each instance, rather then one at a time. The description in the issue is a bit ambiguous: Currently all counters are indeed measured, but, for example, measuring 8 counters on 8 cores will give 64 messages, rather than 8 messages (each with 8 fields). This diff fixes that problem. I'm new to go, so feel free to tidy up. @danielnelson Do I need to sign the CLA or is this snippet small enough not to worry?

diff --git a/plugins/inputs/win_perf_counters/win_perf_counters.go b/plugins/inputs/win_perf_counters/win_perf_counters.go
index 7e3991d..9a9e6b4 100644
--- a/plugins/inputs/win_perf_counters/win_perf_counters.go
+++ b/plugins/inputs/win_perf_counters/win_perf_counters.go
@@ -192,6 +192,14 @@ func (m *Win_PerfCounters) Gather(acc telegraf.Accumulator) error {
        var size uint32 = uint32(unsafe.Sizeof(PDH_FMT_COUNTERVALUE_ITEM_DOUBLE{}))
        var emptyBuf [1]PDH_FMT_COUNTERVALUE_ITEM_DOUBLE // need at least 1 addressable null ptr.
 
+       type InstanceGrouping struct {
+               name       string
+               instance   string
+               objectname string
+       }
+
+       var collectFields = make(map[InstanceGrouping]map[string]interface{})
+
        // For iterate over the known metrics and get the samples.
        for _, metric := range m.itemCache {
                // collect
@@ -231,20 +239,22 @@ func (m *Win_PerfCounters) Gather(acc telegraf.Accumulator) error {
                                        }
 
                                        if add {
-                                               fields := make(map[string]interface{})
                                                tags := make(map[string]string)
                                                if s != "" {
                                                        tags["instance"] = s
                                                }
                                                tags["objectname"] = metric.objectName
-                                               fields[sanitizedChars.Replace(metric.counter)] =
-                                                       float32(c.FmtValue.DoubleValue)
 
                                                measurement := sanitizedChars.Replace(metric.measurement)
                                                if measurement == "" {
                                                        measurement = "win_perf_counters"
                                                }
-                                               acc.AddFields(measurement, fields, tags)
+                                               var instance = InstanceGrouping{measurement, s, metric.objectName}
+
+                                               if collectFields[instance] == nil {
+                                                       collectFields[instance] = make(map[string]interface{})
+                                               }
+                                               collectFields[instance][sanitizedChars.Replace(metric.counter)] = float32(c.FmtValue.DoubleValue)
                                        }
                                }
 
@@ -257,6 +267,14 @@ func (m *Win_PerfCounters) Gather(acc telegraf.Accumulator) error {
                }
        }
 
+       for instance, fields := range collectFields {
+               var tags = map[string]string{
+                       "instance":   instance.instance,
+                       "objectname": instance.objectname,
+               }
+               acc.AddFields(instance.name, fields, tags)
+       }
+
        return nil
 }

@danielnelson
Copy link
Contributor

Thanks @cdjc, you will need to sign the CLA still, can you open this as a pull request as well?

@cdjc
Copy link

cdjc commented Apr 17, 2018

I can sign the CLA, but I've never done a pull request before, so I'll have to figure it out. I did briefly look at what was required for a pull request, but it just seemed so much more complicated than posting a diff.

@danielnelson
Copy link
Contributor

I can create the PR if your not setup for it. If you have git running though can you create the diff using: git format-patch 7e3991d, this way the change is properly attributed.

@cdjc
Copy link

cdjc commented Apr 18, 2018

I have the patch file. Should I attach it here, or wait until you have set up the pull request?

@danielnelson
Copy link
Contributor

Attach it here please

@cdjc
Copy link

cdjc commented Apr 18, 2018

0001-Fix-win_perf_counters-to-collect-counters-per-instan.zip

Here it is (zipped, as .patch aren't allowed to be uploaded).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/windows Related to windows plugins (win_eventlog, win_perf_counters, win_services) bug unexpected problem or unintended behavior platform/windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants