Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

pmval does not print negative values #67

Closed
rvernica opened this Issue Feb 9, 2016 · 5 comments

Comments

Projects
None yet
3 participants
Contributor

rvernica commented Feb 9, 2016

I modified the acme.c example provided in the Programmer's Guide and here do generate negative numbers.

When reading the values produced, pminfo displays the values correctly but pmval prints ! in place of the negative values.

$ pminfo -f mmv.acme.products.count

mmv.acme.products.count
    inst [0 or "Anvils"] value -1849.6024
    inst [1 or "Rockets"] value -3.6371837e+31
    inst [2 or "Giant_Rubber_Bands"] value -6.3034399e+09

$ pmval mmv.acme.products.count

metric:    mmv.acme.products.count
host:      beaglebone-01
semantics: instantaneous value
units:     byte
samples:   all
full label for instance[0]: Anvils
full label for instance[1]: Rockets
full label for instance[2]: Giant_Rubber_Bands

       Anvils       Rockets Giant_Rubber_
            !     1.194E-05     3.127E-35
            !     2.816E-30     1.712E-17
    4.185E-28        0.8811     1.438E-32

This might be the location in pmval where this might need to be fixed pmval.c#L505

You can see here the changes made to acme.c to print negative numbers.

Contributor

natoscott commented Feb 9, 2016

Bit more info...

| nathans: Rares, ooc, what are you measuring that you're expecting to go negative?
| Rares: I am measuring curents
| Rares: temperature would be another example
| nathans> ok, yep
| Rares: BTW, I struggled a bit on setting the dimensions
| nathans> i'll bet
| Rares: I used this MMV_UNITS(1,0,0,PM_SPACE_BYTE,0,0), so I put my current/temperature in the Space dimension
| Rares: did I get it right?
| nathans: well, there is no real "right" in that situation - pcp was not really designed to measure that sort of thing - i would be inclined to just go with all zeroes for units
| Rares: I see. OK, I will play around with it

Contributor

kmcdonell commented Feb 9, 2016

OK ... in 21 years this is the first time I've ever seen a PMDA that exports a negative value for an instantaneous metric ... nothing wrong with that, just outside the design envelope (and hence out of mind).

I would suspect that a lot more than pmval would be broken by this ... we'll need to extend the sample PMDA, then audit all the tools. But I do not expect the scope of the changes to be large (once you've addressed all 4 PM_TYPE_*s that could be used to hold a negative value).

Thanks, Rares.

Contributor

kmcdonell commented Feb 9, 2016

Temperature ... that's a blast from the past!
One of the first PMDAs I wrote as a proof of concept for data outside Unix kernel metrics (this was very early on) was the roomtemp PMDA that used a 1-wire Dallas digital thermometer connected via the COM port to my laptop.
At the time the only testing environment was the dodgy freezer of the refrigerator in the hotel room where I wrote the PMDA one evening, and it was barely able to get to 0, much less anything negative!

Contributor

kmcdonell commented Feb 9, 2016

But as Frank has pointed out there is already at least one metric with instantaneous semantics and often negative value ...
kenj@bozo:~$ pminfo -dtf swapdev.priority

swapdev.priority [swap resource priority]
Data Type: 32-bit int InDom: 60.6 0xf000006
Semantics: instant Units: count
inst [0 or "/dev/sda7"] value -1

Now the Units are wrong (I'll fix that), but otherwise similar ... but interestingly

kenj@bozo:~$ pmval swapdev.priority

metric: swapdev.priority
host: bozo
semantics: instantaneous value
units: count
samples: all

/dev/sda7
-1
-1

works ... so this may be as small as a corner case in the handling of non-integer values in pmval, as Rares originally suggested.

Contributor

kmcdonell commented Feb 14, 2016

As expected, this required a small number of relatively localized code changes ... the bulk of the PMAPI code paths work just fine with negative metric values.
All fixed in my tree now, commits will flow upstream in due course, so I'm closing this issue.
Thanks Rares.

@kmcdonell kmcdonell closed this Feb 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment